From: Daniele Varrazzo <daniele.varrazzo@gmail.com>
Date: Mon, 31 Jul 2023 22:52:45 +0000 (+0100)
Subject: fix: fix resetting read_only/deferrable to default using None
X-Git-Tag: 3.1.10~8
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8b9bf434c4824e9b220da8b574425766d4ff57cb;p=thirdparty%2Fpsycopg.git

fix: fix resetting read_only/deferrable to default using None
---

diff --git a/docs/news.rst b/docs/news.rst
index e526b49b2..6655a8135 100644
--- a/docs/news.rst
+++ b/docs/news.rst
@@ -23,6 +23,8 @@ Psycopg 3.1.10 (unreleased)
 - Fix possible (ignored) exception on objects deletion (:ticket:`#591`).
 - Don't clobber a Python exception raised during COPY FROM with the resulting
   `!QueryCanceled` raised as a consequence (:ticket:`#593`).
+- Fix resetting `Connection.read_only` and `~Connection.deferrable` to their
+  default value using `!None` (:ticket:`#612`).
 
 
 Current release
diff --git a/psycopg/psycopg/connection.py b/psycopg/psycopg/connection.py
index 195c784e4..0ff3bcf68 100644
--- a/psycopg/psycopg/connection.py
+++ b/psycopg/psycopg/connection.py
@@ -231,7 +231,7 @@ class BaseConnection(Generic[Row]):
 
     def _set_read_only_gen(self, value: Optional[bool]) -> PQGen[None]:
         yield from self._check_intrans_gen("read_only")
-        self._read_only = bool(value)
+        self._read_only = bool(value) if value is not None else None
         self._begin_statement = b""
 
     @property
@@ -250,7 +250,7 @@ class BaseConnection(Generic[Row]):
 
     def _set_deferrable_gen(self, value: Optional[bool]) -> PQGen[None]:
         yield from self._check_intrans_gen("deferrable")
-        self._deferrable = bool(value)
+        self._deferrable = bool(value) if value is not None else None
         self._begin_statement = b""
 
     def _check_intrans_gen(self, attribute: str) -> PQGen[None]:
diff --git a/tests/test_connection.py b/tests/test_connection.py
index 57c6c788f..1cfeea4aa 100644
--- a/tests/test_connection.py
+++ b/tests/test_connection.py
@@ -582,22 +582,26 @@ class ParamDef:
     name: str
     guc: str
     values: List[Any]
+    non_default: str
 
 
 param_isolation = ParamDef(
     name="isolation_level",
     guc="isolation",
     values=list(psycopg.IsolationLevel),
+    non_default="serializable",
 )
 param_read_only = ParamDef(
     name="read_only",
     guc="read_only",
     values=[True, False],
+    non_default="on",
 )
 param_deferrable = ParamDef(
     name="deferrable",
     guc="deferrable",
     values=[True, False],
+    non_default="on",
 )
 
 # Map Python values to Postgres values for the tx_params possible values
@@ -655,6 +659,30 @@ def test_set_transaction_param_implicit(conn, param, autocommit):
         conn.rollback()
 
 
+@pytest.mark.parametrize("param", tx_params_isolation)
+def test_set_transaction_param_reset(conn, param):
+    conn.execute(
+        "select set_config(%s, %s, false)",
+        [f"default_transaction_{param.guc}", param.non_default],
+    )
+    conn.commit()
+
+    for value in param.values:
+        setattr(conn, param.name, value)
+        (pgval,) = conn.execute(
+            "select current_setting(%s)", [f"transaction_{param.guc}"]
+        ).fetchone()
+        assert tx_values_map[pgval] == value
+        conn.rollback()
+
+        setattr(conn, param.name, None)
+        (pgval,) = conn.execute(
+            "select current_setting(%s)", [f"transaction_{param.guc}"]
+        ).fetchone()
+        assert tx_values_map[pgval] == tx_values_map[param.non_default]
+        conn.rollback()
+
+
 @pytest.mark.parametrize("autocommit", [True, False])
 @pytest.mark.parametrize("param", tx_params_isolation)
 def test_set_transaction_param_block(conn, param, autocommit):
diff --git a/tests/test_connection_async.py b/tests/test_connection_async.py
index fe1a403f1..61277872f 100644
--- a/tests/test_connection_async.py
+++ b/tests/test_connection_async.py
@@ -622,6 +622,32 @@ async def test_set_transaction_param_implicit(aconn, param, autocommit):
         await aconn.rollback()
 
 
+@pytest.mark.parametrize("param", tx_params_isolation)
+async def test_set_transaction_param_reset(aconn, param):
+    await aconn.execute(
+        "select set_config(%s, %s, false)",
+        [f"default_transaction_{param.guc}", param.non_default],
+    )
+    await aconn.commit()
+
+    for value in param.values:
+        await getattr(aconn, f"set_{param.name}")(value)
+        cur = await aconn.execute(
+            "select current_setting(%s)", [f"transaction_{param.guc}"]
+        )
+        (pgval,) = await cur.fetchone()
+        assert tx_values_map[pgval] == value
+        await aconn.rollback()
+
+        await getattr(aconn, f"set_{param.name}")(None)
+        cur = await aconn.execute(
+            "select current_setting(%s)", [f"transaction_{param.guc}"]
+        )
+        (pgval,) = await cur.fetchone()
+        assert tx_values_map[pgval] == tx_values_map[param.non_default]
+        await aconn.rollback()
+
+
 @pytest.mark.parametrize("autocommit", [True, False])
 @pytest.mark.parametrize("param", tx_params_isolation)
 async def test_set_transaction_param_block(aconn, param, autocommit):