Skip to content

fix(datagrid): carry identity columns through so a new row pre-fills DEFAULT - #2589

Merged
datlechin merged 2 commits into
mainfrom
fix/identity-column-default-prefill
Aug 31, 2026
Merged

fix(datagrid): carry identity columns through so a new row pre-fills DEFAULT#2589
datlechin merged 2 commits into
mainfrom
fix/identity-column-default-prefill

Conversation

@datlechin

Copy link
Copy Markdown
Member

Fixes #2588.

Root cause

PostgreSQL reports an identity column's generation in pg_attribute.attidentity and leaves column_default null. Measured on PostgreSQL 17:

 column_name |              column_default              | is_identity | identity_generation | attidentity | atthasdef
-------------+------------------------------------------+-------------+---------------------+-------------+-----------
 id          |                                          | YES         | ALWAYS              | a           | f
 code        |                                          | YES         | BY DEFAULT          | d           | f
 ser         | nextval('bank_lookup_ser_seq'::regclass) | NO          |                     |             | t

The PostgreSQL plugin reads that correctly into PluginColumnInfo.identityKind, but PluginDriverAdapter.mapPluginColumns never copied the field into the app's own ColumnInfo, which had no such property. Identity died at the plugin boundary, so every downstream decision fell back to "does the column have a default", which is false for an identity column.

Blast radius, measured

Wider than the report:

  • GENERATED BY DEFAULT AS IDENTITY fails too, with null value in column "code" violates not-null constraint. Only GENERATED ALWAYS was reported.
  • An identity column that is not the primary key survives Duplicate Row unchanged, so the documented workaround does not cover it.
  • Editing an existing GENERATED ALWAYS identity cell hits the same server refusal on save.

The fix

One predicate, in one place, fed by metadata that now survives the plugin boundary.

  • ColumnInfo gains identityKind, and PluginDriverAdapter carries it through. fetchAllColumns now routes through mapPluginColumns rather than its own inline copy, which had already drifted and was dropping isGenerated, generationExpression and generationKind.
  • ParsedSchemaMetadata gains columnIdentity, and TableRows carries it to the two decision points.
  • TableRows.serverAssignsValue(forColumn:) is the single answer to "does leaving this column out of an INSERT make the server supply the value". Add Row, Duplicate Row and the Set Value menu all ask it.
  • A GENERATED ALWAYS AS IDENTITY column joins generatedColumns, the set the app must never write. That is exactly what it is: the engine refuses both an explicit INSERT value and an UPDATE. GENERATED BY DEFAULT stays writable, because it legitimately accepts an explicit value.
  • RowOperationsManager.addNewRow and duplicateRow read their column names and metadata from the TableRows they are already handed, instead of taking a second copy from the caller that could disagree.

MySQL/MariaDB and SQL Server report identity in the Extra string rather than through identityKind, so both now set it: mysqlIdentityKind(extra:) beside the existing mysqlColumnIsGenerated, and .always for SQL Server IDENTITY.

Fixes found while building this, that the change is not safe without

  • The non-writable set evaporated on every tab switch and cached rerun. DataChangeManager.configureForTable clears generatedColumns and only a phase-2 schema fetch refilled it, so a rerun answered from cache, a tab switch, a result-set switch or a column change left generated and identity columns writable again. generatedColumns is now a required parameter of configureForTable and restoreState so it cannot be forgotten, and TableRows carries the set for the cached paths to restore from.
  • A new row of nothing but server-assigned columns was silently dropped. generateInsertSQLFromStoredData returned nil when no column remained, and generateAttributedStatements drops a nil statement, so on a table like CREATE TABLE t (id int GENERATED ALWAYS AS IDENTITY) the save committed its other statements, reported success, and the row vanished. It now emits the dialect's own all-defaults form: DEFAULT VALUES on PostgreSQL and SQLite (verified against PostgreSQL 17), () VALUES () on MySQL.
  • OVERRIDING SYSTEM VALUE and setval were not dialect-gated in SQL export. Both keyed off identityKind/isIdentity alone, so teaching SQL Server to report identity would have put PostgreSQL-only syntax into a SQL Server dump. Both are now gated on the PostgreSQL dialect.
  • PGlite resolved to the generic SQL dialect. SqlDialect.from("PGlite") returned .generic although PGlite is PostgreSQL 17 in WASM and PGlitePluginDriver subclasses PostgreSQLPluginDriver. Beyond the export gate above, that made every statement splitter, the limit detector and the fold scanner miss $$ bodies and E'…' strings on PGlite. It now maps to .postgres.

