Opened 13 years ago

Closed 10 years ago

Last modified 10 years ago

#15145 closed Bug (fixed)

__in is ignored by an excluded query if foo__in is set to an empty iterable

Reported by: Stephen Burrows Owned by: nobody
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords:
Cc: stephen.r.burrows@…, botondus@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Please note that the arguments may be ignored by a filtered query. I haven't tested that.

Assume the following model setup:

from django.db import models
from django.contrib.contenttypes.models import ContentType


class TestModel1(models.Model):
        content_type = models.ForeignKey(ContentType)
        integer = models.PositiveIntegerField()

Now run the following code:

>>> from test.models import TestModel1 # or wherever you're keeping it.
>>> from django.contrib.contenttypes.models import ContentType
>>> integer = 1
>>> ct = ContentType.objects.all()[0]
>>> TestModel1.objects.create(integer=integer, content_type=ct)
<TestModel1: TestModel1 object>
>>> TestModel1.objects.all()
[<TestModel1: TestModel1 object>]
>>> # This is where it starts getting interesting.
>>> TestModel1.objects.exclude(content_type=ct, integer__in=[])
[]

According to the documentation, this kind of exclude should exclude all rows where the content type is ct AND integer is in the list. Now since the list is empty, there should be no rows matching the exclusion, so all rows should be returned. Instead, I get an empty queryset.

Here's a look at the sql being generated by various queries (Line breaks added for readability):


TestModel1.objects.all() or TestModel1.objects.exclude(integer__in=[]) or TestModel1.objects.exclude(content_type__in=[])

SELECT "test_testmodel1"."id", "test_testmodel1"."content_type_id", "test_testmodel1"."integer" FROM "test_testmodel1"

TestModel1.objects.exclude(integer__in=[2])

SELECT "test_testmodel1"."id", "test_testmodel1"."content_type_id", "test_testmodel1"."integer"
FROM "test_testmodel1" WHERE NOT ("test_testmodel1"."integer" IN (2))

TestModel1.objects.exclude(integer__in=[], content_type=ct) or TestModel1.objects.exclude(content_type=ct)

SELECT "test_testmodel1"."id", "test_testmodel1"."content_type_id", "test_testmodel1"."integer"
FROM "test_testmodel1" WHERE NOT ("test_testmodel1"."content_type_id" = 1 )

---

TestModel1.objects.exclude(content_type__in=[], integer=1) or TestModel1.objects.exclude(integer=1)

SELECT "test_testmodel1"."id", "test_testmodel1"."content_type_id", "test_testmodel1"."integer"
FROM "test_testmodel1" WHERE NOT ("test_testmodel1"."integer" = 1 )

---

As you can see, the __in kwarg is being completely ignored. Unfortunately, I can't for the life of me figure out where the bug is happening, or I would try to write a patch. In case it's relevant, I'm using a sqlite3 database.

Attachments (1)

15145_lame_solution.diff (533 bytes ) - added by Sam Thompson 13 years ago.
illustrative patch, no a solution

Download all attachments as: .zip

Change History (11)

comment:1 by Russell Keith-Magee, 13 years ago

Triage Stage: UnreviewedAccepted

comment:2 by Sam Thompson, 13 years ago

Some initial investigation:

One possible source of the problem is in WhereNode.make_atom(). When called on an in lookup with an empty list, it raises an EmptyResultSet which drops it from the query entirely. It should first check to see if it is part of an exclude, but I'm not sure that information is available at that point.

This should either be a wontfix (check your list before excluding on it), or a not-small refactor of the make_atom/as_sql portion of WhereNode to pass state into inner nodes of the tree.

I'll upload a non-solution patch to illustrate the problem. This breaks one other test, of course: aggregation_regress.AggregationTests.test_empty_filter_aggregate

Version 0, edited 13 years ago by Sam Thompson (next)

by Sam Thompson, 13 years ago

Attachment: 15145_lame_solution.diff added

illustrative patch, no a solution

comment:3 by Sam Thompson, 13 years ago

Patch needs improvement: set

comment:4 by Béres Botond, 13 years ago

Cc: botondus@… added

comment:5 by Łukasz Rekucki, 13 years ago

Severity: Normal
Type: Bug

comment:6 by Aymeric Augustin, 12 years ago

UI/UX: unset

Change UI/UX from NULL to False.

comment:7 by Aymeric Augustin, 12 years ago

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:8 by Juan J. Martinez, 10 years ago

I can't reproduce the issue in master.

comment:9 by Stephen Burrows, 10 years ago

Resolution: fixed
Status: newclosed

Yeah, seems to be fixed in master. (Going by the test case I presented.)

comment:10 by Baptiste Mispelon, 10 years ago

Looks like it was fixed in bd283aa844b04651b7c8b4e85f48c6dced1678f0 (found using git bisect).

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