mirror of https://github.com/django/django.git
Fixed #35028 -- Disabled server-side bindings for named cursors on psycopg >= 3.
While we provide a `cursor_factory` based on the value of the `server_side_bindings` option to `psycopg.Connection` it is ignored by the `cursor` method when `name` is specified for `QuerySet.iterator()` usage and it causes the usage of `psycopg.ServerCursor` which performs server-side bindings. Since the ORM doesn't generates SQL that is suitable for server-side bindings when dealing with parametrized expressions a specialized cursor must be used to allow server-side cursors to be used with client-side bindings. Thanks Richard Ebeling for the report. Thanks Florian Apolloner and Daniele Varrazzo for reviews.
This commit is contained in:
parent
02eaee1209
commit
92d6cff6a2
|
@ -321,11 +321,26 @@ class DatabaseWrapper(BaseDatabaseWrapper):
|
|||
@async_unsafe
|
||||
def create_cursor(self, name=None):
|
||||
if name:
|
||||
# In autocommit mode, the cursor will be used outside of a
|
||||
# transaction, hence use a holdable cursor.
|
||||
cursor = self.connection.cursor(
|
||||
name, scrollable=False, withhold=self.connection.autocommit
|
||||
)
|
||||
if is_psycopg3 and (
|
||||
self.settings_dict.get("OPTIONS", {}).get("server_side_binding")
|
||||
is not True
|
||||
):
|
||||
# psycopg >= 3 forces the usage of server-side bindings for
|
||||
# named cursors so a specialized class that implements
|
||||
# server-side cursors while performing client-side bindings
|
||||
# must be used if `server_side_binding` is disabled (default).
|
||||
cursor = ServerSideCursor(
|
||||
self.connection,
|
||||
name=name,
|
||||
scrollable=False,
|
||||
withhold=self.connection.autocommit,
|
||||
)
|
||||
else:
|
||||
# In autocommit mode, the cursor will be used outside of a
|
||||
# transaction, hence use a holdable cursor.
|
||||
cursor = self.connection.cursor(
|
||||
name, scrollable=False, withhold=self.connection.autocommit
|
||||
)
|
||||
else:
|
||||
cursor = self.connection.cursor()
|
||||
|
||||
|
@ -469,6 +484,23 @@ if is_psycopg3:
|
|||
class Cursor(CursorMixin, Database.ClientCursor):
|
||||
pass
|
||||
|
||||
class ServerSideCursor(
|
||||
CursorMixin, Database.client_cursor.ClientCursorMixin, Database.ServerCursor
|
||||
):
|
||||
"""
|
||||
psycopg >= 3 forces the usage of server-side bindings when using named
|
||||
cursors but the ORM doesn't yet support the systematic generation of
|
||||
prepareable SQL (#20516).
|
||||
|
||||
ClientCursorMixin forces the usage of client-side bindings while
|
||||
ServerCursor implements the logic required to declare and scroll
|
||||
through named cursors.
|
||||
|
||||
Mixing ClientCursorMixin in wouldn't be necessary if Cursor allowed to
|
||||
specify how parameters should be bound instead, which ServerCursor
|
||||
would inherit, but that's not the case.
|
||||
"""
|
||||
|
||||
class CursorDebugWrapper(BaseCursorDebugWrapper):
|
||||
def copy(self, statement):
|
||||
with self.debug_sql(statement):
|
||||
|
|
|
@ -4,12 +4,18 @@ from collections import namedtuple
|
|||
from contextlib import contextmanager
|
||||
|
||||
from django.db import connection, models
|
||||
from django.db.utils import ProgrammingError
|
||||
from django.test import TestCase
|
||||
from django.test.utils import garbage_collect
|
||||
from django.utils.version import PYPY
|
||||
|
||||
from ..models import Person
|
||||
|
||||
try:
|
||||
from django.db.backends.postgresql.psycopg_any import is_psycopg3
|
||||
except ImportError:
|
||||
is_psycopg3 = False
|
||||
|
||||
|
||||
@unittest.skipUnless(connection.vendor == "postgresql", "PostgreSQL tests")
|
||||
class ServerSideCursorsPostgres(TestCase):
|
||||
|
@ -20,8 +26,8 @@ class ServerSideCursorsPostgres(TestCase):
|
|||
|
||||
@classmethod
|
||||
def setUpTestData(cls):
|
||||
Person.objects.create(first_name="a", last_name="a")
|
||||
Person.objects.create(first_name="b", last_name="b")
|
||||
cls.p0 = Person.objects.create(first_name="a", last_name="a")
|
||||
cls.p1 = Person.objects.create(first_name="b", last_name="b")
|
||||
|
||||
def inspect_cursors(self):
|
||||
with connection.cursor() as cursor:
|
||||
|
@ -108,3 +114,43 @@ class ServerSideCursorsPostgres(TestCase):
|
|||
# collection breaks the transaction wrapping the test.
|
||||
with self.override_db_setting(DISABLE_SERVER_SIDE_CURSORS=True):
|
||||
self.assertNotUsesCursor(Person.objects.iterator())
|
||||
|
||||
@unittest.skipUnless(
|
||||
is_psycopg3, "The server_side_binding option is only effective on psycopg >= 3."
|
||||
)
|
||||
def test_server_side_binding(self):
|
||||
"""
|
||||
The ORM still generates SQL that is not suitable for usage as prepared
|
||||
statements but psycopg >= 3 defaults to using server-side bindings for
|
||||
server-side cursors which requires some specialized logic when the
|
||||
`server_side_binding` setting is disabled (default).
|
||||
"""
|
||||
|
||||
def perform_query():
|
||||
# Generates SQL that is known to be problematic from a server-side
|
||||
# binding perspective as the parametrized ORDER BY clause doesn't
|
||||
# use the same binding parameter as the SELECT clause.
|
||||
qs = (
|
||||
Person.objects.order_by(
|
||||
models.functions.Coalesce("first_name", models.Value(""))
|
||||
)
|
||||
.distinct()
|
||||
.iterator()
|
||||
)
|
||||
self.assertSequenceEqual(list(qs), [self.p0, self.p1])
|
||||
|
||||
with self.override_db_setting(OPTIONS={}):
|
||||
perform_query()
|
||||
|
||||
with self.override_db_setting(OPTIONS={"server_side_binding": False}):
|
||||
perform_query()
|
||||
|
||||
with self.override_db_setting(OPTIONS={"server_side_binding": True}):
|
||||
# This assertion could start failing the moment the ORM generates
|
||||
# SQL suitable for usage as prepared statements (#20516) or if
|
||||
# psycopg >= 3 adapts psycopg.Connection(cursor_factory) machinery
|
||||
# to allow client-side bindings for named cursors. In the first
|
||||
# case this whole test could be removed, in the second one it would
|
||||
# most likely need to be adapted.
|
||||
with self.assertRaises(ProgrammingError):
|
||||
perform_query()
|
||||
|
|
Loading…
Reference in New Issue