Opened 11 years ago

Closed 10 years ago

#21612 closed Bug (fixed)

queryset update ignores to_field on foreign keys

Reported by: berndtj@… Owned by: nobody
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords:
Cc: berndtj@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When issuing a batch update on a queryset, the to_field of the related model is ignored in some cases. For instance, I am using a uuid field as a foreign key field (to_field). When I try to update the relationship, the pk is used for in the foreign key column (<foreign_model>_id) instead of the uuid. For example given a queryset of BracketVolumeTemplates which have a parent field which refers to a BracketVolume:

# The field definition
parent = models.ForeignKey(
        'BracketVolume', null=True, blank=True, to_field='uuid',
        related_name='parent_templates')

# We start with a queryset of one which has a BracketVolumeTemplate which refers to a BracketVolume via the parent filed
ipdb> queryset
[<BracketVolumeTemplate [snapshot based] ID: 10dc67d94f8347d195e25eca6f3c22bb - created by test@brkt.com>]
ipdb> queryset[0].parent
<BracketVolume [snapshot] Requested State: DELETED ID: 2de07185d9744fdd83b5f683dfe5a2aa - created by test@brkt.com>

# The parent_id is correctly set as the UUID
ipdb> queryset[0].parent_id
u'2de07185d9744fdd83b5f683dfe5a2aa'

# We are updating with the kwargs as follows
ipdb> kwargs
{'parent': <BracketVolume [snapshot] Requested State: AVAILABLE ID: 62b72525425b4252925ba9ce48d42428 - created by test@brkt.com>}
ipdb> queryset.update(**kwargs)
1

# Wait, parent_id should be the UUID, but instead the pk is used for the foreign key
ipdb> queryset[0].parent_id
u'3'

# Now the relationship is broken
ipdb> queryset[0].parent
*** DoesNotExist: BracketVolume matching query does not exist. Lookup parameters were {'uuid__exact': u'3'}

# Queryset hides the db_columns, so we can't work around directly
ipdb> queryset.update(parent_id='2de07185d9744fdd83b5f683dfe5a2aa')
*** FieldDoesNotExist: BracketVolumeTemplate has no field named 'parent_id'

# The below workaround does work...
ipdb> queryset.update(parent='2de07185d9744fdd83b5f683dfe5a2aa')
1
ipdb> queryset[0].parent
<BracketVolume [snapshot] Requested State: DELETED ID: 2de07185d9744fdd83b5f683dfe5a2aa - created by test@brkt.com>
ipdb> c

I can work around this by adding the following code in my update code, but it's not ideal:

for key in kwargs:
            field = queryset.model._meta.get_field(key)
            if isinstance(field, ForeignKey):
                model = kwargs[key]
                if not model:
                    continue
                rel_att = field.rel.field_name
                kwargs[key] = getattr(model, rel_att)
        queryset.update(modified_by=modified_by_id, **kwargs)

Change History (5)

comment:1 by Tim Graham, 11 years ago

Component: UncategorizedDatabase layer (models, ORM)

I see you've set the version to 1.5. Any chance you can test on 1.6 and/or master? If the bug is still present, a test case for Django's test suite would be helpful.

comment:2 by berndtj@…, 11 years ago

I was gone during the holidays.

I can verify that this behavior exists on the 1.6 branch:

In [23]: b
Out[23]: <BracketUser: new4@berndt.com>

In [25]: BracketGroup.objects.filter(created_by='berndt@…')
Out[25]: [<BracketGroup [made by old] ID: 18b72e3766d947b29f802b637785caf8 - created by berndt@…>, <BracketGroup [default] ID: 96c6018e9ebc4c69a96636bab4ee9aaf - created by berndt@…>]

In [26]: qs = BracketGroup.objects.filter(created_by='berndt@…')

In [27]: qs.update(created_by=b)


IntegrityError Traceback (most recent call last)
/home/vagrant/work/django/<ipython-input-27-1536333ef175> in <module>()


/home/vagrant/work/django/django/db/models/query.pyc in update(self, kwargs)

488 query.add_update_values(kwargs)
489 with transaction.commit_on_success_unless_managed(using=self.db):

--> 490 rows = query.get_compiler(self.db).execute_sql(None)

491 self._result_cache = None
492 return rows

/home/vagrant/work/django/django/db/transaction.pyc in exit(self, exc_type, exc_value, traceback)

303 # Commit transaction

304 try:

--> 305 connection.commit()

306 except DatabaseError:
307 connection.rollback()

/home/vagrant/work/django/django/db/backends/init.pyc in commit(self)

166 self.validate_thread_sharing()
167 self.validate_no_atomic_block()

--> 168 self._commit()

169 self.set_clean()
170

/home/vagrant/work/django/django/db/backends/init.pyc in _commit(self)

134 if self.connection is not None:
135 with self.wrap_database_errors:

--> 136 return self.connection.commit()

137
138 def _rollback(self):

/home/vagrant/work/django/django/db/utils.pyc in exit(self, exc_type, exc_value, traceback)

97 if dj_exc_type not in (DataError, IntegrityError):
98 self.wrapper.errors_occurred = True

---> 99 six.reraise(dj_exc_type, dj_exc_value, traceback)

100
101 def call(self, func):

/home/vagrant/work/django/django/db/backends/init.pyc in _commit(self)

134 if self.connection is not None:
135 with self.wrap_database_errors:

--> 136 return self.connection.commit()

137
138 def _rollback(self):

IntegrityError: insert or update on table "nomos_bracketgroup" violates foreign key constraint "created_by_id_refs_username_32f63683ba6621ac"
DETAIL: Key (created_by_id)=(8) is not present in table "nomos_bracketuser".

In [28]: b.pk
Out[28]: 8

It's a bit of a different example, but the issue is the same. As you can see, django is trying to update the created_by_id with the PK instead of the to_field which in this case is the username attribute of BracketUser... so it fails.

Testing with master is a bit more work, as things don't appear to just work...

comment:3 by Baptiste Mispelon, 11 years ago

Triage Stage: UnreviewedAccepted
Type: UncategorizedBug
Version: 1.5master

Hi,

I can reproduce the issue on master as well, using the following models:

class Foo(models.Model):
    c = models.CharField(max_length=10, unique=True)


class Bar(models.Model):
    foo = models.ForeignKey(Foo, to_field='c')

Using those models, the following test case demonstrates the issue:

def test_issue_21612(self):
    a = Foo.objects.create(c='a')
    b = Foo.objects.create(c='b')
    Bar.objects.create(foo=a)
    Bar.objects.update(foo=b)
    bar = Bar.objects.get()
    self.assertEqual(bar.foo_id, b.c)

comment:4 by Karen Tracey, 10 years ago

Has patch: set

Verified this still doesn't work. Here is a pull request with test case and possible fix, which seems surprisingly simple:

https://github.com/django/django/pull/3548

comment:5 by Tim Graham <timograham@…>, 10 years ago

Resolution: fixed
Status: newclosed

In dec93d89912965f94f9e942f0a89ca586fb91454:

Fixed #21612 -- Made QuerySet.update() respect to_field

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