Changes between Initial Version and Version 1 of Ticket #34580, comment 1


Ignore:
Timestamp:
May 21, 2023, 10:48:01 AM (2 years ago)
Author:
Simon Charette

Legend:

Unmodified
Added
Removed
Modified

  • Ticket #34580, comment 1

    initial v1  
    66
    77If that's the case then I'm surprised this particular commit caused a regression in these `Count` tests as none of them make use or ordering. If that's truly the culprit then it's likely only a matter of not generating `selected_exprs` if there is no `ordering` in the first place.
     8
     9
     10{{{#!diff
     11diff --git a/django/db/models/sql/compiler.py b/django/db/models/sql/compiler.py
     12index a7a8c98b99..b28dc925ba 100644
     13--- a/django/db/models/sql/compiler.py
     14+++ b/django/db/models/sql/compiler.py
     15@@ -331,7 +331,9 @@ def _order_by_pairs(self):
     16             default_order, _ = ORDER_DIR["DESC"]
     17
     18         selected_exprs = {}
     19-        if select := self.select:
     20+        # Avoid computing `selected_exprs` if there is no `ordering` as it's
     21+        # relatively expensive.
     22+        if ordering and (select := self.select):
     23             for ordinal, (expr, _, alias) in enumerate(select, start=1):
     24                 pos_expr = PositionRef(ordinal, alias, expr)
     25                 if alias:
     26}}}
     27
     28Is there any way to confirm that the above patch addresses the regression? Can be run performance tests against the baseline from a particular PR?
Back to Top