Django 1.6RC1 exclude behavior change

[jga...@gmail.com]

I've found what looks like a serious behavior change in the exclude queryset method from Django 1.5.5 to Django 1.6 rc1.

It seems that on 1.5.5 exclude when traversing relationships only excluded items if all criteria on the kwargs were matched on the same related item. On 1.6rc1 it excludes items even if the criteria on the kwargs is only matched across multiple related items. I guess this explanation is not very clear, so here is a sample code that show the behavior change:

http://pastebin.kde.org/pe1vlzd3v

Since I didn't find anything on the change notes about this, it looks to me like a bug. Is it? Or am I missing something?

[Kääriäinen Anssi]

I'll look into this.

- Anssi
________________________________________
From: django-d...@googlegroups.com [django-d...@googlegroups.com] On Behalf Of jga...@gmail.com [jga...@gmail.com]
Sent: Monday, November 04, 2013 17:16
To: django-d...@googlegroups.com
Subject: Django 1.6RC1 exclude behavior change

--
You received this message because you are subscribed to the Google Groups "Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/8267eeb8-f1a7-46db-969e-79d819c8f797%40googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[Anssi Kääriäinen]

On Monday, November 4, 2013 6:06:55 PM UTC+2, Anssi Kääriäinen wrote:

I'll look into this.

The situation is that this query didn't work properly in 1.5.x, but this doesn't work properly in 1.6.x either.

The basic problem here is that in 1.5.x .exclude(Q(anything)) didn't work correctly. Using the example models, try these equivalent queries:
    bees1 = B.objects.exclude(Q(a__state=1))
    bees2 = B.objects.exclude(a__state=1)

bees1 produces querystr:
    SELECT "new_basic_b"."id" FROM "new_basic_b" INNER JOIN "new_basic_a" ON ("new_basic_b"."id" = "new_basic_a"."b_id") WHERE NOT ("new_basic_a"."state" = 1 )

while bees2 produces:
    SELECT "new_basic_b"."id" FROM "new_basic_b" WHERE NOT (("new_basic_b"."id" IN (SELECT U1."b_id" FROM "new_basic_a" U1 WHERE (U1."state" = 1  AND U1."b_id" IS NOT NULL)) AND "new_basic_b"."id" IS NOT NULL))

Note that bees2 has correctly subquery in it. The bees1 query will produce incorrect results.

Now, for the example query, the same "exclude hiding" happens for Q(a__confirmation=False) | Q(a__confirmation__isnull=True). Due to this the produced query is:
    SELECT "new_basic_b"."id" FROM "new_basic_b" LEFT OUTER JOIN "new_basic_a" ON ("new_basic_b"."id" = "new_basic_a"."b_id") WHERE NOT (("new_basic_a"."confirmation" = False  OR "new_basic_a"."confirmation" IS NULL) AND ("new_basic_b"."id" IN (SELECT U1."b_id" FROM "new_basic_a" U1 WHERE (U1."state" = 1  AND U1."b_id" IS NOT NULL)) AND "new_basic_b"."id" IS NOT NULL))

Note the LEFT OUTER JOIN for the ORed condition, but subquery for the state condition. That is incorrect, both filters should be in the same subquery.

In 1.6.x the situation is, well, different. The generated query is:
    SELECT "new_basic_b"."id" FROM "new_basic_b" WHERE NOT (("new_basic_b"."id" IN (SELECT U1."b_id" FROM "new_basic_a" U1 WHERE U1."confirmation" = False ) OR "new_basic_b"."id" IN (SELECT U0."id" FROM "new_basic_b" U0 LEFT OUTER JOIN "new_basic_a" U1 ON ( U0."id" = U1."b_id" ) WHERE U1."confirmation" IS NULL)) AND "new_basic_b"."id" IN (SELECT U1."b_id" FROM "new_basic_a" U1 WHERE U1."state" = 1 ))

Now we have each of the conditions correctly in a subquery, but in different subqueries which isn't correct (filters inside single .filter()/.exclude() should target the same row when having multiple clauses for the same multivalued relation).

Complex filters in .exclude() didn't work correctly, and do not work correctly in the upcoming 1.6 either. If the results were correct in 1.5 that was luck, not a result of Django generating the correct query. It should be possible to construct data that highlights the problem in the 1.5 version of the query.

 - Anssi

[jga...@gmail.com]

Anssi,

Thanks for helping.

I'm sorry to say that your answer went somewhat over my head, my proficiency with SQL is lacking.

What I understood from your explanation:

 - A filter/exclude that traverses a 1:N relationship(such as foreign key) should target the same row with all of its criteria(kwargs).

 - Complex queries don't work correctly in exclude when using relationships in 1.5.x

 - Complex queries don't work correctly in exclude when using relationships in 1.6.x

Did I understand correctly?

If that was the whole of the situation I would be ok, I can work around this issue with multiple exclude statements, such as:

bees = B.objects.exclude(a__confirmation=False, a__state=1)

bees = bees.exclude(a__confirmation__isnull=True, a__state=1)

That should be equivalent to what I was trying to do with:

confirm_q = Q(a__confirmation=False) | Q(a__confirmation__isnull=True)

bees = B.objects.exclude(confirm_q, a__state=1)

But my solution of splitting the Q into two queries didn't work, for either 1.5.5 or 1.6rc1.

Did I miss something?

Gastal

[jga...@gmail.com]

I managed to get the desired behavior by doing the following ugly query:

q_obj = (Q(a__confirmation=True) & Q(a__state=1)) | (Q(a__state__gt=1) & Q(a__state__lt=1))

bees = B.objects.filter(q_obj)

This is obviously not an ideal solution but is working for me so far...