Also fixed

Add Row put the cursor on column 0 unconditionally. With GENERATED ALWAYS identity columns now read-only, that lands on a cell the editor refuses and nothing opens, which reads as Add Row having done nothing. beginEditingFirstEditableColumn puts the caret in the first cell the row can actually take a value in.

The Set Value menu offered Empty, NULL and Default on columns no statement can carry. Its selectors call setCellValueAtColumn directly, bypassing the edit gate, so on a generated column, a MongoDB _id or a GENERATED ALWAYS identity the edit was staged, filtered out at statement generation, and cleared by the successful save. The menu is now gated on isColumnWritable, the same predicate the inline editor uses.

Verified

Step Result
build TablePro PASS
test (18 suites) PASS, 288 cases, 0 failed
build MySQLDriver / MSSQLDriver / SQLExport / PostgreSQLDriver PASS
abi main PASS, no PluginKit symbol change
lint TablePro Plugins TableProTests 0 violations
docs PASS

SqlDialect.from changed body only, so the PluginKit ABI is unchanged and no version bump is needed.

The plugins aggregate fails locally on a pre-existing OracleNIO issue (macro expansion @TaskLocal: unknown attribute 'usableFromInlinenonisolated'), unrelated to this change, so the four plugin targets this touches were built individually. CI runs the aggregate on its own toolchain.

Behaviour was measured against a live PostgreSQL 17: identity columns report a null column_default, an explicit NULL is refused for both identity kinds, omitting the column works, and INSERT ... DEFAULT VALUES works on an identity-only table.

No UI automation: the flow needs a live PostgreSQL connection with an identity table, which the UI test harness cannot provision deterministically. SQLite, which the harness can reach, has no identity columns in this sense. The decisions are covered by unit tests instead: TableRowsServerAssignedValueTests, TableRowsGeneratedColumnsTests, the identity and generated cases in RowOperationsManagerTests, SchemaMetadataGeneratedColumnTests, MySQLIdentityClassificationTests, and the all-defaults INSERT cases in SQLStatementGeneratorTests.

The SQL export dialect gates have no unit test because SQLExportPlugin.swift is not in the test target's sources; they are compile-checked by the plugin build and rest on SqlDialect.from, which is tested.

Reviewed by

Codex read the diff cold, twice: a defect review and an adversarial review of the approach. Six findings were acted on (the two cached-metadata P1s, the PGlite dialect regression, the ungated Set Value menu, the over-long docs bullet, and the SQL export gating). Four of its findings were verified as pre-existing rather than introduced here and are reported separately rather than folded in: the cached-rerun read of the active session registry is not bound to a result-set id, inline metadata publishes an empty set so a result is briefly writable before phase 2 lands, paste and Fill Column and the row inspector stage edits without consulting the non-writable set, and a SQL Server export writes identity values with no SET IDENTITY_INSERT.

@mintlify

mintlify Bot commented Aug 31, 2026

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
TablePro 🔴 Failed Aug 31, 2026, 5:32 PM

💡 Tip: Enable Workflows to automatically generate PRs for you.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@datlechin
datlechin merged commit e275073 into main Aug 31, 2026
9 checks passed
@datlechin
datlechin deleted the fix/identity-column-default-prefill branch August 31, 2026 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Row pre-fills NULL for PostgreSQL identity columns (GENERATED ALWAYS AS IDENTITY); Set Value has no "Default" option

1 participant