From: Daniele Varrazzo <daniele.varrazzo@gmail.com>
Date: Mon, 9 May 2022 13:36:19 +0000 (+0200)
Subject: fix: fix Transaction commit in pipeline mode
X-Git-Tag: 3.1~113^2~6
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7a4061439d5987e1162e1f42110427e9902adc60;p=thirdparty%2Fpsycopg.git

fix: fix Transaction commit in pipeline mode

Make sure we receive the commit message, so we can raise an immediate
exception, as we do for connection-handled transactions.

Highlight, in the test, a behaviour difference between commit in
pipeline and non-pipeline mode. In the latter, after a failed commit,
the connection is left IDLE; in pipeline mode it is left INERROR so we
need a further rollback to keep on using it. Will look if this can be
made consistent.
---

diff --git a/psycopg/psycopg/transaction.py b/psycopg/psycopg/transaction.py
index 3477f8cfe..be46ead97 100644
--- a/psycopg/psycopg/transaction.py
+++ b/psycopg/psycopg/transaction.py
@@ -138,6 +138,9 @@ class BaseTransaction(Generic[ConnectionType]):
         for command in self._get_commit_commands():
             yield from self._conn._exec_command(command)
 
+        if self._conn._pipeline:
+            yield from self._conn._pipeline._sync_gen()
+
     def _rollback_gen(self, exc_val: Optional[BaseException]) -> PQGen[bool]:
         if isinstance(exc_val, Rollback):
             logger.debug(f"{self._conn}: Explicit rollback from: ", exc_info=True)
diff --git a/tests/test_pipeline.py b/tests/test_pipeline.py
index b20359f8c..d8783f35b 100644
--- a/tests/test_pipeline.py
+++ b/tests/test_pipeline.py
@@ -222,8 +222,41 @@ def test_errors_raised_on_commit(conn):
         conn.execute("select 1 from nosuchtable")
         with pytest.raises(e.UndefinedTable):
             conn.commit()
-        conn.rollback()
+        conn.rollback()  # TODO: inconsistent with non-pipeline.
+        cur1 = conn.execute("select 1")
+    cur2 = conn.execute("select 2")
+
+    assert cur1.fetchone() == (1,)
+    assert cur2.fetchone() == (2,)
+
+
+def test_errors_raised_on_transaction_exit(conn):
+    here = False
+    with conn.pipeline():
+        with pytest.raises(e.UndefinedTable):
+            with conn.transaction():
+                conn.execute("select 1 from nosuchtable")
+                here = True
+        conn.rollback()  # TODO: inconsistent with non-pipeline.
+        cur1 = conn.execute("select 1")
+    assert here
+    cur2 = conn.execute("select 2")
+
+    assert cur1.fetchone() == (1,)
+    assert cur2.fetchone() == (2,)
+
+
+def test_errors_raised_on_nested_transaction_exit(conn):
+    here = False
+    with conn.pipeline():
+        with pytest.raises(e.UndefinedTable):
+            with conn.transaction():
+                with conn.transaction():
+                    conn.execute("select 1 from nosuchtable")
+                    here = True
+        conn.rollback()  # TODO: inconsistent with non-pipeline.
         cur1 = conn.execute("select 1")
+    assert here
     cur2 = conn.execute("select 2")
 
     assert cur1.fetchone() == (1,)
diff --git a/tests/test_pipeline_async.py b/tests/test_pipeline_async.py
index 5d7e669dd..c69fef788 100644
--- a/tests/test_pipeline_async.py
+++ b/tests/test_pipeline_async.py
@@ -225,8 +225,41 @@ async def test_errors_raised_on_commit(aconn):
         await aconn.execute("select 1 from nosuchtable")
         with pytest.raises(e.UndefinedTable):
             await aconn.commit()
-        await aconn.rollback()
+        await aconn.rollback()  # TODO: inconsistent with non-pipeline.
+        cur1 = await aconn.execute("select 1")
+    cur2 = await aconn.execute("select 2")
+
+    assert await cur1.fetchone() == (1,)
+    assert await cur2.fetchone() == (2,)
+
+
+async def test_errors_raised_on_transaction_exit(aconn):
+    here = False
+    async with aconn.pipeline():
+        with pytest.raises(e.UndefinedTable):
+            async with aconn.transaction():
+                await aconn.execute("select 1 from nosuchtable")
+                here = True
+        await aconn.rollback()  # TODO: inconsistent with non-pipeline.
+        cur1 = await aconn.execute("select 1")
+    assert here
+    cur2 = await aconn.execute("select 2")
+
+    assert await cur1.fetchone() == (1,)
+    assert await cur2.fetchone() == (2,)
+
+
+async def test_errors_raised_on_nested_transaction_exit(aconn):
+    here = False
+    async with aconn.pipeline():
+        with pytest.raises(e.UndefinedTable):
+            async with aconn.transaction():
+                async with aconn.transaction():
+                    await aconn.execute("select 1 from nosuchtable")
+                    here = True
+        await aconn.rollback()  # TODO: inconsistent with non-pipeline.
         cur1 = await aconn.execute("select 1")
+    assert here
     cur2 = await aconn.execute("select 2")
 
     assert await cur1.fetchone() == (1,)