[Anssi Kääriäinen]

On Monday, November 4, 2013 7:24:47 PM UTC+2, jga...@gmail.com wrote:

Anssi,

Thanks for helping.

I'm sorry to say that your answer went somewhat over my head, my proficiency with SQL is lacking.

What I understood from your explanation:

 - A filter/exclude that traverses a 1:N relationship(such as foreign key) should target the same row with all of its criteria(kwargs).

 - Complex queries don't work correctly in exclude when using relationships in 1.5.x

 - Complex queries don't work correctly in exclude when using relationships in 1.6.x

Did I understand correctly?

That is a good summary of the situation, except that the exclude bugs are about 1:N (or N:N) relationships, not just any relationship.

 

If that was the whole of the situation I would be ok, I can work around this issue with multiple exclude statements, such as:

bees = B.objects.exclude(a__confirmation=False, a__state=1)

bees = bees.exclude(a__confirmation__isnull=True, a__state=1)

That should be equivalent to what I was trying to do with:

confirm_q = Q(a__confirmation=False) | Q(a__confirmation__isnull=True)

bees = B.objects.exclude(confirm_q, a__state=1)

But my solution of splitting the Q into two queries didn't work, for either 1.5.5 or 1.6rc1.

Did I miss something?

I'd go for a solution where you do:
    a_qs = A.objects.filter(Q(confirmation__isnull=True) | ..., a__state = 1).values_list('b_id')
    bees = B.objects.exclude(pk__in=a_qs)
If I am not mistaken that is what Django should be doing automatically for you.

Seems like django-users is the right forum to continue this discussion unless there are more items that are about development of Django itself.

 - Anssi

[jga...@gmail.com]

I was going to file a ticket in trac about this and found this one(https://code.djangoproject.com/ticket/21192) which seems related. The thing is that one was supposedly resolved 5 weeks ago, which would mean that fix would be in 1.6rc1...

Should I reopen that ticket or file a new one?

[Anssi Kääriäinen]

On Monday, November 4, 2013 8:16:12 PM UTC+2, jga...@gmail.com wrote:

I was going to file a ticket in trac about this and found this one(https://code.djangoproject.com/ticket/21192) which seems related. The thing is that one was supposedly resolved 5 weeks ago, which would mean that fix would be in 1.6rc1...

Should I reopen that ticket or file a new one?

The issue about multiple filters for same relation in single .exclude() query is tracked in #14645.

Just for the record: I don't consider the change in the original report's query a release blocker (or a bug at all). The query works differently in 1.6.x, but as it didn't work correctly in 1.5.x either there is nothing to do.

 - Anssi

[Elyézer Rezende]

Maybe it could be add to a "Known Bugs" or something like that section in the release notes? 

--

You received this message because you are subscribed to the Google Groups "Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/a901cb40-b80d-487e-b751-147471c94900%40googlegroups.com.

For more options, visit https://groups.google.com/groups/opt_out.

--
Elyézer Rezende
http://elyezer.com

[jonas gastal]

I wouldn't ask for a bug that has long existed to be considered a
release blocker. However a behavior change with no documentation is
not a nice thing to do to your users, it seems to me Elyézer makes a
good suggestion, informing users of the known bug in the release notes
seems quite reasonable.

Gastal

> You received this message because you are subscribed to a topic in the

> Google Groups "Django developers" group.

> To unsubscribe from this topic, visit
> https://groups.google.com/d/topic/django-developers/wQYmvtrTqIs/unsubscribe.
> To unsubscribe from this group and all its topics, send an email to

> django-develop...@googlegroups.com.
> To post to this group, send email to django-d...@googlegroups.com.
> Visit this group at http://groups.google.com/group/django-developers.
> To view this discussion on the web visit

> https://groups.google.com/d/msgid/django-developers/CAFsoaZs2VWEzGgQ2GhvZbnprWJQrYy6bwWK-fmESZHEzaTCCPQ%40mail.gmail.com.

[Anssi Kääriäinen]

Adding something about this to release notes shouldn't hurt anybody.

I will aim for a more generic wording about changes in the ORM. For
example, there are likely cases where the generated join aliases of the
query aren't the same as they were in 1.5. This could affect .extra()
users for example. Then there are all those changes that affect ORM
internal APIs, and there have been a lot of those changes.

- Anssi

[Anssi Kääriäinen]

On Tuesday, November 5, 2013 1:02:41 PM UTC+2, Anssi Kääriäinen wrote:

Adding something about this to release notes shouldn't hurt anybody.

As usual, I am a bit late. Luckily release notes can be changed after release.

Proposed changes here:  https://github.com/django/django/pull/1887

 - Anssi

[Elyézer Rezende]

+1

This gives an overview that what behaviours users could find when dealing with some use cases of the ORM.

--

You received this message because you are subscribed to the Google Groups "Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/5a3a812e-9620-41ba-bd97-6b85baa253ef%40googlegroups.com.

For more options, visit https://groups.google.com/groups/opt_out.

[jonas gastal]

LGTM

> You received this message because you are subscribed to a topic in the

> Google Groups "Django developers" group.

> To unsubscribe from this topic, visit
> https://groups.google.com/d/topic/django-developers/wQYmvtrTqIs/unsubscribe.
> To unsubscribe from this group and all its topics, send an email to

> django-develop...@googlegroups.com.
> To post to this group, send email to django-d...@googlegroups.com.
> Visit this group at http://groups.google.com/group/django-developers.
> To view this discussion on the web visit

> https://groups.google.com/d/msgid/django-developers/CAFsoaZse%2BrnEmD2Fb1Lkn3MNtkL8g%2BBD5kZOUY0%2ByWD1A4FG-g%40mail.gmail.com.