fix: exclude non-editable/alias columns from Query Tool row UPDATE

[kundansable]

Summary

Fixes #10103 — editing a row in the Query Tool result grid fails to save with column "..." does not exist when the query includes an aliased/computed column (e.g. first_name || ' ' || last_name AS the_name), even though that column is already shown as read-only (lock icon) in the grid.

Root Cause

save_changed_data() has two code paths — added (INSERT) and updated (UPDATE). The added path already filters the outgoing payload down to real, editable table columns (fixed for #9939 in #10015), using the same columns_info[...]['is_editable'] metadata that drives the grid's lock icon. The updated path never got the equivalent filter, so an edited row that includes an alias/expression column sends that alias straight into the generated UPDATE ... SET clause — and Postgres rejects it because there's no such real column.

Fix

Apply the same editable-columns guard to the updated branch: drop any key from the changed-row payload that isn't a real, editable table column, and skip the row entirely if nothing editable remains after filtering (avoiding an empty/invalid SET clause).

Test Steps

1. In the Query Tool, run:

   DROP TABLE IF EXISTS some_table;
   CREATE TABLE some_table (id INT PRIMARY KEY, first_name TEXT, last_name TEXT);
   INSERT INTO some_table VALUES (1, 'John', 'Doe');
   
   SELECT id, first_name, last_name,
          first_name || ' ' || last_name AS the_name
   FROM some_table;

2. In the result grid, edit first_name from John to Jane on the existing row.
3. Press F5 / Save.
4. Expected: save succeeds; only first_name is updated. No column "the_name" ... does not exist error.
5. Regression: repeat on a query with no alias/expression columns and confirm normal edits still save correctly.

Summary by CodeRabbit

• Bug Fixes
  • Improved saving of edited table data when query results include calculated, aliased, or otherwise non-editable columns.
  • Prevented invalid update operations by ignoring non-editable fields and skipping rows that have no editable values.
  • Prevented read-only fields from appearing as changed or being committed through the editor.
  • Updates now reliably reference the correct row identifier when applying changes, avoiding misaligned or malformed statements.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 5fd5f8a3-ad60-4a96-a971-66f28fc6bc06

📥 Commits

Reviewing files that changed from the base of the PR and between e15b0a8 and b0b4256.

📒 Files selected for processing (4)

• web/pgadmin/tools/sqleditor/static/js/components/QueryToolDataGrid/Editors.jsx
• web/pgadmin/tools/sqleditor/static/js/components/sections/ResultSet.jsx
• web/pgadmin/tools/sqleditor/utils/tests/test_save_changed_data.py
• web/regression/javascript/sqleditor/text_editor_readonly.spec.js

Included review availability: Your plan includes up to 8 reviews per rolling hour; 6 remain after this review.

---

Walkthrough

The save path preserves row identifiers, filters updates to editable columns, and skips rows without editable fields. The result grid and text editor reject changes to non-editable alias columns. Backend and frontend tests cover these behaviors.

Changes

Editable column filtering

Layer / File(s)	Summary
Filter editable update fields web/pgadmin/tools/sqleditor/utils/save_changed_data.py	The updated operation preserves row identifiers, removes non-editable fields, skips empty updates, and uses the captured identifier.
Block read-only grid edits web/pgadmin/tools/sqleditor/static/js/components/QueryToolDataGrid/Editors.jsx, web/pgadmin/tools/sqleditor/static/js/components/sections/ResultSet.jsx	The text editor and result grid reject changes to columns with can_edit set to false.
Validate alias update handling web/pgadmin/tools/sqleditor/utils/tests/test_save_changed_data.py, web/regression/javascript/sqleditor/text_editor_readonly.spec.js	Tests cover editable updates, alias-only changes, read-only button behavior, and Enter-key commits.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to b0b42

The change limits row updates to editable table columns and skips rows with no editable fields, preventing invalid updates for aliased or computed columns. No actionable merge-blocking risk remains beyond normal checks and review.

Suggested reviewers: hiteshjambhale, dpage

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: excluding non-editable and alias columns from Query Tool row updates.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

web/pgadmin/tools/sqleditor/utils/save_changed_data.py (1)

180-194: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add regression coverage for updated rows with read-only aliases.

Please add or verify tests covering:

• an updated row containing both an editable column and an alias/computed column;
• a row containing only a non-editable alias.

Assert that the editable value is persisted and the alias-only row produces no UPDATE.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@web/pgadmin/tools/sqleditor/utils/save_changed_data.py` around lines 180 -
194, Extend the regression tests covering the save-changed-data flow around the
data filtering logic to include a row with both an editable column and a
non-editable alias, asserting the editable value is persisted, and a row
containing only the alias, asserting no UPDATE is generated or executed. Reuse
the existing test fixtures and assertions for updated rows and SQL persistence.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@web/pgadmin/tools/sqleditor/utils/save_changed_data.py`:
- Around line 180-194: Extend the regression tests covering the
save-changed-data flow around the data filtering logic to include a row with
both an editable column and a non-editable alias, asserting the editable value
is persisted, and a row containing only the alias, asserting no UPDATE is
generated or executed. Reuse the existing test fixtures and assertions for
updated rows and SQL persistence.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d701b2fa-7918-4e34-9c15-30690aac3d3f

📥 Commits

Reviewing files that changed from the base of the PR and between b15c745 and 8e95bb4.

📒 Files selected for processing (1)

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

[asheshv]

Consistency review: added-row vs updated-row column filtering

Nice fix for #10103 — filtering non-editable columns out of data before rendering the SQL template stops the UPDATE from referencing expression/alias columns.

One thing worth double-checking: the added-row path explicitly strips the tracking keys before filtering:

# lines 113-115
data.pop(client_primary_key, None)
data.pop('is_row_copied', None)
...
data = {
    k: v for k, v in data.items()
    if k in columns_info and columns_info[k].get('is_editable', True)
}

The updated-row path (this PR, lines 179-183) relies solely on the k in columns_info filter to drop them:

data = changed_data[of_type][each_row]['data']
data = {
    k: v for k, v in data.items()
    if k in columns_info and columns_info[k].get('is_editable', True)
}

I traced client_primary_key/is_row_copied on the frontend — they're only injected into the added payload (ResultSet.jsx:759 calls setClientPK for new rows), so today the updated-row data dict never actually contains them, and the columns_info filter alone is sufficient. So I don't think this is a live bug right now.

Two things I'd still flag:

1. Defense-in-depth / consistency — if a future change ever starts tagging updated rows the same way (e.g. multi-row copy-paste into existing rows), this path would silently rely on columns_info to save it, instead of being explicit like the added-row path. Mirroring the explicit .pop() calls here would make both code paths symmetric and remove the implicit assumption.
2. row_id after filtering (line 197-200):

   list_of_sql[of_type].append({'sql': sql,
                                'data': data,
                                'row_id':
                                    data.get(client_primary_key)})

   Since data here is now the filtered dict, data.get(client_primary_key) will always be None for updated rows (as it presumably already was, since client_primary_key was never in the updated payload to begin with — so no regression, just flagging that this line is dead weight / possibly should read from the unfiltered changed_data[...]['data'] if row_id was ever meant to be populated here).

Not blocking, just wanted these on record in case someone hits it while extending this code path later.