Opened 2 years ago
Last modified 2 years ago
#33768 closed Bug
MySQL ordering of nulls last/first is broken in combination with UNION — at Initial Version
Reported by: | Florian Apolloner | Owned by: | nobody |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Adam Johnson, Simon Charette | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
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 :)