Re: [Django] #33655: Unnecessary column in a GROUP BY clause with QuerySet.exists() (was: Interaction between exists() and group by)

2 views
Skip to first unread message

Django

unread,
Apr 21, 2022, 3:00:47 AM4/21/22
to django-...@googlegroups.com
#33655: Unnecessary column in a GROUP BY clause with QuerySet.exists()
-------------------------------------+-------------------------------------
Reporter: Marc Perrin | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: exists group by | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):

* cc: Simon Charette (added)
* type: Uncategorized => Cleanup/optimization
* stage: Unreviewed => Accepted


Comment:

Thanks for the report. I agree that `(1)` is unnecessary in the `GROUP BY`
clause unfortunately we cannot always remove constants from it because
folks can use them as an alias for n-th column (as you already noticed).
Maybe we could select only the first column from the `GROUP BY` clause
instead:
{{{#!diff
diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py
index 64e7927f7a..940d3141d1 100644
--- a/django/db/models/sql/query.py
+++ b/django/db/models/sql/query.py
@@ -582,8 +582,11 @@ class Query(BaseExpression):
q.clear_ordering(force=True)
if limit:
q.set_limits(high=1)
- q.add_extra({"a": 1}, None, None, None, None, None)
- q.set_extra_mask(["a"])
+ if q.group_by and not q.select:
+ q.add_select_col(q.group_by[0], "a")
+ else:
+ q.add_extra({"a": 1}, None , None, None, None, None)
+ q.set_extra_mask(["a"])
return q

def has_results(self, using):
}}}
Tentatively accepted for future investigation.

--
Ticket URL: <https://code.djangoproject.com/ticket/33655#comment:1>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Reply all
Reply to author
Forward
0 new messages