Re: [Django] #31956: QuerySet.order_by() chained with values() crashes on JSONField with a custom decoder on PostgreSQL.

[Django]

#31956: QuerySet.order_by() chained with values() crashes on JSONField with a
custom decoder on PostgreSQL.
-------------------------------------+-------------------------------------
Reporter: Marc DEBUREAUX | Owner: felixxm
Type: Bug | Status: assigned
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by felixxm):

We cannot use proposed solution because it breaks ordering for non-text
values.

--
Ticket URL: <https://code.djangoproject.com/ticket/31956#comment:3>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#31956: QuerySet.order_by() chained with values() crashes on JSONField with a
custom decoder on PostgreSQL.
-------------------------------------+-------------------------------------
Reporter: Marc DEBUREAUX | Owner: felixxm
Type: Bug | Status: assigned
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by felixxm):

* has_patch: 0 => 1

Comment:

[https://github.com/django/django/pull/13358 PR]

--
Ticket URL: <https://code.djangoproject.com/ticket/31956#comment:4>

[Django]

#31956: QuerySet.order_by() chained with values() crashes on JSONField with a
custom decoder on PostgreSQL.
-------------------------------------+-------------------------------------
Reporter: Marc DEBUREAUX | Owner: felixxm
Type: Bug | Status: assigned
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mariusz Felisiak <felisiak.mariusz@…>):

