From d87bd29c4f8dfcdf3f4a4eb8340e6770a2416fe3 Mon Sep 17 00:00:00 2001 From: can Date: Wed, 17 Apr 2019 09:24:28 +0300 Subject: [PATCH] Fixed #30335, #29139 -- Fixed crash when ordering or aggregating over a nested JSONField key transform. --- django/db/models/sql/compiler.py | 11 +++++++---- docs/releases/2.2.1.txt | 5 +++++ tests/postgres_tests/test_json.py | 25 ++++++++++++++++++++++++- 3 files changed, 36 insertions(+), 5 deletions(-) diff --git a/django/db/models/sql/compiler.py b/django/db/models/sql/compiler.py index f4b1faabc2..3d133cb672 100644 --- a/django/db/models/sql/compiler.py +++ b/django/db/models/sql/compiler.py @@ -14,6 +14,7 @@ from django.db.models.sql.query import Query, get_order_dir from django.db.transaction import TransactionManagementError from django.db.utils import DatabaseError, NotSupportedError from django.utils.deprecation import RemovedInDjango31Warning +from django.utils.hashable import make_hashable FORCE = object() @@ -126,9 +127,10 @@ class SQLCompiler: for expr in expressions: sql, params = self.compile(expr) - 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): @@ -352,9 +354,10 @@ class SQLCompiler: # 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 diff --git a/docs/releases/2.2.1.txt b/docs/releases/2.2.1.txt index 3a2453dbd1..a740f516ec 100644 --- a/docs/releases/2.2.1.txt +++ b/docs/releases/2.2.1.txt @@ -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`). diff --git a/tests/postgres_tests/test_json.py b/tests/postgres_tests/test_json.py index eab7f8a272..ee0bb6d5ea 100644 --- a/tests/postgres_tests/test_json.py +++ b/tests/postgres_tests/test_json.py @@ -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 @@ -15,6 +16,7 @@ from .models import JSONModel, PostgreSQLModel 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 @@ -162,6 +164,27 @@ class TestQuerying(PostgreSQLTestCase): 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(