#35586: Queryset count and length do not match when annotating using
JSONB_PATH_QUERY
-------------------------------------+-------------------------------------
Reporter: devin13cox | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: 5.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Simon Charette):
* cc: Simon Charette (added)
* stage: Unreviewed => Accepted
Comment:
This one is tricky, the ORM and its aggregation logic was not built in a
way that accounts for annotations spawning rows themselves (instead of
through `JOIN`s) like some [https://www.postgresql.org/docs/current
/functions-srf.html Postgres set-returning functions] (e.g. `UNNEST`).
These do effectively throw a wrench in the optimization introduced in 4.2
if the set-returning annotation is not referenced by the aggregation.
What is really needed here is likely a `Expression.set_returning: bool =
False` attribute that the ORM can consider to enforce the subquery
pushdown and preventing of annotation eliding.
Something like
{{{#!diff
diff --git a/django/db/models/expressions.py
b/django/db/models/expressions.py
index 4ee22420d9..589fd30b0f 100644
--- a/django/db/models/expressions.py
+++ b/django/db/models/expressions.py
@@ -182,6 +182,8 @@ class BaseExpression:
allowed_default = False
# Can the expression be used during a constraint validation?
constraint_validation_compatible = True
+ # Does the expression possibly return more than one row?
+ set_returning = False
def __init__(self, output_field=None):
if output_field is not None:
diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py
index 438bb5ddbd..0eb1a2476c 100644
--- a/django/db/models/sql/query.py
+++ b/django/db/models/sql/query.py
@@ -491,6 +491,10 @@ def get_aggregation(self, using, aggregate_exprs):
)
or having
)
+ has_set_returning_annotation = any(
+ getattr(annotation, "set_returning", False)
+ for annotation in self.annotations.values()
+ )
# Decide if we need to use a subquery.
#
# Existing aggregations would cause incorrect results as
@@ -510,6 +514,7 @@ def get_aggregation(self, using, aggregate_exprs):
or qualify
or self.distinct
or self.combinator
+ or has_set_returning_annotation
):
from django.db.models.sql.subqueries import AggregateQuery
}}}
That could then be used like
{{{#!python
class JSONPathQuery(Func):
function = "jsonb_path_query"
output_field = JSONField()
set_returning = True
JSONPathQuery("test_json", Value("$.test_key[*]"))
}}}
The challenge here is that Django doesn't have any ''core'' set-returning
function (which also explains why this slipped under the radar for so
long) so maybe we should also consider adding support for
`contrib.postgres.Unnest` which is a common one that would allow us to
ensure proper test coverage and document it as an example of when this
flag should be set?
--
Ticket URL: <https://code.djangoproject.com/ticket/35586#comment:3>
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/0107019099428d0f-5940fdee-0697-4131-a036-6ac385f4d97b-000000%40eu-central-1.amazonses.com.
