Skip to content

Commit f9b9bc9

Browse files
asheshvdpage
authored andcommitted
fix(sqleditor): skip read-only columns when saving inserted rows (pgadmin-org#10015)
1 parent 39684cd commit f9b9bc9

5 files changed

Lines changed: 108 additions & 1 deletion

File tree

docs/en_US/release_notes_9_16.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,3 +35,4 @@ Bug fixes
3535
| `Issue #9896 <https://github.com/pgadmin-org/pgadmin4/issues/9896>`_ - Fix invalid DDL reconstruction for SERIAL columns in Schema Diff and the generated SQL/CREATE Script so the output round-trips on a clean target.
3636
| `Issue #9935 <https://github.com/pgadmin-org/pgadmin4/issues/9935>`_ - Fix "Illegal instruction" crash on startup of the Linux DEB and RPM packages on older x86_64 CPUs by pinning the psycopg C extension build to the x86-64 baseline.
3737
| `Issue #9936 <https://github.com/pgadmin-org/pgadmin4/issues/9936>`_ - Fix the AI panel silently falling back to the default provider when a custom LLM API URL or key file was set, and allow self-hosted LLM endpoints on any loopback port.
38+
| `Issue #9939 <https://github.com/pgadmin-org/pgadmin4/issues/9939>`_ - Fix saving a newly-added row in the Query Tool failing when the result set includes expression or alias columns that are not real columns of the underlying table.

web/pgadmin/tools/sqleditor/static/js/components/sections/ResultSet.jsx

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1257,8 +1257,17 @@ export function ResultSet() {
12571257
try {
12581258
/* Convert the added info to actual rows */
12591259
let added = {...dataChangeStore.added};
1260+
/* Strip read-only columns (expression/alias columns in Query Tool)
1261+
* from the new-row payload so the backend doesn't try to INSERT
1262+
* them into the base table. Issue #9939. */
1263+
let nonEditableKeys = new Set(
1264+
columns.filter((c)=>c.can_edit === false).map((c)=>c.key)
1265+
);
12601266
Object.keys(added).forEach((clientPK)=>{
1261-
added[clientPK].data = _.find(rows, (r)=>rowKeyGetter(r)==clientPK);
1267+
let rowData = _.find(rows, (r)=>rowKeyGetter(r)==clientPK);
1268+
added[clientPK].data = _.omitBy(
1269+
rowData, (_v, k)=>nonEditableKeys.has(k)
1270+
);
12621271
});
12631272
let {data: respData} = await rsu.current.saveData({
12641273
updated: dataChangeStore.updated,

web/pgadmin/tools/sqleditor/utils/get_column_types.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,11 @@ def get_columns_types(is_query_tool, columns_info, table_oid, conn, has_oids,
5050
col_type['type_name'] = None
5151
col_type['internal_size'] = col['internal_size']
5252
col_type['display_size'] = col['display_size']
53+
# Propagate per-column editability so the save path can refuse to
54+
# write expression/alias columns that don't exist in the base table.
55+
# View Data Tool columns are always real table columns: default True.
56+
col_type['is_editable'] = col.get('is_editable', True) \
57+
if is_query_tool else True
5358
column_types[col['name']] = col_type
5459

5560
if rset['rows']:

web/pgadmin/tools/sqleditor/utils/save_changed_data.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,18 @@ def save_changed_data(changed_data, columns_info, conn, command_obj,
118118
if command_obj.has_oids():
119119
data.pop('oid', None)
120120

121+
# Drop any column the client included that isn't a real
122+
# editable column of the underlying table (e.g.
123+
# `first_name || ' ' || last_name as the_name`). Keys not
124+
# known to the result set are dropped too. Without this
125+
# guard the rendered INSERT references a non-existent
126+
# column and Postgres rejects the row. Issue #9939.
127+
data = {
128+
k: v for k, v in data.items()
129+
if k in columns_info and
130+
columns_info[k].get('is_editable', True)
131+
}
132+
121133
# Update columns value with columns having
122134
# not_null=False and has no default value
123135
column_data.update(data)

web/pgadmin/tools/sqleditor/utils/tests/test_save_changed_data.py

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -960,3 +960,83 @@ def _close_query_tool(self):
960960
url = '/sqleditor/close/{0}'.format(self.trans_id)
961961
response = self.tester.delete(url)
962962
self.assertEqual(response.status_code, 200)
963+
964+
965+
class TestSaveAddedRowSkipsNonEditableColumn(TestSaveChangedData):
966+
"""Regression test for issue #9939.
967+
968+
When a Query Tool result includes an expression or alias column
969+
(e.g. ``first_name || ' ' || last_name AS the_name``), the alias is
970+
not a real column of the underlying table. The new-row save flow
971+
must drop those keys before rendering INSERT; otherwise PostgreSQL
972+
rejects the row with ``column "the_name" does not exist``.
973+
"""
974+
975+
scenarios = [
976+
('Insert via SELECT that aliases a concatenation', dict(
977+
save_payload={
978+
"updated": {},
979+
"added": {
980+
"2": {
981+
"err": False,
982+
"data": {
983+
"id": "1",
984+
"__temp_PK": "2",
985+
"first_name": "John",
986+
"last_name": "Doe",
987+
# The client populates every column when
988+
# building a new row. ``the_name`` is the
989+
# aliased expression — sending it must not
990+
# break the INSERT.
991+
"the_name": None
992+
}
993+
}
994+
},
995+
"staged_rows": {},
996+
"deleted": {},
997+
"updated_index": {},
998+
"added_index": {"2": "2"},
999+
"columns": [
1000+
{"name": "id", "pos": 0, "can_edit": True,
1001+
"type": "integer", "cell": "number",
1002+
"not_null": True, "has_default_val": False,
1003+
"is_array": False, "display_name": "id"},
1004+
{"name": "first_name", "pos": 1, "can_edit": True,
1005+
"type": "text", "cell": "string",
1006+
"not_null": False, "has_default_val": False,
1007+
"is_array": False, "display_name": "first_name"},
1008+
{"name": "last_name", "pos": 2, "can_edit": True,
1009+
"type": "text", "cell": "string",
1010+
"not_null": False, "has_default_val": False,
1011+
"is_array": False, "display_name": "last_name"},
1012+
{"name": "the_name", "pos": 3, "can_edit": False,
1013+
"type": "text", "cell": "string",
1014+
"not_null": False, "has_default_val": False,
1015+
"is_array": False, "display_name": "the_name"},
1016+
]
1017+
},
1018+
save_status=True,
1019+
check_sql='SELECT id, first_name, last_name '
1020+
'FROM %s WHERE id = 1',
1021+
check_result=[[1, "John", "Doe"]]
1022+
)),
1023+
]
1024+
1025+
def _create_test_table(self):
1026+
self.test_table_name = "test_for_save_data_alias_" + \
1027+
str(secrets.choice(range(1000, 9999)))
1028+
create_sql = """
1029+
DROP TABLE IF EXISTS "{0}";
1030+
1031+
CREATE TABLE "{0}"(
1032+
id INT PRIMARY KEY,
1033+
first_name TEXT,
1034+
last_name TEXT
1035+
);
1036+
""".format(self.test_table_name)
1037+
self.select_sql = (
1038+
"SELECT id, first_name, last_name, "
1039+
"first_name || ' ' || last_name AS the_name "
1040+
"FROM {0};"
1041+
).format(self.test_table_name)
1042+
utils.create_table_with_query(self.server, self.db_name, create_sql)

0 commit comments

Comments
 (0)