In [changeset:"22105391424cc56f29f153bb76d6a15246152674" 2210539]:
{{{
#!CommitTicketReference repository=""
revision="22105391424cc56f29f153bb76d6a15246152674"
Refs #31956 -- Added test for ordering by JSONField with a custom decoder.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/31956#comment:5>

[Django]

#31956: QuerySet.order_by() chained with values() crashes on JSONField with a
custom decoder on PostgreSQL.
-------------------------------------+-------------------------------------
Reporter: Marc DEBUREAUX | Owner: felixxm

Type: Bug | Status: closed

Component: Database layer | Version: 3.1
(models, ORM) |

Severity: Release blocker | Resolution: fixed

Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mariusz Felisiak <felisiak.mariusz@…>):

In [changeset:"655e1ce6b1ca8df71518060ae770c3ee647b9801" 655e1ce6]:
{{{
#!CommitTicketReference repository=""
revision="655e1ce6b1ca8df71518060ae770c3ee647b9801"
[3.1.x] 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.

Backport of 0be51d2226fce030ac9ca840535a524f41e9832c from master
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/31956#comment:7>

[Django]

#31956: QuerySet.order_by() chained with values() crashes on JSONField with a
custom decoder on PostgreSQL.
-------------------------------------+-------------------------------------
Reporter: Marc DEBUREAUX | Owner: felixxm
Type: Bug | Status: closed
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak <felisiak.mariusz@…>):

* status: assigned => closed
* resolution: => fixed

Comment:

In [changeset:"0be51d2226fce030ac9ca840535a524f41e9832c" 0be51d2]:
{{{
#!CommitTicketReference repository=""
revision="0be51d2226fce030ac9ca840535a524f41e9832c"

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.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/31956#comment:6>

[Django]

#31956: QuerySet.order_by() chained with values() crashes on JSONField with a
custom decoder on PostgreSQL.
-------------------------------------+-------------------------------------
Reporter: Marc DEBUREAUX | Owner: felixxm
Type: Bug | Status: closed
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Agris Ameriks):

Hello,
I would ask to reopen this ticket.

The fix breaks default functionality of JSONField.

Let me explain the issue.

I have such an model:
{{{
class Instance(models.Model):
title = models.CharField('Title', max_length=255, blank=True)
responses = models.JSONField(default=dict)
}}}

Code:
{{{
query = "SELECT id, responses FROM advancedform_instance"
with connection.cursor() as cursor:
cursor.execute(query)
data = cursor.fetchall()
}}}

In django 3.1 responses is returned as a dict.
In django 3.1.1 it is returned as a string.

If I remove the code that was made to fix the issue, it works correctly.

--
Ticket URL: <https://code.djangoproject.com/ticket/31956#comment:8>

[Django]

#31956: QuerySet.order_by() chained with values() crashes on JSONField with a
custom decoder on PostgreSQL.
-------------------------------------+-------------------------------------
Reporter: Marc DEBUREAUX | Owner: felixxm

Type: Bug | Status: new

Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Release blocker | Resolution:

Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Agris Ameriks):

* status: closed => new
* resolution: fixed =>

--
Ticket URL: <https://code.djangoproject.com/ticket/31956#comment:9>

[Django]

#31956: QuerySet.order_by() chained with values() crashes on JSONField with a
custom decoder on PostgreSQL.
-------------------------------------+-------------------------------------
Reporter: Marc DEBUREAUX | Owner: felixxm

Type: Bug | Status: closed

Component: Database layer | Version: 3.1
(models, ORM) |

Severity: Release blocker | Resolution: fixed

Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by felixxm):

* status: new => closed
* resolution: => fixed

Comment:

Please don't reopen closed tickets. You should create a new ticket if you
want to report a regression. Moreover we're aware of this behavior change.
Is there any reason to use a row cursor and SQL instead of the ORM? You
can always use `json.loads()` on fetched data.

--
Ticket URL: <https://code.djangoproject.com/ticket/31956#comment:10>

[Django]

#31956: QuerySet.order_by() chained with values() crashes on JSONField with a
custom decoder on PostgreSQL.
-------------------------------------+-------------------------------------
Reporter: Marc DEBUREAUX | Owner: felixxm
Type: Bug | Status: closed
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Agris Ameriks):

Sorry, I will add comment to already created bug for the regression
(#31973).
Yes, I simplified the query that I have. It is much complex and it's much
harder to implement it in ORM.
I think that JSONField should return the dict either way and it was
working fine in 3.1.
Also the bug is related to "with a custom decoder", but in current
scenario I'm not specifying different decoder.

Yes, I can use `json.loads()` and I was using it till Django started
supporting JSONField with built-in decoder (not sure which version, but
long time ago). I don't think that it is ok to return to "unsupporting"
default behavior.

Replying to [comment:10 felixxm]:

> Please don't reopen closed tickets. You should create a new ticket if
you want to report a regression. Moreover we're aware of this behavior
change. Is there any reason to use a row cursor and SQL instead of the
ORM? You can always use `json.loads()` on fetched data.

--
Ticket URL: <https://code.djangoproject.com/ticket/31956#comment:11>

[Django]

#31956: QuerySet.order_by() chained with values() crashes on JSONField with a
custom decoder on PostgreSQL.
-------------------------------------+-------------------------------------
Reporter: Marc DEBUREAUX | Owner: felixxm
Type: Bug | Status: closed
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by felixxm):

Agris, see #31991.

--
Ticket URL: <https://code.djangoproject.com/ticket/31956#comment:12>

[Django]

#31956: QuerySet.order_by() chained with values() crashes on JSONField with a
custom decoder on PostgreSQL.
-------------------------------------+-------------------------------------
Reporter: Marc DEBUREAUX | Owner: felixxm
Type: Bug | Status: closed
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Thiago Bellini Ribeiro):

* cc: Thiago Bellini Ribeiro (added)

--
Ticket URL: <https://code.djangoproject.com/ticket/31956#comment:13>

[Django]

#31956: QuerySet.order_by() chained with values() crashes on JSONField with a
custom decoder on PostgreSQL.
-------------------------------------+-------------------------------------
Reporter: Marc DEBUREAUX | Owner: felixxm
Type: Bug | Status: closed
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by GitHub <noreply@…>):

In [changeset:"438b85dfab4f16a2e709e2bcdbfefecd7bfee89e" 438b85df]:
{{{
#!CommitTicketReference repository=""
revision="438b85dfab4f16a2e709e2bcdbfefecd7bfee89e"
Refs #31956 -- Doc'd consequences of disabling psycopg2's JSONB
typecaster.

Follow up to 0be51d2226fce030ac9ca840535a524f41e9832c.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/31956#comment:14>

[Django]

#31956: QuerySet.order_by() chained with values() crashes on JSONField with a
custom decoder on PostgreSQL.
-------------------------------------+-------------------------------------
Reporter: Marc DEBUREAUX | Owner: felixxm
Type: Bug | Status: closed
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mariusz Felisiak <felisiak.mariusz@…>):

In [changeset:"6b16623bd9613c3b0dc7d1fb6f14c00768f364a9" 6b16623b]:
{{{
#!CommitTicketReference repository=""
revision="6b16623bd9613c3b0dc7d1fb6f14c00768f364a9"
[3.1.x] Refs #31956 -- Doc'd consequences of disabling psycopg2's JSONB
typecaster.

Follow up to 0be51d2226fce030ac9ca840535a524f41e9832c.
Backport of 438b85dfab4f16a2e709e2bcdbfefecd7bfee89e from master
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/31956#comment:15>

[Django]

#31956: QuerySet.order_by() chained with values() crashes on JSONField with a
custom decoder on PostgreSQL.
-------------------------------------+-------------------------------------

Reporter: Marc DEBUREAUX | Owner: Mariusz
| Felisiak

Type: Bug | Status: closed
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Andrew):

How did this *manage* to break basic querying?

Replying to [comment:10 Mariusz Felisiak]:

> Moreover we're aware of this behavior change. Is there any reason to use
a row cursor and SQL instead of the ORM? You can always use `json.loads()`
on fetched data.

This is just silly. Yes, there are *obviously* reasons to use raw sql
over an ORM in some cases, this is not new. Putting a tiny note in a
release that no one will see isn't enough. This needs to be documented
front and center in the raw sql main docs and the postgres areas, because
I did read those and no hint.

"just use x" is a bad solution to something that broke basic
functionality, and isn't necessarily even easy to do. It turns what is a
simple automapping into manual mapping, at best. I guess we could
implement something that parses python typehints on result classes detects
if the cursor has been effed with and is returning the wrong data type,
though. How many more data types do you expect to be broken on purpose in
the future?

Also, the workaround is just to add `::json` to any `jsonb` columns in raw
queries. So this is only broken for HALF types?

It feels like an easy fix for one problem which ignores some other
underlying issue, breaking use cases that a few people don't personally
use. If I want a query to work in the future *as written* am I going to
have to start creating raw psycopg2 cursors? Because those still work!

--
Ticket URL: <https://code.djangoproject.com/ticket/31956#comment:16>

[Django]

#31956: QuerySet.order_by() chained with values() crashes on JSONField with a
custom decoder on PostgreSQL.
-------------------------------------+-------------------------------------
Reporter: Marc DEBUREAUX | Owner: Mariusz
| Felisiak
Type: Bug | Status: closed
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by nitishxp):

I agree with @Andrew my env also broked with no clue what happend.

--
Ticket URL: <https://code.djangoproject.com/ticket/31956#comment:17>

[Django]

#31956: QuerySet.order_by() chained with values() crashes on JSONField with a
custom decoder on PostgreSQL.
-------------------------------------+-------------------------------------
Reporter: Marc DEBUREAUX | Owner: Mariusz
| Felisiak
Type: Bug | Status: closed
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by mccartnm):

For anyone that falls into the "why isn't it a pythonic object already?"
pitfall - We've been able to achieve our existing patterns, post this
change, using:

1. Take complete control over the `psycopg2.extras.register_default_jsonb`
function, providing our own globally registered load call. Note: You can
supply an alternate JSON `loads` function to the
`_original_jsonb_function` as required and it will stick (hooray!)

{{{
import json
import psycopg2
from django.db.backends.signals import connection_created

_original_jsonb_function = None
def _handle_jsonb(sender, connection, **kwargs):
"""
Workaround for pipeline-altering solution in:

https://code.djangoproject.com/ticket/31956
"""
global _original_jsonb_function

def _ignore_command(*args, **kwargs):
pass

if _original_jsonb_function is None:
_original_jsonb_function = psycopg2.extras.register_default_jsonb
_original_jsonb_function(globally=True)
psycopg2.extras.register_default_jsonb = _ignore_command

class CoreConfig(AppConfig):
name = 'core'
verbose_name = 'Core'

def ready(self):
connection_created.connect(_handle_jsonb)
}}}

2. Overloading the JSONField model and returning the data unharmed. This
is key to allow both raw and ORM queries to operate as expected. You'll
need to replace all instances of `models.JSONField` in your application.

{{{
class DirectJSONField(models.JSONField):
""" ... """
def from_db_value(self, value, expression, connection):
if isinstance(value, (dict, list)):
return value
return super().from_db_value(value, expression, connection)
}}}

2. (alternate) If you're not inclined to replace all of your JSONFields,
you can monkey patch the class.

{{{
def from_db_value(self, value, expression, connection):
if isinstance(value, (dict, list)):
return value
return super().from_db_value(value, expression, connection)

models.JSONField.from_db_value = from_db_value
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/31956#comment:18>