Fix BEGIN/COMMIT/ROLLBACK silently failing under server cursor mode

web/pgadmin/tools/sqleditor/__init__.py

@@ -1150,7 +1150,8 @@ def poll(trans_id):
                 'transaction_status': transaction_status,
                 'explain_query_length':
                 get_explain_query_length(conn._Connection__async_cursor._query)
-                if conn._Connection__async_cursor else 0
+                if conn._Connection__async_cursor and
+                conn._Connection__async_cursor._query else 0
             }
             return internal_server_error(result, query_len_data)
         elif status == ASYNC_OK:

web/pgadmin/tools/sqleditor/tests/test_poll_explain_query_length_guard.py

@@ -0,0 +1,90 @@
+##########################################################################
+#
+# pgAdmin 4 - PostgreSQL Tools
+#
+# Copyright (C) 2013 - 2026, The pgAdmin Development Team
+# This software is released under the PostgreSQL Licence
+#
+##########################################################################
+
+"""Regression test for a review comment on PR #10321 (pgAdmin issue
+#8991): poll()'s error-handling branch built the 'explain_query_length'
+value with::
+
+    get_explain_query_length(conn._Connection__async_cursor._query)
+    if conn._Connection__async_cursor else 0
+
+which only guarded against the cached async cursor itself being falsy,
+not against its ``_query`` attribute being ``None``. PR #10321's own fix
+runs BEGIN/COMMIT/ROLLBACK through a throwaway plain cursor under
+"server cursor" mode; once that has happened the cached async cursor
+that poll() sees next can be a cursor that has not yet executed a real
+statement, so ``_query`` is still ``None``. get_explain_query_length()
+then immediately does ``query_obj.query.decode()``, and with
+``query_obj`` being ``None`` that crashes with::
+
+    AttributeError: 'NoneType' object has no attribute 'query'
+
+turning any query error that follows a commit under "server cursor"
+mode into an unhandled 500 and leaving the Query Tool unusable, instead
+of the normal JSON error response."""
+
+import json
+import secrets
+from unittest.mock import MagicMock, patch
+
+from pgadmin.utils.route import BaseTestGenerator
+
+
+class TestPollExplainQueryLengthGuard(BaseTestGenerator):
+    """poll() must not crash while building 'explain_query_length' when
+    the cached async cursor has not yet executed any statement."""
+
+    scenarios = [
+        ('Cached async cursor has not executed a statement yet '
+         '(_query is None) - poll() must not crash', dict())
+    ]
+
+    def runTest(self):
+        trans_id = secrets.choice(range(1, 9999999))
+
+        # A cursor left over from execute_void()'s throwaway plain
+        # cursor (or a freshly (re)created server-side cursor) that has
+        # not executed a real statement yet - exactly the state PR
+        # #10321's own fix can leave behind after a commit under
+        # "server cursor" mode.
+        async_cursor = MagicMock()
+        async_cursor._query = None
+
+        conn = MagicMock()
+        conn.poll.return_value = (False, 'some query error')
+        conn.connected.return_value = True
+        conn.messages.return_value = []
+        conn.transaction_status.return_value = 0
+        conn._Connection__async_cursor = async_cursor
+
+        trans_obj = MagicMock()
+        trans_obj.get_thread_native_id.return_value = None
+
+        session_obj = {}
+
+        with patch(
+            'pgadmin.tools.sqleditor.check_transaction_status',
+            return_value=(True, None, conn, trans_obj, session_obj)
+        ):
+            response = self.tester.get(
+                '/sqleditor/poll/{0}'.format(trans_id))
+
+        # Before the fix this either raised AttributeError outright, or
+        # (via the app's generic exception handler) came back as a 500
+        # whose errormsg was the raw AttributeError text instead of the
+        # intended query-error response.
+        response_text = response.data.decode('utf-8')
+        self.assertNotIn(
+            "'NoneType' object has no attribute 'query'", response_text)
+
+        response_data = json.loads(response_text)
+        self.assertEqual(response.status_code, 500)
+        self.assertEqual(response_data['errormsg'], 'some query error')
+        self.assertEqual(
+            response_data['data']['explain_query_length'], 0)

web/pgadmin/utils/driver/psycopg3/connection.py

