Opened 15 years ago
Closed 12 years ago
#12823 closed Bug (fixed)
Wrong SQL query when using .exclude()
Reported by: | olepw | Owned by: | |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 1.1 |
Severity: | Normal | Keywords: | |
Cc: | philippe@…, hvdklauw@…, sebastian.goll@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Given the following model:
from django.db import models class Book(models.Model): title = models.TextField() chapter = models.ForeignKey('Chapter') class Chapter(models.Model): title = models.TextField() paragraph = models.ForeignKey('Paragraph') class Paragraph(models.Model): text = models.TextField() page = models.ManyToManyField('Page') class Page(models.Model): text = models.TextField()
Create the following query:
q = Book.objects.exclude(chapter__paragraph__page__text='foo')
From this I would expect to get all books that does not contain the text 'foo'
print q.query
This produces:
SELECT `book_book`.`id`, `book_book`.`title`, `book_book`.`chapter_id` FROM `book_book` INNER JOIN `book_chapter` ON (`book_book`.`chapter_id` = `book_chapter`.`id`) WHERE NOT (`book_chapter`.`paragraph_id` IN (SELECT U1.`id` FROM `book_chapter` U1 INNER JOIN `book_paragraph` U2 ON (U1.`paragraph_id` = U2.`id`) INNER JOIN `book_paragraph_page` U3 ON (U2.`id` = U3.`paragraph_id`) INNER JOIN `book_page` U4 ON (U3.`page_id` = U4.`id`) WHERE U4.`text` = foo ) AND `book_chapter`.`paragraph_id` IS NOT NULL)
The error here is:
WHERE NOT (`book_chapter`.`paragraph_id` IN (SELECT U1.`id` FROM `book_chapter` U1
Where django tries to compare book_chapter.paragraph_id with book_chapter.id, two fields from the same table.
I would expect the query to be:
WHERE NOT (`book_book`.`chapter_id` IN (SELECT U1.`id` FROM `book_chapter` U1
Attachments (6)
Change History (28)
comment:1 by , 15 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 14 years ago
comment:3 by , 14 years ago
Has patch: | set |
---|
A quick note on the tests: I had to modify some ids to make the bug show because it basically boils down to comparing one model's PK against a PK value from another model. When you're testing with hand-created models whose PKs are all 1,2,3... it's easy to miss.
comment:4 by , 14 years ago
Patch needs improvement: | set |
---|
The patch should be against SVN trunk. Version 1.1 is in security-fixes-only mode.
comment:5 by , 14 years ago
Patch needs improvement: | unset |
---|
Sorry I was in the process of porting the tests to trunk. My production app uses 1.1 so that's what I used to work.
comment:6 by , 14 years ago
Cc: | added |
---|
comment:7 by , 14 years ago
milestone: | → 1.3 |
---|
I can confirm this bug.
Don't look at the models, the example is horrible and makes no sense in relation to real books ;-)
Setting this as 1.3 milestone.
comment:8 by , 14 years ago
Resuling SQL from the patch applied to Django 1.2.3 (with books as appname instead of book):
SELECT "books_book"."id", "books_book"."title", "books_book"."chapter_id" FROM "books_book" INNER JOIN "books_chapter" ON ("books_book"."chapter_id" = "books_chapter"."id") WHERE NOT (("books_book"."chapter_id" IN (SELECT U1."id" FROM "books_chapter" U1 INNER JOIN "books_paragraph" U2 ON (U1."paragraph_id" = U2."id") INNER JOIN "books_paragraph_page" U3 ON (U2."id" = U3."paragraph_id") INNER JOIN "books_page" U4 ON (U3."page_id" = U4."id") WHERE U4."text" = foo ) AND "books_chapter"."paragraph_id" IS NOT NULL))
I'm a bit unsure about the JOIN on the "books_chapter" in the main query which then isn't used at all.
comment:9 by , 14 years ago
Cc: | added |
---|
comment:10 by , 14 years ago
I think I understand the problem: we need to unref the corresponding alias in the query so the extra unused join isn't generated.
comment:11 by , 14 years ago
After giving it some thought I realized the join is actually used by the AND "books_chapter"."paragraph_id" IS NOT NULL))
clause. It looks like this is by design, but then maybe we could optimize the inner query to use the chapter table directly instead of joining it again ?
comment:12 by , 14 years ago
Type: | → Bug |
---|
comment:13 by , 14 years ago
Severity: | → Normal |
---|
comment:14 by , 14 years ago
Easy pickings: | unset |
---|---|
Patch needs improvement: | set |
12823_trunk.diff fails to apply cleanly on to trunk
by , 13 years ago
Attachment: | 12823_1.2.diff added |
---|
comment:15 by , 13 years ago
Patch needs improvement: | unset |
---|---|
UI/UX: | unset |
Disregard previous patches, but I can't remove them because I used a different account...
comment:17 by , 13 years ago
Cc: | added |
---|
comment:18 by , 13 years ago
Owner: | changed from | to
---|
comment:19 by , 12 years ago
Status: | new → assigned |
---|
comment:20 by , 12 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
comment:21 by , 12 years ago
This is fixed in current master. The generated query is:
SELECT "queries_book"."id", "queries_book"."title", "queries_book"."chapter_id" FROM "queries_book" INNER JOIN "queries_chapter" ON ("queries_book"."chapter_id" = "queries_chapter"."id") WHERE NOT ("queries_chapter"."paragraph_id" IN (SELECT U3."paragraph_id" FROM "queries_paragraph_page" U3 INNER JOIN "queries_page" U4 ON (U3."page_id" = U4."id") WHERE U4."text" = pg1 ))
I will also add a little patch to sql/query.py which will get rid of one extra IS NOT NULL check.
comment:22 by , 12 years ago
Owner: | set to |
---|---|
Resolution: | → fixed |
Status: | new → closed |
by , 11 years ago
Attachment: | 12823_1.5.diff added |
---|
updated patch for django 1.5.5 (bug not fixed in this release)
This may or may not be a dup of #11052