Opened 15 years ago

Closed 15 years ago

Last modified 13 years ago

#11311 closed (fixed)

Deleting model instance with a string id and m2m relation fails

Reported by: Ronny Haryanto Owned by: nobody
Component: Database layer (models, ORM) Version: dev
Severity: Keywords: delete string pk m2m
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


Say a model has a CharField as its primary key and the model also has a many to many relation to some other "normal" model, deleting an instance of the model with string primary key like "abc" fails with an exception, but not with a string id that can be converted into an int, e.g. "1". Seems like the code assumes that primary keys are always int()?

Using django from svn trunk r10982.

The following is a very small test case ( that demonstrate the issue.

from django.db import models

class Line(models.Model):
    name = models.CharField(max_length=100)

class Worksheet(models.Model):
    id = models.CharField(primary_key=True, max_length=100)
    lines = models.ManyToManyField(Line, blank=True, null=True)

After running syncdb (happens with sqlite3 and postgresql-8.3 btw), then the following is the output from shell:

In [1]: from x.y.models import *

In [2]: w = Worksheet(id='abc')

In [3]:

In [4]: w
Out[4]: <Worksheet: Worksheet object>

In [5]: w.delete()
ERROR: An unexpected error occurred while tokenizing input
The following traceback may be corrupted or invalid
The error message is: ('EOF in multi-line statement', (20, 0))

ValueError                                Traceback (most recent call last)

/private/tmp/x/<ipython console> in <module>()

/Library/Python/2.5/site-packages/django/db/models/base.pyc in delete(self)
    568         # Actually delete the objects.
--> 569         delete_objects(seen_objs)
    571     delete.alters_data = True

/Library/Python/2.5/site-packages/django/db/models/query.pyc in delete_objects(seen_objs)
   1035             pk_list = [pk for pk,instance in items]
   1036             del_query = sql.DeleteQuery(cls, connection)
-> 1037             del_query.delete_batch_related(pk_list)
   1039             update_query = sql.UpdateQuery(cls, connection)

/Library/Python/2.5/site-packages/django/db/models/sql/subqueries.pyc in delete_batch_related(self, pk_list)
     68                 where.add((Constraint(None, f.m2m_column_name(), f), 'in',
     69                         pk_list[offset : offset + GET_ITERATOR_CHUNK_SIZE]),
---> 70                         AND)
     71                 if w1:
     72                     where.add(w1, AND)

/Library/Python/2.5/site-packages/django/db/models/sql/where.pyc in add(self, data, connector)
     54         if hasattr(obj, "process"):
     55             try:
---> 56                 obj, params = obj.process(lookup_type, value)
     57             except (EmptyShortCircuit, EmptyResultSet):
     58                 # There are situations where we want to short-circuit any

/Library/Python/2.5/site-packages/django/db/models/sql/where.pyc in process(self, lookup_type, value)
    267         try:
    268             if self.field:
--> 269                 params = self.field.get_db_prep_lookup(lookup_type, value)
    270                 db_type = self.field.db_type()
    271             else:

/Library/Python/2.5/site-packages/django/db/models/fields/ in get_db_prep_lookup(self, lookup_type, value)
    160             return [pk_trace(value)]
    161         if lookup_type in ('range', 'in'):
--> 162             return [pk_trace(v) for v in value]
    163         elif lookup_type == 'isnull':
    164             return []

/Library/Python/2.5/site-packages/django/db/models/fields/ in pk_trace(value)
    137             if lookup_type in ('range', 'in'):
    138                 v = [v]
--> 139             v = field.get_db_prep_lookup(lookup_type, v)
    140             if isinstance(v, list):
    141                 v = v[0]

/Library/Python/2.5/site-packages/django/db/models/fields/__init__.pyc in get_db_prep_lookup(self, lookup_type, value)
    210             return [self.get_db_prep_value(value)]
    211         elif lookup_type in ('range', 'in'):
--> 212             return [self.get_db_prep_value(v) for v in value]
    213         elif lookup_type in ('contains', 'icontains'):
    214             return ["%%%s%%" % connection.ops.prep_for_like_query(value)]

/Library/Python/2.5/site-packages/django/db/models/fields/__init__.pyc in get_db_prep_value(self, value)
    359         if value is None:
    360             return None
--> 361         return int(value)
    363     def contribute_to_class(self, cls, name):

ValueError: invalid literal for int() with base 10: 'abc'

Change History (5)

comment:1 by Russell Keith-Magee, 15 years ago

Triage Stage: UnreviewedAccepted

Ok... this one opens up a nest of vipers.

This is a regression caused by [10952], which closed #10785. That ticket in turn makes reference to #10243, and brokenness in get_db_prep_lookup(). Now that I'm digging deeper, I'm starting to see the magnitude of the problem that Malcolm was referring to.

For v1.1, we may need to simply revert [10952] and live with the edge case that won't work.

comment:2 by Russell Keith-Magee, 15 years ago

Resolution: fixed
Status: newclosed

(In [11007]) Fixed #11311 -- Reverted [10952], Refs #10785. Changeset [10952] caused problems with m2m relations between models that had non-integer primary keys. Thanks to Ronny for the report and test case.

comment:3 by Russell Keith-Magee, 15 years ago

(In [11008]) [1.0.X] Fixed #11311 -- Reverted [10952], Refs #10785. Changeset [10952] caused problems with m2m relations between models that had non-integer primary keys. Thanks to Ronny for the report and test case.

comment:4 by Ronny Haryanto, 15 years ago

Thanks, Russell. It's working again now.

comment:5 by Jacob, 13 years ago

milestone: 1.1

Milestone 1.1 deleted

Note: See TracTickets for help on using tickets.
Back to Top