[Django] #28848: nulls_first on SQLite can not be used on the result of a filtered subquery
Django
-------------------------------------+-------------------------------------
Reporter: Raphael | Owner: nobody
Michel |
Type: Bug | Status: new
Component: Database | Version: 2.0
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
`NULLS FIRST` is not natively supported on some database backends, e.g.
SQLite, but Django supports it everywhere through
`F(…).asc(nulls_first=True)`. For those database backends, Django
simulates it by passing two expressions in the ORDER BY to the database,
the first of which only sorts away all ``NULL`` values.
If you try to use this to sort on an expression that is calculated using a
subquery, this still works fine, as long as the sub query does not contain
any literal values that are passed to the database as query parameters. In
this case, the SQL compiler does not take into account that due to the
duplicate expression in the ORDER BY clause, it needs to send the
parameter more often.
I found the bug in a real-world project using Django 1.11.7, where the
queryset apparently is just empty in this case. In current master, the bug
still exists but Django at least throws a meaningful exception in this
case.
I wrote a regression test to reproduce it. You can just drop the following
test into `tests/ordering/tests.py`:
{{{
def test_orders_nulls_first_on_filtered_subquery(self):
Article.objects.filter(headline="Article
1").update(author=self.author_1)
Article.objects.filter(headline="Article
2").update(author=self.author_1)
Article.objects.filter(headline="Article
4").update(author=self.author_2)
Author.objects.filter(name__isnull=True).delete()
author_3 = Author.objects.create(name="Name 3")
article_subquery = Article.objects.filter(
author=OuterRef('pk'),
headline__icontains="Article"
).order_by().values('author').annotate(
last_date=Max('pub_date')
).values('last_date')
# Works, but might not be consistent between database backends and
does not allow me
# to switch to nulls_last
self.assertQuerysetEqualReversible(
Author.objects.annotate(
last_date=Subquery(article_subquery,
output_field=DateTimeField())
).order_by(
'last_date'
),
[author_3, self.author_1, self.author_2],
)
# Works fine, but is not easy to reverse by hand and requires all
real values
# to be in a certain range
self.assertQuerysetEqualReversible(
Author.objects.annotate(
last_date=Subquery(article_subquery,
output_field=DateTimeField())
).order_by(
Coalesce('last_date', datetime(1970, 1, 1))
),
[author_3, self.author_1, self.author_2],
)
# "Correct" solution, but raises
# django.db.utils.ProgrammingError: Incorrect number of bindings
supplied. The current statement uses 5, and there are 3 supplied.
# on SQLite.
self.assertQuerysetEqualReversible(
Author.objects.annotate(
last_date=Subquery(article_subquery,
output_field=DateTimeField())
).order_by(
F('last_date').asc(nulls_first=True)
).distinct(),
[author_3, self.author_1, self.author_2],
)
}}}
The test output is:
{{{
======================================================================
ERROR: test_orders_nulls_first_on_filtered_subquery
(ordering.tests.OrderingTests)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/raphael/proj/django/django/db/backends/utils.py", line 85,
in _execute
return self.cursor.execute(sql, params)
File "/home/raphael/proj/django/django/db/backends/sqlite3/base.py",
line 301, in execute
return Database.Cursor.execute(self, query, params)
sqlite3.ProgrammingError: Incorrect number of bindings supplied. The
current statement uses 5, and there are 3 supplied.
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "/home/raphael/proj/django/tests/ordering/tests.py", line 466, in
test_orders_nulls_first_on_filtered_subquery
[author_3, self.author_1, self.author_2],
File "/home/raphael/proj/django/tests/ordering/tests.py", line 96, in
assertQuerysetEqualReversible
self.assertSequenceEqual(queryset, sequence)
File "/usr/lib/python3.6/unittest/case.py", line 931, in
assertSequenceEqual
len1 = len(seq1)
File "/home/raphael/proj/django/django/db/models/query.py", line 255, in
__len__
self._fetch_all()
File "/home/raphael/proj/django/django/db/models/query.py", line 1177,
in _fetch_all
self._result_cache = list(self._iterable_class(self))
File "/home/raphael/proj/django/django/db/models/query.py", line 55, in
__iter__
results = compiler.execute_sql(chunked_fetch=self.chunked_fetch,
chunk_size=self.chunk_size)
File "/home/raphael/proj/django/django/db/models/sql/compiler.py", line
1058, in execute_sql
cursor.execute(sql, params)
File "/home/raphael/proj/django/django/db/backends/utils.py", line 68,
in execute
return self._execute_with_wrappers(sql, params, many=False,
executor=self._execute)
File "/home/raphael/proj/django/django/db/backends/utils.py", line 77,
in _execute_with_wrappers
return executor(sql, params, many, context)
File "/home/raphael/proj/django/django/db/backends/utils.py", line 85,
in _execute
return self.cursor.execute(sql, params)
File "/home/raphael/proj/django/django/db/utils.py", line 89, in
__exit__
raise dj_exc_value.with_traceback(traceback) from exc_value
File "/home/raphael/proj/django/django/db/backends/utils.py", line 85,
in _execute
return self.cursor.execute(sql, params)
File "/home/raphael/proj/django/django/db/backends/sqlite3/base.py",
line 301, in execute
return Database.Cursor.execute(self, query, params)
django.db.utils.ProgrammingError: Incorrect number of bindings supplied.
The current statement uses 5, and there are 3 supplied.
}}}
Note that the test passes when you omit the
`headline__icontains="Article"`.
--
Ticket URL: <https://code.djangoproject.com/ticket/28848>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
| Michel
Type: Bug | Status: assigned
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => Raphael Michel
* status: new => assigned
* has_patch: 0 => 1
Comment:
It is possible to solve the problem with an one-line patch. I created a
pull request for it: https://github.com/django/django/pull/9388/files
There might be more elegant solutions or solutions that solve the problem
more deeply because it might exist in other places as well, but I did not
come up with one in the available time.
It would be great to see this get into 2.0 or even a 1.11.8 if there will
be one!
--
Ticket URL: <https://code.djangoproject.com/ticket/28848#comment:1>
Django
-------------------------------------+-------------------------------------
| Michel
Type: Bug | Status: assigned
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* version: 2.0 => 1.11
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
Comment:
This should qualify for a backport to 1.11.8 as this a bug in newly
introduced features (`nulls_first` and `Subquery`).
--
Ticket URL: <https://code.djangoproject.com/ticket/28848#comment:2>
Django
-------------------------------------+-------------------------------------
| Michel
Component: Database layer | Version: 1.11
(models, ORM) |
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"616f468760e4984915bb2ccca6b9eb3d80ddadb0" 616f4687]:
{{{
#!CommitTicketReference repository=""
revision="616f468760e4984915bb2ccca6b9eb3d80ddadb0"
Fixed #28848 -- Fixed SQLite/MySQL crash when ordering by a filtered
subquery that uses nulls_first/nulls_last.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28848#comment:3>
Django
-------------------------------------+-------------------------------------
| Michel
Type: Bug | Status: closed
Component: Database layer | Version: 1.11
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"75c1fd653858997449f7ac47a3f3379723bbfc73" 75c1fd6]:
{{{
#!CommitTicketReference repository=""
revision="75c1fd653858997449f7ac47a3f3379723bbfc73"
[2.0.x] Fixed #28848 -- Fixed SQLite/MySQL crash when ordering by a
filtered subquery that uses nulls_first/nulls_last.
Backport of 616f468760e4984915bb2ccca6b9eb3d80ddadb0 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28848#comment:4>
Django
-------------------------------------+-------------------------------------
| Michel
Type: Bug | Status: closed
Component: Database layer | Version: 1.11
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"899999db4293d40613626833860de28e8ccdd413" 899999db]:
{{{
#!CommitTicketReference repository=""
revision="899999db4293d40613626833860de28e8ccdd413"
[1.11.x] Fixed #28848 -- Fixed SQLite/MySQL crash when ordering by a
filtered subquery that uses nulls_first/nulls_last.
Backport of 616f468760e4984915bb2ccca6b9eb3d80ddadb0 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28848#comment:5>