Opened 5 years ago

Closed 18 months ago

Last modified 18 months ago

#15145 closed Bug (fixed)

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

Reported by: melinath Owned by: nobody
Component: Database layer (models, ORM) Version: master
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 GDorn 5 years ago.
illustrative patch, no a solution

Download all attachments as: .zip

Change History (11)

comment:1 Changed 5 years ago by russellm

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 5 years ago by GDorn

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

Last edited 5 years ago by GDorn (previous) (diff)

Changed 5 years ago by GDorn

illustrative patch, no a solution

comment:3 Changed 5 years ago by GDorn

  • Patch needs improvement set

comment:4 Changed 4 years ago by bberes

  • Cc botondus@… added

comment:5 Changed 4 years ago by lrekucki

  • Severity set to Normal
  • Type set to Bug

comment:6 Changed 4 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:7 Changed 4 years ago by aaugustin

  • Easy pickings unset

Change Easy pickings from NULL to False.

comment:8 Changed 19 months ago by reidrac

I can't reproduce the issue in master.

comment:9 Changed 18 months ago by melinath

  • Resolution set to fixed
  • Status changed from new to closed

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

comment:10 Changed 18 months ago by bmispelon

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

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