From 0be51d2226fce030ac9ca840535a524f41e9832c Mon Sep 17 00:00:00 2001 From: Mariusz Felisiak Date: Fri, 28 Aug 2020 07:56:04 +0200 Subject: [PATCH] Fixed #31956 -- Fixed crash of ordering by JSONField with a custom decoder on PostgreSQL. Thanks Marc Debureaux for the report. Thanks Simon Charette, Nick Pope, and Adam Johnson for reviews. --- django/contrib/postgres/aggregates/general.py | 2 +- django/db/backends/base/operations.py | 7 ------- django/db/backends/postgresql/base.py | 5 ++++- django/db/backends/postgresql/operations.py | 3 --- django/db/models/fields/json.py | 10 ---------- docs/releases/3.1.1.txt | 4 ++++ tests/backends/base/test_operations.py | 5 ----- tests/model_fields/test_jsonfield.py | 12 ++++++++++++ 8 files changed, 21 insertions(+), 27 deletions(-) diff --git a/django/contrib/postgres/aggregates/general.py b/django/contrib/postgres/aggregates/general.py index f78eed7a60..ea94ab282a 100644 --- a/django/contrib/postgres/aggregates/general.py +++ b/django/contrib/postgres/aggregates/general.py @@ -48,7 +48,7 @@ class JSONBAgg(OrderableAggMixin, Aggregate): def convert_value(self, value, expression, connection): if not value: - return [] + return '[]' return value diff --git a/django/db/backends/base/operations.py b/django/db/backends/base/operations.py index 617ac95907..d8496f7e2c 100644 --- a/django/db/backends/base/operations.py +++ b/django/db/backends/base/operations.py @@ -153,13 +153,6 @@ class BaseDatabaseOperations: """ return self.date_extract_sql(lookup_type, field_name) - def json_cast_text_sql(self, field_name): - """Return the SQL to cast a JSON value to text value.""" - raise NotImplementedError( - 'subclasses of BaseDatabaseOperations may require a ' - 'json_cast_text_sql() method' - ) - def deferrable_sql(self): """ Return the SQL to make a constraint "initially deferred" during a diff --git a/django/db/backends/postgresql/base.py b/django/db/backends/postgresql/base.py index ed911a91da..03c7a3c284 100644 --- a/django/db/backends/postgresql/base.py +++ b/django/db/backends/postgresql/base.py @@ -200,7 +200,10 @@ class DatabaseWrapper(BaseDatabaseWrapper): # Set the isolation level to the value from OPTIONS. if self.isolation_level != connection.isolation_level: connection.set_session(isolation_level=self.isolation_level) - + # Register dummy loads() to avoid a round trip from psycopg2's decode + # to json.dumps() to json.loads(), when using a custom decoder in + # JSONField. + psycopg2.extras.register_default_jsonb(conn_or_curs=connection, loads=lambda x: x) return connection def ensure_timezone(self): diff --git a/django/db/backends/postgresql/operations.py b/django/db/backends/postgresql/operations.py index a3442231af..1ce5755bf5 100644 --- a/django/db/backends/postgresql/operations.py +++ b/django/db/backends/postgresql/operations.py @@ -74,9 +74,6 @@ class DatabaseOperations(BaseDatabaseOperations): def time_trunc_sql(self, lookup_type, field_name): return "DATE_TRUNC('%s', %s)::time" % (lookup_type, field_name) - def json_cast_text_sql(self, field_name): - return '(%s)::text' % field_name - def deferrable_sql(self): return " DEFERRABLE INITIALLY DEFERRED" diff --git a/django/db/models/fields/json.py b/django/db/models/fields/json.py index 00df1ae206..a249f4cdbf 100644 --- a/django/db/models/fields/json.py +++ b/django/db/models/fields/json.py @@ -70,8 +70,6 @@ class JSONField(CheckFieldDefaultMixin, Field): def from_db_value(self, value, expression, connection): if value is None: return value - if connection.features.has_native_json_field and self.decoder is None: - return value try: return json.loads(value, cls=self.decoder) except json.JSONDecodeError: @@ -91,14 +89,6 @@ class JSONField(CheckFieldDefaultMixin, Field): return transform return KeyTransformFactory(name) - def select_format(self, compiler, sql, params): - if ( - compiler.connection.features.has_native_json_field and - self.decoder is not None - ): - return compiler.connection.ops.json_cast_text_sql(sql), params - return super().select_format(compiler, sql, params) - def validate(self, value, model_instance): super().validate(value, model_instance) try: diff --git a/docs/releases/3.1.1.txt b/docs/releases/3.1.1.txt index 68c02392bd..452a922608 100644 --- a/docs/releases/3.1.1.txt +++ b/docs/releases/3.1.1.txt @@ -51,3 +51,7 @@ Bugfixes * Fixed detecting an async ``get_response`` callable in various builtin middlewares (:ticket:`31928`). + +* Fixed a ``QuerySet.order_by()`` crash on PostgreSQL when ordering and + grouping by :class:`~django.db.models.JSONField` with a custom + :attr:`~django.db.models.JSONField.decoder` (:ticket:`31956`). diff --git a/tests/backends/base/test_operations.py b/tests/backends/base/test_operations.py index 5c622e157e..1cfea44e83 100644 --- a/tests/backends/base/test_operations.py +++ b/tests/backends/base/test_operations.py @@ -117,11 +117,6 @@ class SimpleDatabaseOperationTests(SimpleTestCase): with self.assertRaisesMessage(NotImplementedError, self.may_require_msg % 'datetime_extract_sql'): self.ops.datetime_extract_sql(None, None, None) - def test_json_cast_text_sql(self): - msg = self.may_require_msg % 'json_cast_text_sql' - with self.assertRaisesMessage(NotImplementedError, msg): - self.ops.json_cast_text_sql(None) - class DatabaseOperationTests(TestCase): def setUp(self): diff --git a/tests/model_fields/test_jsonfield.py b/tests/model_fields/test_jsonfield.py index 320d518554..665e46e6a3 100644 --- a/tests/model_fields/test_jsonfield.py +++ b/tests/model_fields/test_jsonfield.py @@ -359,6 +359,18 @@ class TestQuerying(TestCase): ).values('value__d__0').annotate(count=Count('value__d__0')).order_by('count') self.assertQuerysetEqual(qs, [1, 11], operator.itemgetter('count')) + def test_order_grouping_custom_decoder(self): + NullableJSONModel.objects.create(value_custom={'a': 'b'}) + qs = NullableJSONModel.objects.filter(value_custom__isnull=False) + self.assertSequenceEqual( + qs.values( + 'value_custom__a', + ).annotate( + count=Count('id'), + ).order_by('value_custom__a'), + [{'value_custom__a': 'b', 'count': 1}], + ) + def test_key_transform_raw_expression(self): expr = RawSQL(self.raw_sql, ['{"x": "bar"}']) self.assertSequenceEqual(