From 0e2649fdf40cedc5be7e2c0e5f7711f315e36b84 Mon Sep 17 00:00:00 2001 From: Mariusz Felisiak Date: Mon, 16 Jan 2023 10:22:02 +0100 Subject: [PATCH] Fixed #34255 -- Made PostgreSQL backend use client-side parameters binding with psycopg version 3. Thanks Guillaume Andreu Sabater for the report. Co-authored-by: Florian Apolloner --- django/db/backends/mysql/features.py | 10 +++++++++ django/db/backends/postgresql/base.py | 19 +++++++++++++--- docs/ref/databases.txt | 31 ++++++++++++++++++++++++--- docs/releases/4.2.txt | 4 ++++ tests/aggregation/tests.py | 15 +++++++++++++ tests/backends/postgresql/tests.py | 19 ++++++++++++++++ 6 files changed, 92 insertions(+), 6 deletions(-) diff --git a/django/db/backends/mysql/features.py b/django/db/backends/mysql/features.py index 2f5cc91881..14fde6b08e 100644 --- a/django/db/backends/mysql/features.py +++ b/django/db/backends/mysql/features.py @@ -144,6 +144,16 @@ class DatabaseFeatures(BaseDatabaseFeatures): }, } ) + if "ONLY_FULL_GROUP_BY" in self.connection.sql_mode: + skips.update( + { + "GROUP BY cannot contain nonaggregated column when " + "ONLY_FULL_GROUP_BY mode is enabled on MySQL, see #34262.": { + "aggregation.tests.AggregateTestCase." + "test_group_by_nested_expression_with_params", + }, + } + ) return skips @cached_property diff --git a/django/db/backends/postgresql/base.py b/django/db/backends/postgresql/base.py index 34a98e9e3a..9e24b186cf 100644 --- a/django/db/backends/postgresql/base.py +++ b/django/db/backends/postgresql/base.py @@ -223,6 +223,7 @@ class DatabaseWrapper(BaseDatabaseWrapper): conn_params.pop("assume_role", None) conn_params.pop("isolation_level", None) + conn_params.pop("server_side_binding", None) if settings_dict["USER"]: conn_params["user"] = settings_dict["USER"] if settings_dict["PASSWORD"]: @@ -268,14 +269,20 @@ class DatabaseWrapper(BaseDatabaseWrapper): connection = self.Database.connect(**conn_params) if set_isolation_level: connection.isolation_level = self.isolation_level - if not is_psycopg3: + if is_psycopg3: + connection.cursor_factory = ( + ServerBindingCursor + if options.get("server_side_binding") is True + else Cursor + ) + else: # 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 ) - connection.cursor_factory = Cursor + connection.cursor_factory = Cursor return connection def ensure_timezone(self): @@ -436,7 +443,7 @@ class DatabaseWrapper(BaseDatabaseWrapper): if is_psycopg3: - class Cursor(Database.Cursor): + class CursorMixin: """ A subclass of psycopg cursor implementing callproc. """ @@ -457,6 +464,12 @@ if is_psycopg3: self.execute(stmt) return args + class ServerBindingCursor(CursorMixin, Database.Cursor): + pass + + class Cursor(CursorMixin, Database.ClientCursor): + pass + class CursorDebugWrapper(BaseCursorDebugWrapper): def copy(self, statement): with self.debug_sql(statement): diff --git a/docs/ref/databases.txt b/docs/ref/databases.txt index 2dbf71562f..6755085336 100644 --- a/docs/ref/databases.txt +++ b/docs/ref/databases.txt @@ -117,9 +117,6 @@ PostgreSQL notes Django supports PostgreSQL 12 and higher. `psycopg`_ 3.1.8+ or `psycopg2`_ 2.8.4+ is required, though the latest `psycopg`_ 3.1.8+ is recommended. -.. _psycopg: https://www.psycopg.org/psycopg3/ -.. _psycopg2: https://www.psycopg.org/ - .. note:: Support for ``psycopg2`` is likely to be deprecated and removed at some @@ -251,6 +248,31 @@ database configuration in :setting:`DATABASES`:: }, } +.. _database-server-side-parameters-binding: + +Server-side parameters binding +------------------------------ + +.. versionadded:: 4.2 + +With `psycopg`_ 3.1.8+, Django defaults to the :ref:`client-side binding +cursors `. If you want to use the +:ref:`server-side binding ` set it in the +:setting:`OPTIONS` part of your database configuration in +:setting:`DATABASES`:: + + DATABASES = { + "default": { + "ENGINE": "django.db.backends.postgresql", + # ... + "OPTIONS": { + "server_side_binding": True, + }, + }, + } + +This option is ignored with ``psycopg2``. + Indexes for ``varchar`` and ``text`` columns -------------------------------------------- @@ -373,6 +395,9 @@ non-durable `_. a development machine where you can easily restore the entire contents of all databases in the cluster. +.. _psycopg: https://www.psycopg.org/psycopg3/ +.. _psycopg2: https://www.psycopg.org/ + .. _mariadb-notes: MariaDB notes diff --git a/docs/releases/4.2.txt b/docs/releases/4.2.txt index 83bdedfc75..717a13c08f 100644 --- a/docs/releases/4.2.txt +++ b/docs/releases/4.2.txt @@ -254,6 +254,10 @@ Database backends * The new ``"assume_role"`` option is now supported in :setting:`OPTIONS` on PostgreSQL to allow specifying the :ref:`session role `. +* The new ``"server_side_binding"`` option is now supported in + :setting:`OPTIONS` on PostgreSQL with ``psycopg`` 3.1.8+ to allow using + :ref:`server-side binding cursors `. + Decorators ~~~~~~~~~~ diff --git a/tests/aggregation/tests.py b/tests/aggregation/tests.py index 54e1e6f13a..c1ae9f0dd5 100644 --- a/tests/aggregation/tests.py +++ b/tests/aggregation/tests.py @@ -34,6 +34,7 @@ from django.db.models.functions import ( Cast, Coalesce, Greatest, + Least, Lower, Now, Pi, @@ -1614,6 +1615,20 @@ class AggregateTestCase(TestCase): ).annotate(total=Count("*")) self.assertEqual(dict(has_long_books_breakdown), {True: 2, False: 3}) + def test_group_by_nested_expression_with_params(self): + books_qs = ( + Book.objects.annotate(greatest_pages=Greatest("pages", Value(600))) + .values( + "greatest_pages", + ) + .annotate( + min_pages=Min("pages"), + least=Least("min_pages", "greatest_pages"), + ) + .values_list("least", flat=True) + ) + self.assertCountEqual(books_qs, [300, 946, 1132]) + @skipUnlessDBFeature("supports_subqueries_in_group_by") def test_aggregation_subquery_annotation_related_field(self): publisher = Publisher.objects.create(name=self.a9.name, num_awards=2) diff --git a/tests/backends/postgresql/tests.py b/tests/backends/postgresql/tests.py index 7b0e0bfb52..ab66e7ab0e 100644 --- a/tests/backends/postgresql/tests.py +++ b/tests/backends/postgresql/tests.py @@ -277,6 +277,25 @@ class Tests(TestCase): finally: new_connection.close() + @unittest.skipUnless(is_psycopg3, "psycopg3 specific test") + def test_connect_server_side_binding(self): + """ + The server-side parameters binding role can be enabled with DATABASES + ["OPTIONS"]["server_side_binding"]. + """ + from django.db.backends.postgresql.base import ServerBindingCursor + + new_connection = connection.copy() + new_connection.settings_dict["OPTIONS"]["server_side_binding"] = True + try: + new_connection.connect() + self.assertEqual( + new_connection.connection.cursor_factory, + ServerBindingCursor, + ) + finally: + new_connection.close() + def test_connect_no_is_usable_checks(self): new_connection = connection.copy() try: