fix(datagrid): enable editing tables without a primary key - #600
Merged
Conversation
Tables with no primary key silently refused every cell edit: double click did nothing, the sidebar editor never showed the submit button, and Set NULL / Set EMPTY / Delete Row were no-ops. Rows of keyless tables are now identified by the values of all their comparable columns (binary, geometric, json and hstore columns are excluded), but only when the result set exposes every physical column of the table. Updates on such rows run sequentially per row, threading already-applied values into each WHERE clause. Driver support: NULL entries in the pk map render as IS NULL instead of being rejected, booleans bind natively, and MySQL/MariaDB updates and deletes by pk map carry LIMIT 1 so duplicate rows are never swept by a single statement. When no safe identity exists the grid now explains why editing is unavailable instead of ignoring the double click.
- exclude approximate numeric columns (float, double, real) from the fallback row identity: stored floats may not equality-match the grid's decimal representation, which made updates fail with a misleading error - drop DEFAULT-sentinel columns from subsequent WHERE maps in the keyless update plan instead of matching the literal sentinel, so setting DEFAULT on two columns of the same row no longer fails after partial application - check the affected count of keyless deletions and repeat each DELETE until every duplicate the grid marked is gone: MySQL (LIMIT 1) and PostgreSQL/SQLite (full sweep) now converge on the state the grid showed, and a stale row raises a clear error (new dataGrid.keylessDeleteNotFound key, all 11 locales) instead of silently doing nothing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #598
Problem
Cells of a table without a primary key could not be edited at all, and the UI gave no feedback about why:
The reporter's
prenotazionitable has no primary key and no unique index, which is exactly the case that triggered this. The behavior was not a regression of v0.18.0: row addressing has always requiredpkColumns, and every code path that needs it bailed out silently when the table had none.How it is fixed
Row identity fallback (frontend)
New util
src/utils/rowIdentity.ts:resolveRowIdentity(pkColumns, columnMetadata, resultColumns)returns the primary key columns when the table has them (unchanged behavior), and otherwise falls back to identifying rows by the values of all comparable physical columns. The fallback only activates when the result set exposes every physical column of the table, so a partialSELECT a, b FROM ton a keyless table stays non-editable (it could not distinguish rows that differ only in the omitted columns).isComparableColumnexcludes binary, geometric, json, hstore and floating-point columns from the identity. Binary/geometric/json/hstore values live in the grid as display/wire representations that would not match in an equality comparison, and approximate numerics (FLOAT, DOUBLE, REAL) are not exactly representable, so comparing them against the grid's decimal representation may silently match nothing. Excluded columns stay editable, they just don't take part in addressing the row.buildKeylessUpdatePlanplans the updates for one keyless row. Without a primary key each UPDATE invalidates the previous value of the column it just changed, and that value is part of the next WHERE clause, so updates for the same row run sequentially with the already-applied values threaded into each step's WHERE map. Values whose stored result is unknown client side (the DEFAULT sentinel) are ordered last and dropped from the WHERE maps of any later step, so applying DEFAULT to two columns of the same row works instead of failing halfway through.Editor.tsxderives this identity once per tab and uses it everywhere rows are addressed: pending-change tracking, selection filtering, delete-selected, rollback, submit, and thepkColumnsprop ofDataGrid. On submit, keyless rows are updated per row in plan order, and a zero affected-rows result raises a clear error instead of silently doing nothing (the row no longer matches its original values, e.g. the data changed underneath).Keyless deletions get the same treatment. All grid rows sharing an identity are marked for deletion together (pending deletions are keyed by the serialized identity), so each DELETE is repeated until the number of copies the grid showed is gone: MySQL removes one copy per statement (
LIMIT 1) and loops, PostgreSQL/SQLite sweep them in one statement, and every driver ends up in the state the grid displayed. A zero affected-rows result raises a clear error here too (dataGrid.keylessDeleteNotFound, translated in all 11 locales).When no safe identity exists (keyless table with a partial result), the double click now shows an explanatory alert (
dataGrid.noRowIdentity, translated in all 11 locales) instead of being ignored.Driver support (backend)
The
pk_mapsent toupdate_record/delete_recordcan now legitimately contain NULLs and booleans, since it may carry a full row:push_pk_conditionhelper renderscol IS NULLfor NULL entries (previously rejected with "Unsupported PK type") and binds booleans, on both the prepared and the text (bastion) protocol.update_recordanddelete_recordnow appendLIMIT 1: a no-op when the WHERE is a real primary key match, and a guard against sweeping duplicate rows when the identity is all-columns.build_pk_predicaterendersIS NULLwithout consuming a placeholder index and binds booleans natively.sqlite_push_pk_wheregets the sameIS NULLand boolean handling.PostgreSQL and SQLite do not support
LIMITon UPDATE/DELETE, so on those drivers rows that are entirely identical are updated together; the identity collapses to the same key for such rows in the grid as well, which is consistent. For deletions the frontend loop described above makes all drivers converge, so the per-driver difference no longer leaks into the outcome.Regression safety
resolveRowIdentityreturns the PK columns unchanged, submits stay parallel, and the MySQLLIMIT 1cannot change the outcome of a PK-addressed statement.build_pk_predicate(internal,pub(super), single caller) whose param became optional to express "no bound parameter" for IS NULL.rowIdentity), 1108 Rust tests (18 new across the three drivers' WHERE builders),tsc,eslint.Testing
prenotazioni; a subset SELECT shows the explanatory alert instead of editing.IS NULLaddressing matches the intended row,LIMIT 1touches exactly one of two identical copies (PostgreSQL/SQLite sweep both), and a WHERE built from stale values reports zero affected rows, which now surfaces as an error for both updates and deletions.