Opened 15 years ago

Closed 14 years ago

Last modified 12 years ago

#11764 closed (fixed)

Bugfix: typo with avoid.update(..) in db/models/sql/compiler.py:SQLCompiler:fill_related_selections

Reported by: Aurelio Tinio Owned by: Ramiro Morales
Component: Database layer (models, ORM) Version: dev
Severity: Keywords:
Cc: django@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

There looks to be a bug due to a typo in db/models/sql/query.py .

The issue became apparent when it prevented the display of a ForeignKey field in list_display, when there is another ForeignKey to the base class of the model with the following error displayed:

Caught an exception while rendering: update() takes exactly one argument (2 given)

Original Traceback (most recent call last):
  File "/tippit/django-projects/django/Django-1.1/django/template/debug.py", line 71, in render_node
    result = node.render(context)
  File "/tippit/django-projects/django/Django-1.1/django/template/__init__.py", line 936, in render
    dict = func(*args)
  File "/tippit/django-projects/django/Django-1.1/django/contrib/admin/templatetags/admin_list.py", line 253, in result_list
    'results': list(results(cl))}
  File "/tippit/django-projects/django/Django-1.1/django/contrib/admin/templatetags/admin_list.py", line 247, in results
    for res in cl.result_list:
  File "/tippit/django-projects/django/Django-1.1/django/db/models/query.py", line 106, in _result_iter
    self._fill_cache()
  File "/tippit/django-projects/django/Django-1.1/django/db/models/query.py", line 692, in _fill_cache
    self._result_cache.append(self._iter.next())
  File "/tippit/django-projects/django/Django-1.1/django/db/models/query.py", line 238, in iterator
    for row in self.query.results_iter():
  File "/tippit/django-projects/django/Django-1.1/django/db/models/sql/query.py", line 287, in results_iter
    for rows in self.execute_sql(MULTI):
  File "/tippit/django-projects/django/Django-1.1/django/db/models/sql/query.py", line 2360, in execute_sql
    sql, params = self.as_sql()
  File "/tippit/django-projects/django/Django-1.1/django/db/models/sql/query.py", line 395, in as_sql
    self.pre_sql_setup()
  File "/tippit/django-projects/django/Django-1.1/django/db/models/sql/query.py", line 589, in pre_sql_setup
    self.fill_related_selections()
  File "/tippit/django-projects/django/Django-1.1/django/db/models/sql/query.py", line 1401, in fill_related_selections
    ())
TypeError: update() takes exactly one argument (2 given)

A patch has been attached applied against changeset 11475.

Attachments (4)

patch_against_11475.diff (771 bytes ) - added by Aurelio Tinio 15 years ago.
11764.diff (1.6 KB ) - added by Ramiro Morales 14 years ago.
Same code patch plus regression test
11764.2.diff (1.7 KB ) - added by Greg Wogan-Browne 14 years ago.
Updated patch against rev12438
11764.patch (1.8 KB ) - added by Greg Wogan-Browne 14 years ago.
Updated patch against rev13018

Download all attachments as: .zip

Change History (13)

by Aurelio Tinio, 15 years ago

Attachment: patch_against_11475.diff added

by Ramiro Morales, 14 years ago

Attachment: 11764.diff added

Same code patch plus regression test

comment:1 by Ramiro Morales, 14 years ago

Owner: changed from nobody to Ramiro Morales
Status: newassigned
Triage Stage: UnreviewedAccepted

Finally got around to reproduce this corner case thing by tracing and copying the treatment the queryset gets in the admin app to show a change list for the relevant model (there is no need to create model instances at all). Critical to make the bug show itself are the select_related() call and iterating over the QS (forcing its evaluation with list() doesn't trigger it).

comment:2 by Alex Gaynor, 14 years ago

Triage Stage: AcceptedReady for checkin

by Greg Wogan-Browne, 14 years ago

Attachment: 11764.2.diff added

Updated patch against rev12438

comment:3 by Greg Wogan-Browne, 14 years ago

Cc: django@… added
Summary: Bugfix: typo with avoid.update(..) in db/models/sql/query.py:BaseQuery:fill_related_selectionsBugfix: typo with avoid.update(..) in db/models/sql/compiler.py:SQLCompiler:fill_related_selections

Updated patch as the typo had moved from [source:django/trunk/django/db/models/sql/query.py query.py] to [source:django/trunk/django/db/models/sql/compiler.py compiler.py].

in reply to:  3 comment:4 by Ramiro Morales, 14 years ago

Replying to wogan:

Updated patch as the typo had moved from [source:django/trunk/django/db/models/sql/query.py query.py] to [source:django/trunk/django/db/models/sql/compiler.py compiler.py].

Thanks for updating the patch. I had spotted the code move and implemented a simliar updating locally. Problem is that the regression test case I had added now pass with and without the fix applied. I ran the full Django test suite with code coverage analysis turned on and it shows that exact code path isn't exercised anymore.

This means we need to update the test because the core committers won't get this change in without a regression test. But I don't know if it's possible at all.

Sorry for not updating the ticket, I only made a comment about this on the #django-dev IRC channel.

comment:5 by Alex Gaynor, 14 years ago

Triage Stage: Ready for checkinAccepted

Moving off of RFC since ramiro notes that we can longer reproduce this :/

comment:6 by Russell Keith-Magee, 14 years ago

milestone: 1.21.3

Not critical for 1.2

by Greg Wogan-Browne, 14 years ago

Attachment: 11764.patch added

Updated patch against rev13018

comment:7 by Greg Wogan-Browne, 14 years ago

milestone: 1.31.2
Triage Stage: AcceptedReady for checkin

Updated the patch, made a minor change to the test.

I could successfully run the test (failing appropriately when the patch is not applied).

Is it possible to get this in for 1.2?

comment:8 by Russell Keith-Magee, 14 years ago

Resolution: fixed
Status: assignedclosed

(In [13019]) Fixed #11764 -- Added a missing set of parentheses in a call calculating the select_related tables. Thanks to aurelio for the report and original patch, and wogan for the updated patch.

comment:9 by Jacob, 12 years ago

milestone: 1.2

Milestone 1.2 deleted

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