#30209: Union with group by don't generate correct Subquery
-------------------------------------+-------------------------------------
     Reporter:  Nikolas              |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  Database layer       |                  Version:  master
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:  union, group by      |             Triage Stage:  Accepted
    Has patch:  0                    |      Needs documentation:  1
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by TKdka):

 Replying to [comment:8 Simon Charette]:
 > > I'm not sure what exception class should be used here. TypeError?
 Thoughts?
 >
 > Good question. I know that
 [https://docs.djangoproject.com/en/2.1/ref/models/querysets/#union we're
 currently preventing some operations on combined querysets] so I'd try to
 see what type of exception we're currently raising and stick to it to be
 coherent. I think `.filter()` is disallowed for example. It's possible
 that we simply crash though.
 >
 > I think the best place to add this check would be in
 `query.Queryset.annotate` just before `set_group_by`
 
[https://github.com/django/django/blob/418263c457636d3301f2068c47f09a0f42e15c52/django/db/models/query.py#L1052
 is called] if an annotation contains aggregates.

 On where the exception should be raised...
 I agree with general placement, but I'm thinking the exception should be
 raised as soon as an aggregate function as noticed, so just after
 
[https://github.com/django/django/blob/418263c457636d3301f2068c47f09a0f42e15c52/django/db/models/query.py#L1048
 line 1048 is called]   The other part of the conditional,
 {{{
 clone.query.group_by = True
 }}}
 is problematic as well.  This appears to create a group by over all the
 fields, which is not supported.  Thoughts?


 On the type of exception that should be raised...
 Using union+filter does not raise an exception (nor does it work).  I
 think that the correct exception should be something along the lines of a
 SQLNotSupported exception.  The exception should state what SQL feature
 combination is not supported and then suggest using a raw query or
 rewriting the query.   Is there already something like this?


 On related bugs...
 I found two similar bugs while exploring this feature (one of which is
 mentioned above).  I'll create tickets for them as well, but wanted to
 mention them here too.

 As mentioned above, there is a bug with filter + union as well.
 Performing a filter with a union neither raises an exception nor adds the
 filter.
  {{{
 qs1 = Book.objects.filter(pk__lte=10)
 qs2 = Book.objects.filter(pk__gt=10)
 qs3 = qs1.union(qs2)
 qs4  = qs3.filter(publisher_id=1)
 print (qs4.query)
 }}}
 SELECT "aggregation_book"."id", "aggregation_book"."isbn",
 "aggregation_book"."name", "aggregation_book"."pages",
 "aggregation_book"."rating", "aggregation_book"."price",
 "aggregation_book"."contact_id", "aggregation_book"."publisher_id",
 "aggregation_book"."pubdate" FROM "aggregation_book" WHERE
 "aggregation_book"."id" <= 10 UNION SELECT "aggregation_book"."id",
 "aggregation_book"."isbn", "aggregation_book"."name",
 "aggregation_book"."pages", "aggregation_book"."rating",
 "aggregation_book"."price", "aggregation_book"."contact_id",
 "aggregation_book"."publisher_id", "aggregation_book"."pubdate" FROM
 "aggregation_book" WHERE "aggregation_book"."id" > 10

 There's a similar bug when using filter + non-aggregating annotate
 functions.  The annotated field is not added, and an exception is not
 raised.
 {{{
 qs1 = Book.objects.filter(pk__lte=10)
 qs2 = Book.objects.filter(pk__gt=10)
 qs3 = qs1.union(qs2)
 qs4  = qs3.annotate(annotateField=F('id')+1)
 print (qs4.query)
 }}}
 SELECT "aggregation_book"."id", "aggregation_book"."isbn",
 "aggregation_book"."name", "aggregation_book"."pages",
 "aggregation_book"."rating", "aggregation_book"."price",
 "aggregation_book"."contact_id", "aggregation_book"."publisher_id",
 "aggregation_book"."pubdate" FROM "aggregation_book" WHERE
 "aggregation_book"."id" <= 10 UNION SELECT "aggregation_book"."id",
 "aggregation_book"."isbn", "aggregation_book"."name",
 "aggregation_book"."pages", "aggregation_book"."rating",
 "aggregation_book"."price", "aggregation_book"."contact_id",
 "aggregation_book"."publisher_id", "aggregation_book"."pubdate" FROM
 "aggregation_book" WHERE "aggregation_book"."id" > 10

-- 
Ticket URL: <https://code.djangoproject.com/ticket/30209#comment:9>
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 django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/069.ce76b7f434d15f1eddf5007424d3903d%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to