Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed #31956 -- Fixed crash of ordering by JSONField with a custom decoder on PostgreSQL. #13358

Merged
merged 2 commits into from
Aug 28, 2020

Conversation

felixxm
Copy link
Member

@felixxm felixxm commented Aug 28, 2020

ticket-31956

Thanks Marc Debureaux for the report.

@felixxm felixxm force-pushed the issue-31956 branch 2 times, most recently from e5be296 to 57235fe Compare August 28, 2020 07:30
@felixxm felixxm requested a review from a team August 28, 2020 07:48
@felixxm
Copy link
Member Author

felixxm commented Aug 28, 2020

@adamchainz @pope1ni Thanks both 👍 I pushed updates.

Copy link
Member

@charettes charettes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Patch LGTM, glad to see this usage of select_format and ::text removed.

The CI failures are due to JSONBAgg.convert_value returning [] instead of '[]' as expected after this commit.

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

@felixxm
Copy link
Member Author

felixxm commented Aug 28, 2020

Thanks for reviews 🚀

…coder on PostgreSQL.

Thanks Marc Debureaux for the report.
Thanks Simon Charette, Nick Pope, and Adam Johnson for reviews.
@felixxm felixxm merged commit 0be51d2 into django:master Aug 28, 2020
@felixxm felixxm deleted the issue-31956 branch August 28, 2020 17:54
@laymonage
Copy link
Contributor

Sorry for not jumping into this earlier. For some reason, I didn't receive an email for the ticket and I discovered this just now.

Anyway, just leaving this note:

I think the primary reason why I didn't use psycopg2.extras.register_default_jsonb was that we initially thought the contrib.postgres implementation was going to be left as it is (with a deprecation message), instead of being a subclass of the new one. If that was going to be the case, then using psycopg2.extras.register_default_jsonb would break users who still use the old implementation. That's also why the ::text cast was also backported to the django-jsonfield package.

Thanks, everyone! ✌️

rvause pushed a commit to rvause/django-postgrespool2 that referenced this pull request Aug 8, 2022
rvause pushed a commit to rvause/django-postgrespool2 that referenced this pull request Aug 8, 2022
lcd1232 pushed a commit to lcd1232/django-postgrespool2 that referenced this pull request Oct 3, 2022
Fixes #25; related change in Django project django/django#13358

Co-authored-by: Rick Vause <rick.vause@pathai.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
8 participants