Skip to content

Commit

Permalink
[3.1.x] Fixed #31956 -- Fixed crash of ordering by JSONField with a c…
Browse files Browse the repository at this point in the history
…ustom decoder on PostgreSQL.

Thanks Marc Debureaux for the report.
Thanks Simon Charette, Nick Pope, and Adam Johnson for reviews.

Backport of 0be51d2 from master
  • Loading branch information
felixxm committed Aug 28, 2020
1 parent 3a42c04 commit 655e1ce
Show file tree
Hide file tree
Showing 8 changed files with 21 additions and 27 deletions.
2 changes: 1 addition & 1 deletion django/contrib/postgres/aggregates/general.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class JSONBAgg(Aggregate):

def convert_value(self, value, expression, connection):
if not value:
return []
return '[]'
return value


Expand Down
7 changes: 0 additions & 7 deletions django/db/backends/base/operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,13 +159,6 @@ def time_extract_sql(self, lookup_type, field_name):
"""
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
Expand Down
5 changes: 4 additions & 1 deletion django/db/backends/postgresql/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,10 @@ def get_new_connection(self, conn_params):
# 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):
Expand Down
3 changes: 0 additions & 3 deletions django/db/backends/postgresql/operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,6 @@ def datetime_trunc_sql(self, lookup_type, field_name, tzname):
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"

Expand Down
10 changes: 0 additions & 10 deletions django/db/models/fields/json.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,6 @@ def deconstruct(self):
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:
Expand All @@ -91,14 +89,6 @@ def get_transform(self, name):
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:
Expand Down
4 changes: 4 additions & 0 deletions docs/releases/3.1.1.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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`).
5 changes: 0 additions & 5 deletions tests/backends/base/test_operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,11 +121,6 @@ def test_datetime_extract_sql(self):
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):
Expand Down
12 changes: 12 additions & 0 deletions tests/model_fields/test_jsonfield.py
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,18 @@ def test_ordering_grouping_by_count(self):
).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(
Expand Down

0 comments on commit 655e1ce

Please sign in to comment.