#33768: MySQL ordering of nulls last/first is broken in combination with UNION
-------------------------------------+-------------------------------------
Reporter: Florian Apolloner | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Description changed by Florian Apolloner:
Old description:
> putting the following into test_qs_combinators.py:
> {{{
> def test_ordering_with_null_first(self):
> Number.objects.filter(other_num=5).update(other_num=None)
> qs = (
> Number.objects.filter(num__lte=1)
> .union(Number.objects.filter(num__gte=2))
> .order_by(F("other_num").asc(nulls_first=True))
> .values_list("other_num", flat=True)
> )
> self.assertQuerysetEqual(qs, [None, 1, 2, 3, 4, 6, 7, 8, 9, 10])
> qs = (
> Number.objects.filter(num__lte=1)
> .union(Number.objects.filter(num__gte=2))
> .order_by(F("other_num").asc(nulls_last=True))
> .values_list("other_num", flat=True)
> )
> self.assertQuerysetEqual(qs, [1, 2, 3, 4, 6, 7, 8, 9, 10, None])
> }}}
>
> results in:
>
> {{{
> ======================================================================
> FAIL: test_ordering_with_null_first
> (queries.test_qs_combinators.QuerySetSetOperationTests)
> ----------------------------------------------------------------------
> Traceback (most recent call last):
> File
> "/home/florian/sources/django.git/tests/queries/test_qs_combinators.py",
> line 171, in test_ordering_with_null_first
> self.assertQuerysetEqual(qs, [1, 2, 3, 4, 6, 7, 8, 9, 10, None])
> File "/home/florian/sources/django.git/django/test/testcases.py", line
> 1328, in assertQuerysetEqual
> return self.assertEqual(list(items), values, msg=msg)
> AssertionError: Lists differ: [None, 1, 2, 3, 4, 6, 7, 8, 9, 10] != [1,
> 2, 3, 4, 6, 7, 8, 9, 10, None]
>
> First differing element 0:
> None
> 1
>
> - [None, 1, 2, 3, 4, 6, 7, 8, 9, 10]
> + [1, 2, 3, 4, 6, 7, 8, 9, 10, None]
>
> ----------------------------------------------------------------------
> (0.000) UPDATE `queries_number` SET `other_num` = NULL WHERE
> `queries_number`.`other_num` = 5; args=(5,); alias=default
> (0.000) (SELECT `queries_number`.`other_num` FROM `queries_number` WHERE
> `queries_number`.`num` <= 1) UNION (SELECT `queries_number`.`other_num`
> FROM `queries_number` WHERE `queries_number`.`num` >= 2) ORDER BY (1)
> ASC; args=(1, 2); alias=default
> (0.000) (SELECT `queries_number`.`other_num` FROM `queries_number` WHERE
> `queries_number`.`num` <= 1) UNION (SELECT `queries_number`.`other_num`
> FROM `queries_number` WHERE `queries_number`.`num` >= 2) ORDER BY (1) IS
> NULL, (1) ASC; args=(1, 2); alias=default
>
> ----------------------------------------------------------------------
> }}}
>
> Due to the `UNION` we are rewriting queries to use column numbers so we
> can somewhat reliably target the columns. This breaks down when
> `nulls_first/last` is used because `(1) IS NULL` is interpreted as
> expression in mysql and not as the colname 1 being NULL. Which brings me
> back to https://twitter.com/fapolloner/status/1533512493208936450 -- I
> think that the rewriting of thise expression should happen in the
> compiler and not in `OrderBy` itself. This way we could duplicate the
> `OrderBy` into two and push them into the relevant select clauses like we
> already do in
> https://github.com/django/django/blob/49b470b9187b6be60e573fed08c8f4a87f133750/django/db/models/sql/compiler.py#L410
> for other things.
>
> Sadly it is a mess… Thoughts? Paging Adam since he was the resident MySQL
> specialist IIRC :)
New description:
putting the following into test_qs_combinators.py:
{{{
def test_ordering_with_null_first(self):
Number.objects.filter(other_num=5).update(other_num=None)
qs = (
Number.objects.filter(num__lte=1)
.union(Number.objects.filter(num__gte=2))
.order_by(F("other_num").asc(nulls_first=True))
.values_list("other_num", flat=True)
)
self.assertQuerysetEqual(qs, [None, 1, 2, 3, 4, 6, 7, 8, 9, 10])
qs = (
Number.objects.filter(num__lte=1)
.union(Number.objects.filter(num__gte=2))
.order_by(F("other_num").asc(nulls_last=True))
.values_list("other_num", flat=True)
)
self.assertQuerysetEqual(qs, [1, 2, 3, 4, 6, 7, 8, 9, 10, None])
}}}
results in:
{{{
======================================================================
FAIL: test_ordering_with_null_first
(queries.test_qs_combinators.QuerySetSetOperationTests)
----------------------------------------------------------------------
Traceback (most recent call last):
File
"/home/florian/sources/django.git/tests/queries/test_qs_combinators.py",
line 171, in test_ordering_with_null_first
self.assertQuerysetEqual(qs, [1, 2, 3, 4, 6, 7, 8, 9, 10, None])
File "/home/florian/sources/django.git/django/test/testcases.py", line
1328, in assertQuerysetEqual
return self.assertEqual(list(items), values, msg=msg)
AssertionError: Lists differ: [None, 1, 2, 3, 4, 6, 7, 8, 9, 10] != [1, 2,
3, 4, 6, 7, 8, 9, 10, None]
First differing element 0:
None
1
- [None, 1, 2, 3, 4, 6, 7, 8, 9, 10]
+ [1, 2, 3, 4, 6, 7, 8, 9, 10, None]
----------------------------------------------------------------------
(0.000) UPDATE `queries_number` SET `other_num` = NULL WHERE
`queries_number`.`other_num` = 5; args=(5,); alias=default
(0.000) (SELECT `queries_number`.`other_num` FROM `queries_number` WHERE
`queries_number`.`num` <= 1) UNION (SELECT `queries_number`.`other_num`
FROM `queries_number` WHERE `queries_number`.`num` >= 2) ORDER BY (1) ASC;
args=(1, 2); alias=default
(0.000) (SELECT `queries_number`.`other_num` FROM `queries_number` WHERE
`queries_number`.`num` <= 1) UNION (SELECT `queries_number`.`other_num`
FROM `queries_number` WHERE `queries_number`.`num` >= 2) ORDER BY (1) IS
NULL, (1) ASC; args=(1, 2); alias=default
----------------------------------------------------------------------
}}}
Due to the `UNION` we are rewriting queries to use column numbers so we
can somewhat reliably target the columns. This breaks down when
`nulls_first/last` is used because `(1) IS NULL` is interpreted as
expression in mysql and not as the colname 1 being NULL. Which brings me
back to https://twitter.com/fapolloner/status/1533512493208936450 -- I
think that the rewriting of this expression should happen in the compiler
and not in `OrderBy` itself. This way we could duplicate the `OrderBy`
into two and push them into the relevant select clauses like we already do
in
https://github.com/django/django/blob/49b470b9187b6be60e573fed08c8f4a87f133750/django/db/models/sql/compiler.py#L410
for other things.
Sadly it is a mess… Thoughts? Paging Adam since he was the resident MySQL
specialist IIRC :)
--
--
Ticket URL: <https://code.djangoproject.com/ticket/33768#comment:1>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/0107018135681f3c-7cfdff03-bb80-4994-82e1-b3fa15bebdce-000000%40eu-central-1.amazonses.com.
