[Django] #20024: 'exclude' does not work with lists containing a 'None' element.
Django
----------------------------------------------+--------------------
Reporter: stillwater.ke@… | Owner: nobody
Type: Uncategorized | Status: new
Component: Database layer (models, ORM) | Version: 1.4
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
For example,
Entry.objects.exclude(foo_ _in=[None, 1])
It is supposed to return all items whose foo field is not None or 1, but
it actually returns an empty query set.
(Note there is no space between underlines. The layout looks weird when
two underline characters are put together.)
--
Ticket URL: <https://code.djangoproject.com/ticket/20024>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: stillwater.ke@… | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.4
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by akaariai):
* needs_better_patch: => 0
* stage: Unreviewed => Accepted
* type: Uncategorized => Bug
* needs_tests: => 0
* needs_docs: => 0
Comment:
This is a known failure (tested in queries/tests.py,
test_col_not_in_list_containing_null()).
This is somewhat hard to fix as SQL's "somecol NOT IN lst_containing_null"
is weird - it will always return False (well, technically UNKNOWN but this
doesn't matter here), and this is what causes the bug.
It would be possible to check for presence of None in the given list. If
present, remove it and add in a proper IS NULL check instead. But then a
query like `.exclude(somecol__in=qs_containig_null)` would work
differently from `.exclude(somecol__in=list(qs_containig_null))`. I guess
that could be classified as known abstraction leak.
--
Ticket URL: <https://code.djangoproject.com/ticket/20024#comment:1>
Django
Reporter: stillwater.ke@… | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.4
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by aaugustin):
Let's not be too smart... NULL in SQL is nowhere near as consistent as
None in Python because of SQL's tri-valued boolean logic; we cannot hide
this.
--
Ticket URL: <https://code.djangoproject.com/ticket/20024#comment:2>
Django
Reporter: stillwater.ke@… | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.4
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Old description:
> For example,
> Entry.objects.exclude(foo_ _in=[None, 1])
> It is supposed to return all items whose foo field is not None or 1, but
> it actually returns an empty query set.
> (Note there is no space between underlines. The layout looks weird when
> two underline characters are put together.)
New description:
For example:
{{{
Entry.objects.exclude(foo__in=[None, 1])
}}}
It is supposed to return all items whose foo field is not None or 1, but
it actually returns an empty query set.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/20024#comment:3>
Django
Reporter: stillwater.ke@… | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.4
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
#13768 is a duplicate.
--
Ticket URL: <https://code.djangoproject.com/ticket/20024#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: stillwater.ke@… | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.4
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
--
Ticket URL: <https://code.djangoproject.com/ticket/20024#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: stillwater.ke@… | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.4
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carsten Fuchs):
* cc: Carsten Fuchs (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/20024#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: stillwater.ke@… | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.4
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Adam Sołtysik):
I think this should be fixed, this a seriously surprising flaw in the ORM.
I've proposed several possible fixes for this issue in #31883, I'll try to
summarize them here.
The simplest would be to just ignore `None`s in the list. This would be
consistent with how `filter` currently works. When dealing with querysets
(`__in=qs.values('col')`), it should be enough to add `WHERE col IS NOT
NULL` in the `SELECT` subquery.
A better solution, but a more breaking change, would be to add a proper
`IS [NOT] NULL` check, in both `exclude` and `filter` cases, if `None` is
in the list. This would allow writing `filter(col__in=[something, None])`
instead of `filter(Q(col=something) | Q(col=None))`. Some additional
checks for inner querysets would also be required, so that the behaviour
is consistent, or the queryset could be just converted to a list
beforehand.
--
Ticket URL: <https://code.djangoproject.com/ticket/20024#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: stillwater.ke@… | Owner: Adam
| Sołtysik
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Adam Sołtysik):
* owner: nobody => Adam Sołtysik
* status: new => assigned
* version: 1.4 => master
Comment:
I wanted to fix this, and I've found that it has already been partially
fixed in #31667. However, there are no tests for `exclude`, and the
behaviour is now inconsistent, since
`exclude(col__in=queryset.values('col2'))` still gives no results when
`col2` contains `NULL`. I'll try to provide a patch to clean this up.
--
Ticket URL: <https://code.djangoproject.com/ticket/20024#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: stillwater.ke@… | Owner: Adam
| Sołtysik
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Simon Charette):
* cc: Simon Charette (added)
Comment:
Thanks for surfacing this inconsistency Adam.
It looks like adding an `else` clause to deal with a ''query'' right hand
side value that has explicitly selected fields that can be null (primary
keys can't)
The `RelatedIn` lookup will also need to be adjusted
--
Ticket URL: <https://code.djangoproject.com/ticket/20024#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: stillwater.ke@… | Owner: Adam
| Sołtysik
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Adam Sołtysik):
* has_patch: 0 => 1
Comment:
I've created a
[https://github.com/adamsol/django/commit/f50e315ce315ead117bfe4335a9fda758e88ae6f
patch]. No PR yet, though, because one test fails:
`WindowFunctionTests.test_window_expression_within_subquery`. SQL doesn't
support filtering by window functions, so I guess the code should be
corrected to use another subquery (or maybe an `EXISTS` query).
Unfortunately I don't know how to write this without using QuerySet
utilities.
As for `RelatedIn`, the things are weird here. It seems this case is
handled differently and already works correctly, and worked even before
#31667 (`test_ticket20024_related_in`). It was probably fixed in #13815
long ago. There is `IS NOT NULL` in the subquery, actually there are two
next to each other. One is coming from `split_exclude()` (called in
`build_filter()`), but the other one -- I couldn't find. Also, what looks
like another thing to take a closer look at -- there is `NOT IN (None)`
instead of `NOT IN (NULL)` in the SQL for the first query in that test,
when the `rhs.discard(None)` line is removed.
--
Ticket URL: <https://code.djangoproject.com/ticket/20024#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: stillwater.ke@… | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Adam Sołtysik):
* owner: Adam Sołtysik => (none)
* status: assigned => new
Comment:
It would be best to make the behaviour of `In` lookup consistent with
`RelatedIn`. Sadly, I wasn't able to understand how the latter works. So
my patch currently crashes with window functions, and it also crashes with
`OuterRef` due to #31714. As much as I'd like to finish this, I don't have
enough time and knowledge to look into it further. If anyone comes up with
a solution, feel free to claim this issue.
--
Ticket URL: <https://code.djangoproject.com/ticket/20024#comment:11>
Django
-------------------------------------+-------------------------------------
Reporter: stillwater.ke@… | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by felixxm):
* has_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/20024#comment:12>
Django
-------------------------------------+-------------------------------------
Reporter: stillwater.ke@… | Owner: Simon
| Charette
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Simon Charette):
* owner: (none) => Simon Charette
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/20024#comment:13>
Django
-------------------------------------+-------------------------------------
Reporter: stillwater.ke@… | Owner: (none)
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Simon Charette):
* owner: Simon Charette => (none)
* status: assigned => new
Comment:
I fear I won't be able to work on this issue in the near future so I'm
deassigning myself.
Now that we filter out literal `None` values from `__in=[None]` lookup
(#31667) we do have a mismatch with `__in=nullable_expression` but at
least the behaviour is consistent with the
`nullable_field=F('nullable_field')` and `nullable_field=None` asymmetry
(#32043).
--
Ticket URL: <https://code.djangoproject.com/ticket/20024#comment:14>