#16715: Wrong JOIN with nested null-able foreign keys
-------------------------------------+-------------------------------------
Reporter: sebastian | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: SVN
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: join, values, | Needs documentation: 0
nested, foreign key, null-able | Patch needs improvement: 0
Has patch: 1 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by sebastian):
* has_patch: 0 => 1
Comment:
I uploaded another patch. This one fixes the problem at what I think is
the root of it. The patch makes two small changes to
`django/db/models/sql/compiler.py` and `django/db/models/sql/query.py`.
In `compiler.py`, we force the reverse sides of foreign key relations to
be null-able. This makes sense, since the existence of a remote object
cannot be guaranteed, so we must use LEFT OUTER JOINs instead of INNER
JOINs. I think the original code resulted from simply copy-pasting the
section a couple lines above my fix which deals with non-related objects
(where checking for `field.null` makes perfect sense), and forgetting to
make the appropriate change to apply it to related objects. Please correct
me if I'm wrong in my assumptions here.
The other change applies to `query.py`. Here, I added a simple check when
introducing additional joins in `setup_joins(…)`. The reasoning behind
this change is that we must mark joins referring to existing LEFT OUTER
ones as null-able, so that they may eventually be promoted to LEFT OUTER
JOINs themselves.
These two changes combined fix the `select_related(…)` and `values(…)`
problems in this ticket. My patch also includes a bunch of tests for these
two methods (also some simple `filter`/`exclude` queries for good
measure).
I made sure that all existing Django tests run as before, so that no
failures or errors got introduced by my patch. (In particular,
`regressiontests/queries` still succeeds in `test_tickets_5324_6704(…)`
where the existence of expected INNER JOINs is checked which was created
in response to tickets #5324 and #6704.)
Please let me know what you think. Is this patch/fix mature enough to be
included in trunk? Are there any unintended side-effects?
--
Ticket URL: <https://code.djangoproject.com/ticket/16715#comment:10>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/django-updates?hl=en.
