#26430: Coalesce in Aggregations ignored when EmptyResultSet returned
-------------------------------------+-------------------------------------
Reporter: Ryan Prater | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.9
(models, ORM) |
Severity: Normal | Resolution:
Keywords: aggregation | Triage Stage: Accepted
coalesce in queryset |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
Sorry for the delayed answer Nick.
> It seems that https://github.com/django/django/pull/6361 would be the
way forward. Was there a reason you abandoned that approach, Simon? Would
you be happy for me to pick that up again?
I think it would be the most straightforward way to get there but the main
issue with the `get_empty_result` approach is coalescence to database
expressions e.g. `Coalesce('col', Now())`.
It's tempting to mock our way there and ''cheat'' for provided expressions
in implementing Python equivalent (e.g. `Now.get_empty_result ->
timezone.now`) but it has its limits (e.g. imagine coalescing to calling a
database procedure). In the end `EmptyResultSet` is meant to be an
optimization so I believe we should turn it off in circumstances where it
affects the correctness of the returned results.
A simpler approach, inspired by Anssi's comment:4, would be to add a
`SQLCompiler.supports_empty_result_set = False` property and set it to
`False` for `SQLAggregateCompiler`. Parts of the code that raise this
exception could then be adjusted not to do so when the current compiler
doesn't support it.
(completely untested)
{{{#!diff
diff --git a/django/db/models/lookups.py b/django/db/models/lookups.py
index 8d3648b393..0b02287d2a 100644
--- a/django/db/models/lookups.py
+++ b/django/db/models/lookups.py
@@ -392,7 +392,7 @@ def process_rhs(self, compiler, connection):
except TypeError: # Unhashable items in self.rhs
rhs = [r for r in self.rhs if r is not None]
- if not rhs:
+ if not rhs and compiler.supports_empty_result_set:
raise EmptyResultSet
# rhs should be an iterable; use batch_process_rhs() to
diff --git a/django/db/models/sql/compiler.py
b/django/db/models/sql/compiler.py
index 7264929da8..6cfcbf7f9f 100644
--- a/django/db/models/sql/compiler.py
+++ b/django/db/models/sql/compiler.py
@@ -25,6 +25,7 @@ class SQLCompiler:
r'^(.*)\s(?:ASC|DESC).*',
re.MULTILINE | re.DOTALL,
)
+ supports_empty_result_set = True
def __init__(self, query, connection, using):
self.query = query
@@ -1636,6 +1637,11 @@ def pre_sql_setup(self):
class SQLAggregateCompiler(SQLCompiler):
+ # Due to the nature of aggregation coalescence some expressions might
+ # need to be interpreted on the database to return the correct value
+ # when dealing with an empty result set.
+ supports_empty_result_set = False
+
def as_sql(self):
"""
Create the SQL for this query. Return the SQL string and list of
}}}
I guess we could also use an hybrid approach where `SQLAggregateCompiler`
only sets `supports_empty_result_set = False` if any of
`outer_query.annotation_select.items()` is not a literal value prior to
proceeding with the compilation/`as_sql` phase. That would keep the
optimization for most cases but fallback to the database when it cannot be
computed on the Python side.
--
Ticket URL: <https://code.djangoproject.com/ticket/26430#comment:12>
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 [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/068.56f1ea2c92337b5bb3f6cfd58c93ef4e%40djangoproject.com.
