Opened 10 years ago

Closed 10 years ago

Last modified 9 years ago

#23605 closed Bug (fixed)

ORM neglects to use aliases it has set up when certain multiple subqueries are used

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

Description (last modified by ris)

Using django git 9d7a4ea20510a2e35fdeac21d67f3f4c17634c25

Example models.py:

from django.db import models

class ModelA ( models.Model ):
	pass

class ModelB ( models.Model ):
	modela_fk = models.ForeignKey ( ModelA )
	modelc_fk = models.ForeignKey ( "ModelC" )
	
	field_b0 = models.IntegerField ( null = True )
	field_b1 = models.BooleanField ()

class ModelC ( models.Model ):
	field_c0 = models.FloatField ()

Given the following query (yes, totally redundant subclauses noted):

ModelA.objects.filter (
    Q ( pk__in = ModelA.objects.filter ( Q ( modelb__field_b0__gte = 1000000 / F ( "modelb__modelc_fk__field_c0" ) )
        & Q ( modelb__field_b1__exact = True )
        & ~Q ( modelb__pk__in = ModelB.objects.filter (
            ~(
                Q ( field_b1__exact = True )
                & Q ( field_b0__gte = 1000000 / F ( "modelc_fk__field_c0" ) )
             )
        ) )
    ).filter ( modelb__field_b1__exact = True )
) )

Used against postgres 9.1 generates the error:

ProgrammingError: invalid reference to FROM-clause entry for table "dummy_modelb"
LINE 1: ...") AND U0."field_b0" IS NOT NULL)) AND V1."id" = ("dummy_mod...
                                                             ^
HINT:  Perhaps you meant to reference the table alias "v1".

Change History (20)

comment:1 by Tim Graham, 10 years ago

Did it work in older versions of Django? If so, could you bisect to the commit in Django where things broke?

comment:2 by ris, 10 years ago

Description: modified (diff)

comment:3 by ris, 10 years ago

(added missing trailing bracket in query in description)

Interestingly, switch the order of the .filters in the outermost subquery, making it:

ModelA.objects.filter (
    Q ( pk__in = ModelA.objects.filter ( modelb__field_b1__exact = True ).filter (
        Q ( modelb__field_b0__gte = 1000000 / F ( "modelb__modelc_fk__field_c0" ) )
        & Q ( modelb__field_b1__exact = True )
        & ~Q ( modelb__pk__in = ModelB.objects.filter (
            ~(
                Q ( field_b1__exact = True )
                & Q ( field_b0__gte = 1000000 / F ( "modelc_fk__field_c0" ) )
             )
        ) )
    )
) )

and it works fine.

comment:4 by ris, 10 years ago

(haven't tried older versions of django yet)

comment:5 by ris, 10 years ago

For what it's worth, this works in django 1.4 (ancient I know, but the last version we have in production right now).

comment:6 by Anssi Kääriäinen, 10 years ago

Smells like relabeled_clone problem. I'll investigate.

comment:7 by ris, 10 years ago

From IRC:

<akaariai_> it is a bit hard to see if I am fixing the query to work as in PostgreSQL can parse the query, or actually produce correct results
<ris> akaariai_: well, the inner double-NOT'ed subquery is to test for a ModelA object _all of whose_ ModelB objects conform to certain criteria
<ris> those criteria being
<ris> Q ( modelb__field_b0__gte = 1000000 / F ( "modelb__modelc_fk__field_c0" ) ) & Q ( modelb__field_b1__exact = True )
<ris> akaariai_: so it's saying "ModelA instances who have a ModelB instance that complies to X but _not_ ModelA instances that have any ModelB instances that _dont_ comply to X" (X = Q ( modelb__field_b0__gte = 1000000 / F ( "modelb__modelc_fk__field_c0" ) ) & Q ( modelb__field_b1__exact = True ) here)
<ris> akaariai_: hence the odd double-negative-wrapped-subquery
<ris> the outermost subquery .filter ( modelb__field_b1__exact = True ) is exactly the sort of appendage I'd like to be able to remove if I were able to test for Q-equality
Version 0, edited 10 years ago by ris (next)

comment:8 by Anssi Kääriäinen, 10 years ago

The problem was that django.db.models.sql.Query didn't have relabeled_clone() method. See PR3323 for proposed fix.

comment:9 by Anssi Kääriäinen, 10 years ago

Triage Stage: UnreviewedAccepted

comment:10 by ris, 10 years ago

(hooray! thank you)

comment:11 by ris, 10 years ago

Hmm.. PR3323 doesn't fix this for me. Instead I get

ProgrammingError: missing FROM-clause entry for table "W1"
LINE 1: ...") AND U0."field_b0" IS NOT NULL)) AND V1."id" = ("W1"."id")...

Possibly related: another old bug of mine: #18726

(don't know whether this discussion should continue here or on the pull request)

comment:12 by Tim Graham, 10 years ago

Has patch: set
Patch needs improvement: set

Yes, the test fails on PostgreSQL.

comment:13 by Anssi Kääriäinen, 10 years ago

The problem is that W1 must not be quoted, but this doesn't work correctly in the patched version. The alias W1 comes from the outer query, so the inner query doesn't know it is an alias and hence it gets quoted. I'll check how ugly a fix for this will be.

comment:14 by Anssi Kääriäinen, 10 years ago

Patch needs improvement: unset

Another try.

comment:15 by ris, 10 years ago

Hmm latest version of PR3323 still fails buildbot test on postgres.

comment:16 by Tim Graham, 10 years ago

Patch needs improvement: set

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

Resolution: fixed
Status: newclosed

In 5c481db29572a387651681b43d5d4523f96b3793:

Fixed #23605 -- Fixed nested subquery regression

Added relabeled_clone() method to sql.Query to fix the problem. It
manifested itself in rare cases where at least double nested subquery's
filter condition might target non-existing alias.

Thanks to Trac alias ris for reporting the problem.

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

In 01f2cf2aecc932d43b20b55fc19a8fa440457b5f:

[1.7.x] Fixed #23605 -- Fixed nested subquery regression

Added relabeled_clone() method to sql.Query to fix the problem. It
manifested itself in rare cases where at least double nested subquery's
filter condition might target non-existing alias.

Thanks to Trac alias ris for reporting the problem.

Backport of 5c481db29572a387651681b43d5d4523f96b3793 from master

comment:19 by ris, 9 years ago

Hmm. That's interesting. Just upgraded to 1.7.2 and the test case posted here is indeed fixed. However it doesn't fix our specific test case that this test was distilled from.

So that means somewhere in the distillation from our test case to this abstract one there is another bug lurking.

Should probably open it as another bug if I can pin this one down to another failing test case.

in reply to:  19 comment:20 by ris, 9 years ago

Ok, I've created ticket #24090 covering the new variant of this bug.

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