nulls_first on SQLite can not be used on the result of a filtered subquery

[Raphael Michel]

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".

[Raphael Michel]

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!

[Simon Charette]

This should qualify for a backport to 1.11.8 as this a bug in newly introduced features (nulls_first and Subquery).

[Tim Graham <timograham@…>]

In 616f4687:

Fixed #28848 -- Fixed SQLite/MySQL crash when ordering by a filtered subquery that uses nulls_first/nulls_last.

[Tim Graham <timograham@…>]

In 75c1fd6:

[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

[Tim Graham <timograham@…>]

In 899999db:

[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