From b0e35fd3a4624a70dcfbcaf79f55e00b73422a39 Mon Sep 17 00:00:00 2001 From: Dave Page Date: Wed, 10 Jun 2026 15:25:03 +0100 Subject: [PATCH 1/2] Fix View/Edit Data crash on a stale/non-filter session transaction object initialize_viewdata restores the filter and data-sorting from any command object previously stored in session['gridData'] under the same trans_id. It assumed that object was always a filter-capable (View/Edit Data) command and accessed old_trans_obj._row_filter / ._data_sorting directly. That assumption is wrong: the same trans_id may have been used by the Query Tool (a QueryToolCommand, which does not inherit SQLFilter), or the session may contain an incompatible object persisted by an older version after an upgrade. In those cases the attribute access raised AttributeError, returning a 500 from the endpoint and - in desktop mode, where this runs during startup - preventing the application from loading at all. Guard the restore with isinstance(old_trans_obj, SQLFilter) (short-circuited before the did/obj_id checks) so a non-filter object is simply skipped. Add a regression test that seeds a pickled QueryToolCommand under the trans_id and asserts initialize/viewdata returns 200. Closes #9744 --- docs/en_US/release_notes_9_16.rst | 1 + web/pgadmin/tools/sqleditor/__init__.py | 10 +- .../test_view_data_restore_stale_trans.py | 112 ++++++++++++++++++ 3 files changed, 122 insertions(+), 1 deletion(-) create mode 100644 web/pgadmin/tools/sqleditor/tests/test_view_data_restore_stale_trans.py diff --git a/docs/en_US/release_notes_9_16.rst b/docs/en_US/release_notes_9_16.rst index 4f0740cdff0..edb115128d5 100644 --- a/docs/en_US/release_notes_9_16.rst +++ b/docs/en_US/release_notes_9_16.rst @@ -42,6 +42,7 @@ Bug fixes | `Issue #6308 `_ - Fix the infinite loading spinner after an idle database connection is silently dropped, by detecting stale connections and offering a reconnect dialog. | `Issue #9595 `_ - Fix missing ALTER ... SET DEFAULT statements for inherited columns in the generated table SQL/EDIT script. | `Issue #9677 `_ - Fix the Unlogged table toggle in table properties not generating any ALTER TABLE ... SET LOGGED/UNLOGGED statement. + | `Issue #9744 `_ - Fix a View/Edit Data crash when the session contains a transaction object that is not filter-capable (e.g. left by the Query Tool or persisted by an older version), which could prevent the desktop application from loading after an upgrade. | `Issue #9828 `_ - Fix tool calls failing against OpenAI-compatible providers that emit empty/null name, arguments, or id fields in streaming continuation deltas. | `Issue #9875 `_ - Fixed an issue where EXPLAIN and EXPLAIN ANALYZE failed to execute when blank lines separated clauses in the SQL query. | `Issue #9810 `_ - Use the ServerManager's passfile for the credential gate in connect() so the check matches the passfile actually used for the connection, and warn on conflicting passfile/passexec settings. diff --git a/web/pgadmin/tools/sqleditor/__init__.py b/web/pgadmin/tools/sqleditor/__init__.py index c9e26df2f00..4d7a6bf07c7 100644 --- a/web/pgadmin/tools/sqleditor/__init__.py +++ b/web/pgadmin/tools/sqleditor/__init__.py @@ -319,7 +319,15 @@ def initialize_viewdata(trans_id, cmd_type, obj_type, sgid, sid, did, obj_id): if str(trans_id) in sql_grid_data: old_trans_obj = pickle.loads( sql_grid_data[str(trans_id)]['command_obj']) - if old_trans_obj.did == did and old_trans_obj.obj_id == obj_id: + # Only restore the filter/sorting when the previously stored object is + # a filter-capable (View/Edit Data) command. The same trans_id may + # have been used by a non-filter command such as the Query Tool, or by + # an incompatible object persisted by an older version - neither + # carries the _row_filter/_data_sorting attributes, and blindly + # accessing them raises an AttributeError that prevents the tool (and, + # in desktop mode, the application) from loading. + if isinstance(old_trans_obj, SQLFilter) and \ + old_trans_obj.did == did and old_trans_obj.obj_id == obj_id: command_obj.set_filter(old_trans_obj._row_filter) command_obj.set_data_sorting( dict(data_sorting=old_trans_obj._data_sorting), True) diff --git a/web/pgadmin/tools/sqleditor/tests/test_view_data_restore_stale_trans.py b/web/pgadmin/tools/sqleditor/tests/test_view_data_restore_stale_trans.py new file mode 100644 index 00000000000..af650db6423 --- /dev/null +++ b/web/pgadmin/tools/sqleditor/tests/test_view_data_restore_stale_trans.py @@ -0,0 +1,112 @@ +########################################################################## +# +# pgAdmin 4 - PostgreSQL Tools +# +# Copyright (C) 2013 - 2026, The pgAdmin Development Team +# This software is released under the PostgreSQL Licence +# +########################################################################## + +import uuid +import pickle +import secrets +from pgadmin.utils.route import BaseTestGenerator +from pgadmin.browser.server_groups.servers.databases.tests import utils as \ + database_utils +from regression import parent_node_dict +from regression.python_test_utils import test_utils +from pgadmin.tools.sqleditor.command import QueryToolCommand + + +class TestViewDataRestoreStaleTransObj(BaseTestGenerator): + """ + Regression test for issue #9744. + + When View/Edit Data initialises a transaction it tries to restore the + filter/sorting from any command object previously stored in the session + under the same trans_id. That stored object is not always a filter-capable + (View/Edit Data) command - the same trans_id may have been used by the + Query Tool, or the session may contain an incompatible object persisted by + an older version after an upgrade. In those cases the object has no + _row_filter/_data_sorting attributes, and blindly accessing them raised an + AttributeError that returned a 500 and, in desktop mode, prevented the + application from loading. + + This test seeds the session with a pickled QueryToolCommand (which is what + a restored Query Tool transaction leaves in session['gridData']) under the + trans_id used to initialise View/Edit Data, and asserts the request + succeeds instead of crashing. + """ + scenarios = [ + ('Initialize View/Edit Data over a stale non-filter trans object', + dict()) + ] + + def setUp(self): + self.server_id = self.server_information['server_id'] + self.database_info = parent_node_dict["database"][-1] + self.db_name = self.database_info["db_name"] + self.db_id = self.database_info["db_id"] + + self.connection = test_utils.get_db_connection( + self.db_name, + self.server['username'], + self.server['db_password'], + self.server['host'], + self.server['port'] + ) + + db_con = database_utils.connect_database(self, test_utils.SERVER_GROUP, + self.server_id, self.db_id) + if not db_con["info"] == "Database connected.": + raise Exception("Could not connect to the database.") + + def runTest(self): + self.table = "test_table_%s" % (str(uuid.uuid4())[1:8]) + table_sql = """Create Table %s( + id integer Not Null, + Constraint table_pk Primary Key(id) + );""" % self.table + test_utils.create_table_with_query(self.server, self.db_name, + table_sql) + + # Fetch the table OID + pg_cursor = self.connection.cursor() + pg_cursor.execute("""Select oid FROM pg_catalog.pg_class WHERE + relname = '%s' AND relkind IN ('r','s','t')""" % self.table) + table_id = pg_cursor.fetchall()[0][0] + + trans_id = str(secrets.choice(range(1, 9999999))) + + # Build a QueryToolCommand - the kind of object the Query Tool stores + # in session['gridData'] - and give it the same did/obj_id as the + # table so the restore guard in initialize_viewdata is reached. A + # QueryToolCommand has no _row_filter attribute, which is what + # previously triggered the AttributeError. + stale_obj = QueryToolCommand( + sgid=test_utils.SERVER_GROUP, sid=self.server_id, did=self.db_id) + stale_obj.obj_id = table_id + + with self.tester.session_transaction() as sess: + grid_data = sess.get('gridData', {}) + grid_data[trans_id] = { + 'command_obj': pickle.dumps(stale_obj, -1) + } + sess['gridData'] = grid_data + + url = '/sqleditor/initialize/viewdata/{0}/3/table/{1}/{2}/{3}/{4}' \ + .format(trans_id, test_utils.SERVER_GROUP, self.server_id, + self.db_id, table_id) + response = self.tester.post(url) + + # Before the fix this returned 500 with: + # 'QueryToolCommand' object has no attribute '_row_filter' + self.assertEqual(response.status_code, 200) + + def tearDown(self): + self.connection.cursor().execute( + "DROP TABLE IF EXISTS %s" % self.table) + self.connection.commit() + self.connection.close() + database_utils.disconnect_database(self, self.server_id, + self.db_id) From 65c3027658fa8c5d4e56cbc36284032c246ac642 Mon Sep 17 00:00:00 2001 From: Dave Page Date: Wed, 10 Jun 2026 15:41:43 +0100 Subject: [PATCH 2/2] test: avoid hard-coded PK constraint name colliding across the suite The regression test created its table with a fixed 'table_pk' primary key constraint name, which collides with the same name used by TestViewData in this package when the full suite runs against one database. That made the CREATE TABLE fail in CI (table not found -> IndexError on the OID lookup). Let PostgreSQL auto-name the primary key instead. --- .../sqleditor/tests/test_view_data_restore_stale_trans.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/web/pgadmin/tools/sqleditor/tests/test_view_data_restore_stale_trans.py b/web/pgadmin/tools/sqleditor/tests/test_view_data_restore_stale_trans.py index af650db6423..8b4fe3d8add 100644 --- a/web/pgadmin/tools/sqleditor/tests/test_view_data_restore_stale_trans.py +++ b/web/pgadmin/tools/sqleditor/tests/test_view_data_restore_stale_trans.py @@ -63,9 +63,12 @@ def setUp(self): def runTest(self): self.table = "test_table_%s" % (str(uuid.uuid4())[1:8]) + # Note: do not give the primary key an explicit constraint name - + # other tests in this package create tables in the same database, and + # a hard-coded constraint name would collide across the suite. Letting + # PostgreSQL auto-name it (_pkey) keeps it unique. table_sql = """Create Table %s( - id integer Not Null, - Constraint table_pk Primary Key(id) + id integer Not Null Primary Key );""" % self.table test_utils.create_table_with_query(self.server, self.db_name, table_sql)