Deleting model instance with a string id and m2m relation fails

[Ronny Haryanto]

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 (models.py) 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 manage.py shell:

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

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

In [3]: w.save()

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)
    567 
    568         # Actually delete the objects.
--> 569         delete_objects(seen_objs)
    570 
    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)
   1038 
   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/related.py 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/related.py 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)
    362 
    363     def contribute_to_class(self, cls, name):

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

[Russell Keith-Magee]

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.

[Russell Keith-Magee]

(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.

[Russell Keith-Magee]

(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.

[Ronny Haryanto]

Thanks, Russell. It's working again now.

[Jacob]

Milestone 1.1 deleted