From 693aa971d1c5629e645736332a0a6d3a55383f15 Mon Sep 17 00:00:00 2001 From: Kundan Sable Date: Wed, 22 Jul 2026 14:20:00 +0530 Subject: [PATCH 1/3] fix: exclude non-editable/alias columns from Query Tool row UPDATE The added (INSERT) branch of save_changed_data() already filtered out columns that aren't backed by a real table column (added for #9939 / #10015), but the updated (UPDATE) branch did not. Editing a row that includes an aliased/expression column (e.g. first_name || ' ' || last_name AS the_name) sent that alias into the generated UPDATE's SET clause, which fails with "column ... does not exist" even though the grid already marks the column read-only. Apply the same editable-columns guard to the updated branch, and skip the row entirely if nothing editable remains after filtering. Fixes #10103 --- .../tools/sqleditor/utils/save_changed_data.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/web/pgadmin/tools/sqleditor/utils/save_changed_data.py b/web/pgadmin/tools/sqleditor/utils/save_changed_data.py index 78776697ff3..3965daa9aa9 100644 --- a/web/pgadmin/tools/sqleditor/utils/save_changed_data.py +++ b/web/pgadmin/tools/sqleditor/utils/save_changed_data.py @@ -177,6 +177,21 @@ def save_changed_data(changed_data, columns_info, conn, command_obj, list_of_sql[of_type] = [] for each_row in changed_data[of_type]: data = changed_data[of_type][each_row]['data'] + # Drop any column that isn't a real editable column of the + # underlying table (e.g. an expression/alias column such as + # `first_name || ' ' || last_name as the_name`). Such columns + # carry the read-only lock icon in the grid, but without this + # guard the rendered UPDATE references a non-existent column + # and Postgres rejects the change. Issue #10103. + data = { + k: v for k, v in data.items() + if k in columns_info and + columns_info[k].get('is_editable', True) + } + # Nothing editable left to persist for this row, skip it so we + # don't render an invalid `SET` clause. + if not data: + continue pk_escaped = { pk: pk_val.replace('%', '%%') if hasattr( pk_val, 'replace') else pk_val From fceffbf2f8c50134a6f37c871270b83892ac2345 Mon Sep 17 00:00:00 2001 From: Kundan Sable Date: Fri, 24 Jul 2026 19:40:27 +0530 Subject: [PATCH 2/3] fix: address review feedback on updated-row column filtering Per @asheshv's review on PR #10171: 1. The added-row path explicitly pops the client_primary_key/ is_row_copied tracking keys before filtering to editable columns; the updated-row path relied implicitly on the columns_info filter to drop them, since neither key is ever present on an updated-row payload today. Mirror the explicit pops for symmetry, so this path isn't silently relying on that assumption if a future change starts tagging updated rows the same way (e.g. multi-row copy-paste into existing rows). 2. 'row_id' was read via data.get(client_primary_key) on the already-filtered dict, so it was always None - dead weight now, and actively wrong once point 1 makes the filtering explicit rather than incidental. Capture row_id from the row's data before it's popped/filtered, so a failed update's error report can identify which row failed. --- .../sqleditor/utils/save_changed_data.py | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/web/pgadmin/tools/sqleditor/utils/save_changed_data.py b/web/pgadmin/tools/sqleditor/utils/save_changed_data.py index 3965daa9aa9..adf8999c918 100644 --- a/web/pgadmin/tools/sqleditor/utils/save_changed_data.py +++ b/web/pgadmin/tools/sqleditor/utils/save_changed_data.py @@ -177,6 +177,22 @@ def save_changed_data(changed_data, columns_info, conn, command_obj, list_of_sql[of_type] = [] for each_row in changed_data[of_type]: data = changed_data[of_type][each_row]['data'] + # client_primary_key is a synthetic tracking key (e.g. + # '__temp_PK') chosen specifically to never match a real + # column name, so it must be read before it is popped/ + # filtered out below. + row_id = data.get(client_primary_key) + # Remove our unique tracking keys, mirroring the + # added-row path above. Today neither key is ever + # present on an updated-row payload (only the added-row + # path tags rows this way), so the columns_info filter + # below is already sufficient - but stripping them + # explicitly keeps this path from silently relying on + # that assumption if a future change starts tagging + # updated rows the same way (e.g. multi-row copy-paste + # into existing rows). + data.pop(client_primary_key, None) + data.pop('is_row_copied', None) # Drop any column that isn't a real editable column of the # underlying table (e.g. an expression/alias column such as # `first_name || ' ' || last_name as the_name`). Such columns @@ -211,8 +227,7 @@ def save_changed_data(changed_data, columns_info, conn, command_obj, ) list_of_sql[of_type].append({'sql': sql, 'data': data, - 'row_id': - data.get(client_primary_key)}) + 'row_id': row_id}) # For deleted rows elif of_type == 'deleted': From b229b94d358ff32961741cf174127c6260133262 Mon Sep 17 00:00:00 2001 From: Dave Page Date: Tue, 18 Aug 2026 11:56:50 +0100 Subject: [PATCH 3/3] fix: refuse edits to read-only columns in the Query Tool grid The backend guard in the previous commit stops an expression or alias column from reaching the generated UPDATE, but the edit should never have been staged in the first place: the grid was happy to accept it, so the user got an error (or, with the guard in place, a save that silently did nothing) for a column the header already marks with a lock icon. The editors deliberately open on read-only columns so that wide text and JSON values can be inspected, and the OK button is hidden there, but `TextEditor` committed on Enter regardless because its keydown handler called `onOK()` without consulting `column.can_edit`. `onRowsChange` in `ResultSet` then had no guard of its own, so the change was both applied to the grid and staged for saving. Both now refuse a change to a column whose `can_edit` is false, which leaves the cell showing its real value rather than one that was never going to be written. Also adds the regression coverage the updated-row filter was missing: a Jest spec for the Enter-key path in `TextEditor`, and a Python `TestSaveUpdatedRowSkipsNonEditableColumn` mirroring the existing added-row class, covering both an update that carries an alias alongside a real column and one that carries nothing but an alias. --- .../components/QueryToolDataGrid/Editors.jsx | 9 ++ .../js/components/sections/ResultSet.jsx | 9 ++ .../utils/tests/test_save_changed_data.py | 111 ++++++++++++++++++ .../sqleditor/text_editor_readonly.spec.js | 80 +++++++++++++ 4 files changed, 209 insertions(+) create mode 100644 web/regression/javascript/sqleditor/text_editor_readonly.spec.js diff --git a/web/pgadmin/tools/sqleditor/static/js/components/QueryToolDataGrid/Editors.jsx b/web/pgadmin/tools/sqleditor/static/js/components/QueryToolDataGrid/Editors.jsx index 355bc009519..0d298bb84ac 100644 --- a/web/pgadmin/tools/sqleditor/static/js/components/QueryToolDataGrid/Editors.jsx +++ b/web/pgadmin/tools/sqleditor/static/js/components/QueryToolDataGrid/Editors.jsx @@ -241,6 +241,15 @@ export function TextEditor({row, column, onRowChange, onClose}) { }, []); const onOK = ()=>{ + /* The editor also opens on read-only columns so that wide values can be + * inspected, but committing must be refused there: an expression/alias + * column has no counterpart in the base table and the generated UPDATE + * would fail with `column "..." does not exist` (#10103). The OK button + * is already hidden, this guards the Enter key path. */ + if(!column.can_edit) { + onClose(false); + return; + } if(column.is_array && !isValidArray(localVal)) { pgAdmin.Browser.notifier.error(gettext('Arrays must start with "{" and end with "}"')); } else { diff --git a/web/pgadmin/tools/sqleditor/static/js/components/sections/ResultSet.jsx b/web/pgadmin/tools/sqleditor/static/js/components/sections/ResultSet.jsx index 639bd39598c..10687f4c8bf 100644 --- a/web/pgadmin/tools/sqleditor/static/js/components/sections/ResultSet.jsx +++ b/web/pgadmin/tools/sqleditor/static/js/components/sections/ResultSet.jsx @@ -1621,6 +1621,15 @@ export function ResultSet() { const onRowsChange = (newRows, otherInfo)=>{ + /* Never record, or even display, a change to a read-only column. The + * editors open on such columns so that wide values can be inspected, so a + * stray commit can still arrive here; letting it through would stage an + * expression/alias column that does not exist in the base table and the + * save would fail with `column "..." does not exist` (#10103). */ + if(otherInfo.column?.can_edit === false) { + return; + } + let row = newRows[otherInfo.indexes[0]]; let clientPK = rowKeyGetter(row); diff --git a/web/pgadmin/tools/sqleditor/utils/tests/test_save_changed_data.py b/web/pgadmin/tools/sqleditor/utils/tests/test_save_changed_data.py index b43d0773a9a..db8a4434245 100644 --- a/web/pgadmin/tools/sqleditor/utils/tests/test_save_changed_data.py +++ b/web/pgadmin/tools/sqleditor/utils/tests/test_save_changed_data.py @@ -1040,3 +1040,114 @@ def _create_test_table(self): "FROM {0};" ).format(self.test_table_name) utils.create_table_with_query(self.server, self.db_name, create_sql) + + +# The result set used by both alias regression classes below: three real +# columns of the base table plus an expression column that only exists in +# the query. ``can_edit`` mirrors what the client sends for the lock icon. +ALIAS_RESULT_COLUMNS = [ + {"name": "id", "pos": 0, "can_edit": True, + "type": "integer", "cell": "number", + "not_null": True, "has_default_val": False, + "is_array": False, "display_name": "id"}, + {"name": "first_name", "pos": 1, "can_edit": True, + "type": "text", "cell": "string", + "not_null": False, "has_default_val": False, + "is_array": False, "display_name": "first_name"}, + {"name": "last_name", "pos": 2, "can_edit": True, + "type": "text", "cell": "string", + "not_null": False, "has_default_val": False, + "is_array": False, "display_name": "last_name"}, + {"name": "the_name", "pos": 3, "can_edit": False, + "type": "text", "cell": "string", + "not_null": False, "has_default_val": False, + "is_array": False, "display_name": "the_name"}, +] + + +class TestSaveUpdatedRowSkipsNonEditableColumn(TestSaveChangedData): + """Regression test for issue #10103. + + The counterpart of :class:`TestSaveAddedRowSkipsNonEditableColumn` for + the UPDATE path. An edit staged against an expression or alias column + must be dropped before rendering the UPDATE, because the alias is not a + real column of the underlying table and PostgreSQL would reject the + statement with ``column "the_name" does not exist``. If nothing + editable is left once the alias has been dropped, no UPDATE should be + rendered at all. + """ + + scenarios = [ + ('Update carrying an alias alongside a real column', dict( + save_payload={ + "updated": { + "1": { + "err": False, + "data": { + "first_name": "Jane", + # The alias must be ignored rather than + # written to the base table. + "the_name": "Jane Doe" + }, + "primary_keys": {"id": 1} + } + }, + "added": {}, + "staged_rows": {}, + "deleted": {}, + "updated_index": {}, + "added_index": {}, + "columns": ALIAS_RESULT_COLUMNS + }, + save_status=True, + check_sql='SELECT id, first_name, last_name ' + 'FROM %s WHERE id = 1', + check_result=[[1, "Jane", "Doe"]] + )), + ('Update carrying nothing but an alias', dict( + save_payload={ + "updated": { + "1": { + "err": False, + "data": { + "the_name": "Jane Doe" + }, + "primary_keys": {"id": 1} + } + }, + "added": {}, + "staged_rows": {}, + "deleted": {}, + "updated_index": {}, + "added_index": {}, + "columns": ALIAS_RESULT_COLUMNS + }, + save_status=True, + # Nothing editable remains, so no UPDATE is rendered and the + # row is left exactly as it was. + check_sql='SELECT id, first_name, last_name ' + 'FROM %s WHERE id = 1', + check_result=[[1, "John", "Doe"]] + )), + ] + + def _create_test_table(self): + self.test_table_name = "test_for_save_data_alias_upd_" + \ + str(secrets.choice(range(1000, 9999))) + create_sql = """ + DROP TABLE IF EXISTS "{0}"; + + CREATE TABLE "{0}"( + id INT PRIMARY KEY, + first_name TEXT, + last_name TEXT + ); + + INSERT INTO "{0}" VALUES (1, 'John', 'Doe'); + """.format(self.test_table_name) + self.select_sql = ( + "SELECT id, first_name, last_name, " + "first_name || ' ' || last_name AS the_name " + "FROM {0};" + ).format(self.test_table_name) + utils.create_table_with_query(self.server, self.db_name, create_sql) diff --git a/web/regression/javascript/sqleditor/text_editor_readonly.spec.js b/web/regression/javascript/sqleditor/text_editor_readonly.spec.js new file mode 100644 index 00000000000..55f138445cf --- /dev/null +++ b/web/regression/javascript/sqleditor/text_editor_readonly.spec.js @@ -0,0 +1,80 @@ +///////////////////////////////////////////////////////////// +// +// pgAdmin 4 - PostgreSQL Tools +// +// Copyright (C) 2013 - 2026, The pgAdmin Development Team +// This software is released under the PostgreSQL Licence +// +////////////////////////////////////////////////////////////// + +import { render, screen, fireEvent } from '@testing-library/react'; + +// Stub the heavy JSON editor so importing Editors does not pull in CodeMirror. +jest.mock('../../../pgadmin/static/js/components/JsonEditor', () => ({ + __esModule: true, + default: () =>
, +})); + +// Mock the QueryToolDataGrid index so importing Editors does not pull in the +// whole data grid; Editors only needs RowInfoContext from it. +jest.mock('../../../pgadmin/tools/sqleditor/static/js/components/QueryToolDataGrid', () => { + const ReactActual = require('react'); + return { RowInfoContext: ReactActual.createContext() }; +}); + +import Theme from 'sources/Theme'; +import { TextEditor } from '../../../pgadmin/tools/sqleditor/static/js/components/QueryToolDataGrid/Editors'; +import { RowInfoContext } from '../../../pgadmin/tools/sqleditor/static/js/components/QueryToolDataGrid'; +import { PgAdminProvider } from '../../../pgadmin/static/js/PgAdminProvider'; + +describe('QueryToolDataGrid TextEditor read-only columns', () => { + const KEY = 'the_name'; + let onRowChange, onClose; + + const renderEditor = (canEdit) => { + const pgAdmin = { Browser: { notifier: { error: jest.fn() } } }; + return render( + + + null }}> + + + + + ); + }; + + const editAndPressEnter = () => { + const textarea = screen.getByRole('textbox'); + fireEvent.change(textarea, { target: { value: 'Jane Doe' } }); + fireEvent.keyDown(textarea, { keyCode: 13 }); + }; + + beforeEach(() => { + onRowChange = jest.fn(); + onClose = jest.fn(); + }); + + it('hides the OK button on a read-only column', () => { + renderEditor(false); + expect(screen.queryByText('OK')).not.toBeInTheDocument(); + }); + + it('refuses the Enter-key commit on a read-only column (#10103)', () => { + renderEditor(false); + editAndPressEnter(); + expect(onRowChange).not.toHaveBeenCalled(); + expect(onClose).toHaveBeenCalledWith(false); + }); + + it('still commits the Enter-key edit on an editable column', () => { + renderEditor(true); + editAndPressEnter(); + expect(onRowChange).toHaveBeenCalledWith({ [KEY]: 'Jane Doe' }, true); + }); +});