Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion web/pgadmin/tools/sqleditor/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
13 changes: 13 additions & 0 deletions web/pgadmin/utils/driver/psycopg3/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 <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 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(
Expand Down
Original file line number Diff line number Diff line change
@@ -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 <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)."""

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)
Loading