Skip to content

Commit

Permalink
[2.2.x] Fixed #30335, #29139 -- Fixed crash when ordering or aggregat…
Browse files Browse the repository at this point in the history
…ing over a nested JSONField key transform.

Backport of d87bd29 from master.
  • Loading branch information
can authored and felixxm committed Apr 18, 2019
1 parent ef38777 commit e85317d
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 5 deletions.
11 changes: 7 additions & 4 deletions django/db/models/sql/compiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from django.utils.deprecation import (
RemovedInDjango30Warning, RemovedInDjango31Warning,
)
from django.utils.hashable import make_hashable
from django.utils.inspect import func_supports_parameter

FORCE = object()
Expand Down Expand Up @@ -135,9 +136,10 @@ def get_group_by(self, select, order_by):
# wrapping () because they could be removed when a subquery is
# the "rhs" in an expression (see Subquery._prepare()).
sql = '(%s)' % sql
if (sql, tuple(params)) not in seen:
params_hash = make_hashable(params)
if (sql, params_hash) not in seen:
result.append((sql, params))
seen.add((sql, tuple(params)))
seen.add((sql, params_hash))
return result

def collapse_group_by(self, expressions, having):
Expand Down Expand Up @@ -361,9 +363,10 @@ def get_order_by(self):
# is refactored into expressions, then we can check each part as we
# generate it.
without_ordering = self.ordering_parts.search(sql).group(1)
if (without_ordering, tuple(params)) in seen:
params_hash = make_hashable(params)
if (without_ordering, params_hash) in seen:
continue
seen.add((without_ordering, tuple(params)))
seen.add((without_ordering, params_hash))
result.append((resolved, (sql, params, is_ref)))
return result

Expand Down
5 changes: 5 additions & 0 deletions docs/releases/2.2.1.txt
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,8 @@ Bugfixes
* Reverted an optimization in Django 2.2 (:ticket:`29725`) that caused the
inconsistent behavior of ``count()`` and ``exists()`` on a reverse
many-to-many relationship with a custom manager (:ticket:`30325`).

* Fixed a regression in Django 2.2 where
:class:`~django.core.paginator.Paginator` crashed when ``object_list`` was
a queryset ordered or aggregated over a nested ``JSONField`` key transform
(:ticket:`30335`).
25 changes: 24 additions & 1 deletion tests/postgres_tests/test_json.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import datetime
import operator
import uuid
from decimal import Decimal

from django.core import checks, exceptions, serializers
from django.core.serializers.json import DjangoJSONEncoder
from django.db.models import Q
from django.db.models import Count, Q
from django.forms import CharField, Form, widgets
from django.test.utils import isolate_apps
from django.utils.html import escape
Expand All @@ -15,6 +16,7 @@
try:
from django.contrib.postgres import forms
from django.contrib.postgres.fields import JSONField
from django.contrib.postgres.fields.jsonb import KeyTextTransform, KeyTransform
except ImportError:
pass

Expand Down Expand Up @@ -153,6 +155,27 @@ def test_ordering_by_transform(self):
query = JSONModel.objects.filter(field__name__isnull=False).order_by('field__ord')
self.assertSequenceEqual(query, [objs[4], objs[2], objs[3], objs[1], objs[0]])

def test_ordering_grouping_by_key_transform(self):
base_qs = JSONModel.objects.filter(field__d__0__isnull=False)
for qs in (
base_qs.order_by('field__d__0'),
base_qs.annotate(key=KeyTransform('0', KeyTransform('d', 'field'))).order_by('key'),
):
self.assertSequenceEqual(qs, [self.objs[8]])
qs = JSONModel.objects.filter(field__isnull=False)
self.assertQuerysetEqual(
qs.values('field__d__0').annotate(count=Count('field__d__0')).order_by('count'),
[1, 10],
operator.itemgetter('count'),
)
self.assertQuerysetEqual(
qs.filter(field__isnull=False).annotate(
key=KeyTextTransform('f', KeyTransform('1', KeyTransform('d', 'field'))),
).values('key').annotate(count=Count('key')).order_by('count'),
[(None, 0), ('g', 1)],
operator.itemgetter('key', 'count'),
)

def test_deep_values(self):
query = JSONModel.objects.values_list('field__k__l')
self.assertSequenceEqual(
Expand Down

0 comments on commit e85317d

Please sign in to comment.