Opened 5 years ago

Closed 5 years ago

#20788 closed Bug (fixed)

exclude makes bad query following two FKs and a M2M

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

Description

Hi,

With models like this:

from django.db import models


class Publication(models.Model):
    pass


class Article(models.Model):
    publications = models.ManyToManyField(Publication)


class Section(models.Model):
    article = models.ForeignKey(Article)


class Sentence(models.Model):
    section = models.ForeignKey(Section)

The query to get all sentences that aren't in publication 1 looks like:

>>> models.Sentence.objects.exclude(section__article__publications__id=1)

However, this generates the following SQL:

SELECT "marbury_sentence"."id", "marbury_sentence"."section_id"

FROM "marbury_sentence"
    INNER JOIN "marbury_section" ON ("marbury_sentence"."section_id" = "marbury_section"."id")
    INNER JOIN "marbury_article" ON ("marbury_section"."article_id" = "marbury_article"."id")

WHERE NOT (

    (
        "marbury_section"."article_id" IN (
            SELECT U1."id"
            FROM "marbury_section" U1
                INNER JOIN "marbury_article" U2 ON (U1."article_id" = U2."id")
                INNER JOIN "marbury_article_publications" U3 ON (U2."id" = U3."article_id")
            WHERE (U3."publication_id" = 1  AND U1."id" IS NOT NULL)
        )
        AND "marbury_article"."id" IS NOT NULL
    )
)

The WHERE clause compares "marbury_section"."article_id" to *section* IDs returned by the subquery. This means that the exclusion won't be enforced, or worse, will be enforced incorrectly, if article IDs and section IDs overlap.

Here is a simple test:

# Make two articles
models.Article.objects.create()
article = models.Article.objects.create()

# Make a publication for the second article
publication = article.publications.create()

# Make a section for the article
section = models.Section.objects.create(article=article)

# Make a sentence for the section
models.Sentence.objects.create(section=section)

# Get all sentences that aren't in the publication
sentences_not_in_pub = models.Sentence.objects.exclude(
section__article__publications=publication)

# There should be none
self.assertEquals(len(sentences_not_in_pub), 0)

Change History (3)

comment:1 Changed 5 years ago by Anssi Kääriäinen

This one seems to have been fixed by the changes in master. Using the above models & test query (with the difference that Article is renamed to Article2 due to naming conflict) the test case passes. The generated query is:

SELECT "queries_sentence"."id", "queries_sentence"."section_id"
  FROM "queries_sentence"
 INNER JOIN "queries_section" ON ( "queries_sentence"."section_id" = "queries_section"."id" )
 WHERE NOT ("queries_section"."article_id" IN (
        SELECT U3."article2_id"
          FROM "queries_article2_publications" U3
         WHERE U3."publication_id" = 1 ))

To me the query seems correct.

I will try to work the test case to use existing queries tests models. If that is not possible I will just use the original test case. So, a patch "fixing" this by adding a test case coming hopefully soon.

Unfortunately backpatch to 1.5 wont happen as this isn't a critical issue (and, backpatching would be really hard in any case due to massive amount of changes to the ORM).

comment:2 Changed 5 years ago by Anssi Kääriäinen

Triage Stage: UnreviewedAccepted

comment:3 Changed 5 years ago by Anssi Kääriäinen <akaariai@…>

Resolution: fixed
Status: newclosed

In c00a8525c6a003b57afbe6cdef243ffad397dcbe:

Fixed #20788 -- exclude() generated subquery failure

"Fixed" by adding a test case, the original problem was already fixed
by earlier ORM changes. Thanks to david@… for the report
and test case.

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