Opened 2 weeks ago

Last modified 17 hours ago

#35734 assigned Bug

Tests fail with Postgres 16 and server side bindings

Reported by: john-parton Owned by: john-parton
Component: Database layer (models, ORM) Version: 5.1
Severity: Release blocker Keywords:
Cc: Nick Pope Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Tests fail with Postgres 16 and server side bindings

Steps to replicate

  1. Pull down main
  2. Up a postgres 16 database (supplied docker-compose.yml)
  3. Use supplied settings file
  4. Install requirements
  5. Run ./runtests.py --settings test_postgres db_functions.comparison

docker-compose.yml

services:
    postgres:
        image: postgres:16.3
        shm_size: 1g
        ports:
        - "5436:5432"
        environment:
            POSTGRES_USER: user
            POSTGRES_PASSWORD: postgres
            POSTGRES_DB: django

test_postgres.py

from test_sqlite import *  # NOQA

DATABASES = {
    "default": {
        "ENGINE": "django.db.backends.postgresql",
        "USER": "user",
        "NAME": "django",
        "PASSWORD": "postgres",
        "HOST": "localhost",
        "PORT": 5436,
        "OPTIONS": {
            "server_side_binding": True,
        },
    },
}

Here's the output of my pip freeze for reference

aiohappyeyeballs==2.4.0
aiohttp==3.10.5
aiosignal==1.3.1
aiosmtpd==1.4.6
argon2-cffi==23.1.0
argon2-cffi-bindings==21.2.0
asgiref==3.8.1
atpublic==5.0
attrs==24.2.0
bcrypt==4.2.0
black==24.8.0
certifi==2024.8.30
cffi==1.17.1
charset-normalizer==3.3.2
click==8.1.7
-e git+ssh://git@github.com/django/django.git@aa5293068782dfa2d2173c75c8477f58a9989942#egg=Django
docutils==0.21.2
frozenlist==1.4.1
geoip2==4.8.0
h11==0.14.0
idna==3.8
Jinja2==3.1.4
MarkupSafe==2.1.5
maxminddb==2.6.2
multidict==6.0.5
mypy-extensions==1.0.0
numpy==2.1.1
outcome==1.3.0.post0
packaging==24.1
pathspec==0.12.1
pillow==10.4.0
platformdirs==4.2.2
psycopg==3.2.1
psycopg-binary==3.2.1
psycopg-pool==3.2.2
pycparser==2.22
pylibmc==1.6.3
pymemcache==4.0.0
PySocks==1.7.1
pywatchman==2.0.0
PyYAML==6.0.2
redis==5.0.8
requests==2.32.3
selenium==4.24.0
setuptools==74.1.2
sniffio==1.3.1
sortedcontainers==2.4.0
sqlparse==0.5.1
tblib==3.0.0
trio==0.26.2
trio-websocket==0.11.1
typing_extensions==4.12.2
tzdata==2024.1
urllib3==2.2.2
websocket-client==1.8.0
wsproto==1.2.0
yarl==1.9.11

Python 3.12.3

Made this discovery while trying to finish a JSON feature, comment here: https://github.com/django/django/pull/18541#issuecomment-2331630403

Change History (10)

comment:1 by john-parton, 2 weeks ago

I can confirm this is also an issue with 5.1.1 and 5.1, but not an issue with 5.0.9.

Here's where I think the error was introduced.

5.0.9: https://github.com/django/django/blob/8e68f938f376cf2ca22a7e8ff0bcbe1b7a5832d1/django/db/models/functions/comparison.py#L163-L176
5.1: https://github.com/django/django/blob/373cb3037fe4e67adbac9ac43340391e859aa957/django/db/models/functions/comparison.py#L192

Basically on postgres 16, there's an attempt to use "native" JSON, but it doesn't work for nested objects.

Last edited 2 weeks ago by john-parton (previous) (diff)

comment:2 by john-parton, 2 weeks ago

Version: dev5.1

comment:3 by Natalia Bidart, 2 weeks ago

Cc: Nick Pope added
Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

Thank you John!

Confirmed this is a regression in 81ccf92f154c6d9eac3e30bac0aa67574d0ace15.

comment:4 by john-parton, 2 weeks ago

Thanks. I assume Nick can take a crack at it? Otherwise I can take a look. A little busy at the moment, so not sure when I could get to it.

EDIT

Do we need to add Postgres 16/15 to CI? I know there's already a ton of different configs tested, but it would have caught this if it were in the CI pipeline.

Last edited 2 weeks ago by john-parton (previous) (diff)

comment:6 by john-parton, 2 weeks ago

Has patch: set
Needs tests: set

comment:7 by Sarah Boyce, 11 days ago

Owner: set to john-parton
Status: newassigned

in reply to:  4 comment:8 by Natalia Bidart, 10 days ago

Replying to john-parton:

Thanks. I assume Nick can take a crack at it? Otherwise I can take a look. A little busy at the moment, so not sure when I could get to it.

Thank you John for your PR and work on this. We'll review soon!

Do we need to add Postgres 16/15 to CI? I know there's already a ton of different configs tested, but it would have caught this if it were in the CI pipeline.

This is a very valid point and I would like us to pursue a more sustainable solution for this testing other than "each one does it on their own laptop".
John, would you be willing to raise this issue in the Django Forum to see if we could get some traction to setup this? I think we could have at least a new entry in the scheduled tests to test all supported version, or at least the oldest (current setup) and the newest.

comment:9 by john-parton, 10 days ago

Topic here: https://forum.djangoproject.com/t/postgres-16-ci-postgres-16-in-use/34649

Yeah, I'm not sure what the cost in $$$ to run the CI is. I think this is a pretty unusual corner case, so maybe it's not worth the hassle.

comment:10 by GitHub <noreply@…>, 17 hours ago

In 92f860e3:

Refs #35734 -- Added entry to scheduled tests workflow to test newer PostgreSQL versions.

Note: See TracTickets for help on using tickets.
Back to Top