From bd956835eaa21e521773a70f38cd779a37000235 Mon Sep 17 00:00:00 2001 From: Dave Page Date: Wed, 19 Aug 2026 12:39:43 +0100 Subject: [PATCH] fix: run BEGIN/COMMIT/ROLLBACK on a plain cursor under server cursor mode (#8991) execute_void() blindly reused whatever cursor was cached for the connection, which under "server cursor" mode is the named/server-side AsyncDictServerCursor left over from the last SELECT. A named cursor's execute() always wraps the statement as `DECLARE ... CURSOR FOR `, which cannot express a transaction-control statement, so BEGIN/COMMIT/ROLLBACK silently failed (failing one step earlier still, on a `prepare` keyword the server-side cursor's execute() doesn't accept at all) and the exception was swallowed by the background query thread. The transaction was therefore never actually committed or rolled back, and the next poll() picked up the previous query's leftover column info, which is what made the result grid appear instead of the Messages tab. Run the statement through a throwaway plain cursor instead, leaving the cached server-side cursor untouched, and clear the stale column info so poll() correctly reports no result set. --- .../utils/driver/psycopg3/connection.py | 13 +++ .../tests/test_execute_void_server_cursor.py | 85 +++++++++++++++++++ 2 files changed, 98 insertions(+) create mode 100644 web/pgadmin/utils/driver/psycopg3/tests/test_execute_void_server_cursor.py diff --git a/web/pgadmin/utils/driver/psycopg3/connection.py b/web/pgadmin/utils/driver/psycopg3/connection.py index d07a16cefcd..d8a6cd53172 100644 --- a/web/pgadmin/utils/driver/psycopg3/connection.py +++ b/web/pgadmin/utils/driver/psycopg3/connection.py @@ -1173,6 +1173,19 @@ def execute_void(self, query, params=None, formatted_exception_msg=False): if not status: return False, str(cur) + + if isinstance(cur, AsyncDictServerCursor): + # A named/server-side cursor's execute() always runs the query + # as `DECLARE ... CURSOR FOR `, which cannot express a + # transaction-control statement such as BEGIN/COMMIT/ROLLBACK. + # Run this one statement through a throwaway plain cursor + # instead, leaving the cached server-side cursor untouched, and + # treat it as leaving no result set for whatever poll() call + # comes next. + cur = self.conn.cursor() + self.column_info = None + self.row_count = 0 + query_id = str(secrets.choice(range(1, 9999999))) current_app.logger.log( diff --git a/web/pgadmin/utils/driver/psycopg3/tests/test_execute_void_server_cursor.py b/web/pgadmin/utils/driver/psycopg3/tests/test_execute_void_server_cursor.py new file mode 100644 index 00000000000..c885f66df8e --- /dev/null +++ b/web/pgadmin/utils/driver/psycopg3/tests/test_execute_void_server_cursor.py @@ -0,0 +1,85 @@ +########################################################################## +# +# pgAdmin 4 - PostgreSQL Tools +# +# Copyright (C) 2013 - 2026, The pgAdmin Development Team +# This software is released under the PostgreSQL Licence +# +########################################################################## + +"""Regression test: ``execute_void()`` must not run a transaction-control +statement (BEGIN/COMMIT/ROLLBACK) through a cached named/server-side +cursor. + +A named cursor's ``execute()`` always wraps the statement as +``DECLARE ... CURSOR FOR ``, which cannot express BEGIN/COMMIT/ +ROLLBACK. Before the fix, the Commit/Rollback buttons under "server +cursor" mode silently did nothing: the DECLARE-wrapped call failed +(actually failing one step earlier, on a ``prepare`` keyword the +server-side cursor's ``execute()`` doesn't accept at all), the exception +was swallowed by the background query thread, and the next poll() then +reported the *previous* query's leftover column info, making the result +grid appear instead of the Messages tab (pgAdmin issue #8991).""" + +from unittest.mock import MagicMock, patch + +from pgadmin.utils.driver.psycopg3.connection import Connection +from pgadmin.utils.driver.psycopg3.cursor import AsyncDictServerCursor +from pgadmin.utils.route import BaseTestGenerator + + +class ExecuteVoidServerCursorTest(BaseTestGenerator): + + scenarios = [ + ('COMMIT with a cached server-side cursor runs on a throwaway ' + 'plain cursor and clears stale column info', dict(sql='COMMIT;')), + ('ROLLBACK with a cached server-side cursor runs on a throwaway ' + 'plain cursor and clears stale column info', + dict(sql='ROLLBACK;')), + ] + + def runTest(self): + manager = MagicMock(sid=1) + conn = Connection(manager, 'test-conn-id', 'testdb') + conn.python_encoding = 'utf-8' + + # Leftover state from a previous SELECT executed through the + # server-side cursor. + conn.column_info = [{'name': 'x'}] + conn.row_count = 1 + + server_cursor = MagicMock(spec=AsyncDictServerCursor) + server_cursor.closed = False + + plain_cursor = MagicMock() + plain_cursor.closed = False + + conn.conn = MagicMock() + conn.conn.cursor.return_value = plain_cursor + conn.conn.info.user = 'postgres' + conn.conn.info.host = 'localhost' + conn.conn.info.dbname = 'testdb' + + # current_user needs a real request context to resolve at all; + # patch it only once inside that context, to a stand-in with the + # attribute execute_void()'s log line reads. + with self.app.test_request_context(): + with patch( + 'pgadmin.utils.driver.psycopg3.connection.current_user', + MagicMock(email='test@example.com') + ), patch.object(Connection, '_Connection__cursor', + return_value=(True, server_cursor)): + status, result = conn.execute_void(self.sql) + + self.assertTrue(status) + self.assertIsNone(result) + + # The statement ran on the throwaway plain cursor, not the + # cached server-side one. + plain_cursor.execute.assert_called_once() + server_cursor.execute.assert_not_called() + + # Stale result-set state from the prior SELECT must not leak + # into whatever poll() call comes next. + self.assertIsNone(conn.column_info) + self.assertEqual(conn.row_count, 0)