@@ -1173,6 +1173,30 @@ 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 <query>`, which cannot express a
+            # transaction-control statement such as BEGIN/COMMIT/ROLLBACK.
+            # Run this one statement through a throwaway plain cursor
+            # instead, leaving the cursor cached for the connection in
+            # place for the next query to reuse.
+            cur = self.conn.cursor()
+            # The throwaway also has to become the async cursor, because
+            # poll() and status_message() report on that rather than on
+            # whatever this call used: the cached server-side cursor still
+            # describes the previous query and reports itself open, so a
+            # following poll() would read straight past its "not cur or
+            # cur.closed" guard and put that query's column metadata and
+            # row count back over the "no result set" a transaction
+            # control statement leaves behind. The connection's
+            # cursor_factory is AsyncDictCursor, so the throwaway carries
+            # ordered_description(), get_rowcount() and the rest of the
+            # API poll() calls.
+            self.__async_cursor = cur
+            self.column_info = None
+            self.row_count = 0
+
         query_id = str(secrets.choice(range(1, 9999999)))
 
         current_app.logger.log(

web/pgadmin/utils/driver/psycopg3/tests/test_execute_void_server_cursor.py

@@ -0,0 +1,140 @@
+##########################################################################
+#
+# 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 <query>``, 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).
+
+Clearing ``column_info``/``row_count`` in ``execute_void()`` is not enough
+on its own, because ``poll()`` rebuilds both from whatever
+``self.__async_cursor`` points at, and that is still the cached
+server-side cursor: it reports itself open, so the ``not cur or
+cur.closed`` guard lets it through and the previous query's metadata comes
+straight back. The throwaway cursor therefore has to become the async
+cursor as well, which also makes ``status_message()`` report the
+transaction-control statement rather than the previous query."""
+
+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 a following poll() reports no result set',
+         dict(sql='COMMIT;')),
+        ('ROLLBACK with a cached server-side cursor runs on a throwaway '
+         'plain cursor, and a following poll() reports no result set',
+         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
+
+        # The cursor the previous SELECT ran on, which is both cached for
+        # the connection and still referenced as the async cursor. It
+        # reports itself open, and still describes that SELECT's result.
+        stale_column = MagicMock()
+        stale_column.to_dict.return_value = {'name': 'x'}
+        server_cursor = MagicMock(spec=AsyncDictServerCursor)
+        server_cursor.closed = False
+        server_cursor.description = [stale_column]
+        server_cursor.ordered_description.return_value = [stale_column]
+        # AsyncDictServerCursor.get_rowcount() answers 1 unconditionally.
+        server_cursor.get_rowcount.return_value = 1
+        server_cursor.nextset.return_value = None
+        server_cursor.statusmessage = 'SELECT 1'
+        conn._Connection__async_cursor = server_cursor
+
+        # The throwaway cursor execute_void() should use instead. A
+        # transaction-control statement leaves no result set behind, so it
+        # has no description and no rows.
+        plain_cursor = MagicMock()
+        plain_cursor.closed = False
+        # Values taken from what psycopg actually leaves on the cursor
+        # after a COMMIT/ROLLBACK: no description, and a result with no
+        # tuples in it, which AsyncDictCursor.get_rowcount() reports as 0.
+        plain_cursor.description = None
+        plain_cursor.get_rowcount.return_value = 0
+        plain_cursor.nextset.return_value = None
+        plain_cursor.statusmessage = self.sql.rstrip(';')
+
+        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'
+        # Not ACTIVE, and no connection level error, so poll() gets as far
+        # as reading the cursor rather than answering from either of those.
+        conn.conn.info.transaction_status = 2
+        conn.conn.pgconn.error_message = None
+
+        # 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)
+
+        # ... and the poll() that the Query Tool makes next must not put it
+        # back. This is the call that made the result grid appear instead
+        # of the Messages tab, because it rebuilds column_info and
+        # row_count from the async cursor, which was still the server-side
+        # one describing the previous SELECT.
+        with self.app.test_request_context():
+            status, result = conn.poll(no_result=True)
+            status_message = conn.status_message()
+
+        self.assertEqual(status, 1)
+        self.assertIsNone(result)
+        self.assertIsNone(conn.column_info)
+        self.assertEqual(conn.row_count, 0)
+        server_cursor.ordered_description.assert_not_called()
+
+        # The status message belongs to the statement just run, not to the
+        # previous query.
+        self.assertEqual(status_message, self.sql.rstrip(';'))