QuerySet.union no longer supports empty args

[Antoine Humeau]

Hello there,

Up until Django 5.1 (included), it was possible to use the union queryset method with an empty arg list, example:

User.objects.union()

This example is obviously useless but the following pattern would be equivalent and would be more reallistic:

def unite_querysets(querysets):
    queryset, *other_querysets = querysets
    queryset = queryset.union(*other_querysets)

Since Django 5.2, that triggers the following exception:

AttributeError: 'NoneType' object has no attribute 'elide_empty'

The line that triggers the exception is the following: ​https://github.com/django/django/blob/5.2.1/django/db/models/sql/compiler.py#L616
empty_compiler is never initialized.

I am not sure this really qualifies as a bug, it might be a unsupported usage, still it is a regression.

[Sarah Boyce]

git bisect to 9cb8baa0c4fa2c10789c5c8b65f4465932d4d172

Possible regression test

tests/queries/test_qs_combinators.py

@@ -88 +88 @@
         qs3 = qs1.union(qs2)
         self.assertNumbersEqual(qs3[:1], [0])
 
+    def test_empty_union_slice(self):
+        qs = Number.objects.union()
+        self.assertNumbersEqual(qs[:1], [0])
+
     def test_union_all_none_slice(self):
         qs = Number.objects.filter(id__in=[])
         with self.assertNumQueries(0):

However, I agree that this usage isn't supported. Perhaps a breaking change note is enough here? We might want a nicer error also

[Sarah Boyce]

Marking as a release blocker but willing to downgrade

[Antoine Humeau]

Unsupported usage with an explicit exception seems sensible to me.

[Simon Charette]

Thanks for the report and triage folks. I think it's worth fixing by having QuerySet.union return self if not other_qs given it's a non-invasive patch and dealing with empty sequence in a different manner would be annoyance.

[Simon Charette]

Left some comments on the PR, the proposed approach generates an unnecessary UNION against itself that causes the improper results to be returned when all is used as they are not deduped.

[Colleen Dunlap]

Hi Simon! I responded. It's already returning self and it's still breaking. Also do I make my PR pointing to the 5.2 branch or main?

[Simon Charette]

It should point at main which your new PR does, thank you for that.

Please have a look at ​this comment. The fact the tests are passing is not a good indicative that the right thing is happening here as the adjusted test I provided demonstrates (it does a UNION against itself which is not safe when all is used).

I still believe that the right way of fixing this is to adjust QuerySet.union to return self when not other_qs as the compilation layer is not equipped to deal with an empty combination of queries and it's just faster to avoid the query cloning if we can.

[Colleen Dunlap]

is that not what this is doing already? ​https://github.com/django/django/blob/0b2ed4f7c8396c8d9aa8428a40e6b25c31312889/django/db/models/query.py#L1548

[Colleen Dunlap]

ok nevermind I'm rereading this and it looks like I may have misinterpreted...

[Simon Charette]

No, the branch you linked is gated by a if isinstance(self, EmptyQuerySet) and qs is a list of all non-empty querysets. Django is able to detect when a queryset cannot match any rows (the most common example is filter(pk=[])) and the logic you linked deals with this case.

What I mean is that if the *other_qs args capture is empty then you can immediately return as some_set OR *other_sets means some_set.

[Colleen Dunlap]

Got it! I updated accordingly

[Sarah Boyce <42296566+sarahboyce@…>]

In 802baf5:

Fixed #36388 -- Made QuerySet.union() return self when called with no arguments.

Regression in 9cb8baa0c4fa2c10789c5c8b65f4465932d4d172.
Thank you to Antoine Humeau for the report and Simon Charette for the review.

[Sarah Boyce <42296566+sarahboyce@…>]

In 787f3130:

[5.2.x] Fixed #36388 -- Made QuerySet.union() return self when called with no arguments.

Regression in 9cb8baa0c4fa2c10789c5c8b65f4465932d4d172.
Thank you to Antoine Humeau for the report and Simon Charette for the review.

Backport of 802baf5da5b8d8b44990a8214a43b951e7ab8b39 from main.