Skip to content

fix(coordinator): compose structure and create table previews on the scope save runs on - #3071

Merged
datlechin merged 4 commits into
mainfrom
fix/structure-preview-scope
Sep 23, 2026
Merged

datlechin merged 4 commits into
mainfrom
fix/structure-preview-scope

Conversation

@datlechin

@datlechin datlechin commented Sep 23, 2026

Copy link
Copy Markdown
Member

Summary

Structure Preview SQL, the Create Table SQL Preview tab and a row import's new-table CREATE all built their DDL on the unpinned session driver, while the statements that actually run are built on a driver pinned to the tab's scope. So a preview could name a different schema than Save, and a new table from an import could land in a different schema than its rows. Preview also skipped two things Save does: the primary key constraint name lookup, and the decision to rebuild the table instead of running ALTER.

Every DDL caller now gets its text from one scoped composer, and Save runs exactly what was composed.

Root cause

Several plugins qualify DDL from the driver's own mutable state: PostgreSQL's qualifiedTableName, generateDropIndexSQL and generateCreateTableSQL read core.currentSchema, and DuckDB, SQL Server and Cassandra do the same. pin() moves that state on every scoped session-driver read, and so does the toolbar schema picker. Save built its statements inside withScopedDriver on the tab scope; Preview and Create Table took driver(for:) with no scope, passed no constraint name, and never asked StructureTableRebuildHandler.requiresRebuild. Nothing forced preview and save through the same composer.

On top of that, the primary key name lookup was gated on a closed DatabaseType list (postgresql, redshift, cockroachdb, duckdb) that PGlite was missing from, so a PGlite primary key change saved as DROP CONSTRAINT /* unknown constraint */.

What changed

  • DatabaseManager+SchemaComposition (new): withSchemaComposer(scope:route:) leases the driver for a scope and hands over the pinned plugin driver. On top of it: schemaChangeStatements(tableName:changes:scope:), createTableStatements(plan:scope:) and createTableStatements(definition:scope:route:).
  • PrimaryKeyConstraintLookup (new): a function taking a driver. It asks INFORMATION_SCHEMA.TABLE_CONSTRAINTS for the key being dropped, in the driver's pinned schema, with the table name escaped by the driver itself. It only asks when an existing key is dropped and the engine has schemas. No DatabaseType list.
  • executeSchemaChanges(_ statements:databaseType:scope:) no longer composes; it authorizes, runs and records. Every caller and test moved to compose-then-execute in this commit.
  • StructureSavePlan (new): .alter([SchemaStatement]) or .rebuild(Prepared), decided once by StructureEditingSession.stagedSavePlan(for:). applyStagedChanges and previewStagedChanges(coordinator:) both read it.
    • A rebuild previews as the same TableRebuildReviewRequest Save presents, with its caveats as the warning and Open in Query Editor, but with no action: Preview never offers to run anything. TableRebuildReviewRequest now carries an optional Action, and the sheet only shows it when the plan is runnable.
    • The async preview presents only if, on completion, no sheet is up, the selected tab still shows Structure, and the selected tab still owns this session.
  • Create Table: the composed statements live on CreateTableDraft (coordinator-owned per tab, @Published), recomposed after a 150ms pause keyed on the draft's inputs plus scope. The last composition stays on screen until the next one lands, a failed composition keeps it, a stale one is dropped by generation, and an unchanged draft is not composed again. The preview, the issue label and hasCreateTablePending read it, so a remount no longer blanks them. Create Table composes fresh at press time and checks the plan's own issues synchronously first.
    • The draft forwards its StructureChangeManager's objectWillChange (the ConnectionFormCoordinator pattern). CreateTableView observes only the draft and the AnyChangeManager wrapper, which does not forward, so a column, index or foreign key edit in the grid never re-evaluated the view: the SQL Preview, the issue label and the Create button's readiness stayed on the last state until the name field, a segment switch or a selection re-rendered it. This gap predates this PR. Before it, the view observed the same two objects and read the working rows directly in body, so the old derived preview and readiness went stale on a grid edit the same way. Found by review of this PR and fixed in the follow-up commit.
  • Row import: performImport resolves the browse scope once. The new table's CREATE is composed on executionRoute (the route that runs it), and the same scope goes to ImportService.importFile(scope:), prepareTable, clearRows and the leased statements. ImportService no longer re-derives browseScope; ImportDialog resolves it for its own import.
  • CHANGELOG under Fixed, and the Preview SQL bullet in docs/features/table-structure.mdx.

No PluginKit change, so no ABI check.

Measured

PostgreSQL 17.11 (Homebrew, aarch64), two schemas each with an orders table and an index i, the second table renamed to sales, session search_path moved to the other schema. Probe files: probe.sql (before) and probe-after.sql (after), both in the lane directory.

  • Before: the preview-style DROP INDEX "preview_scope"."i" dropped the other schema's index. DROP CONSTRAINT "sales_pkey" failed with constraint "sales_pkey" of relation "sales" does not exist (the key stays orders_pkey after a rename). DROP CONSTRAINT /* unknown constraint */ failed with syntax error at or near ";". ALTER TABLE "preview_scope"."sales" ... failed with relation "preview_scope.sales" does not exist.
  • After: SELECT CONSTRAINT_NAME FROM INFORMATION_SCHEMA.TABLE_CONSTRAINTS WHERE TABLE_SCHEMA = 'preview_scope_reporting' AND TABLE_NAME = 'sales' AND CONSTRAINT_TYPE = 'PRIMARY KEY' returns orders_pkey. On the pinned schema, DROP CONSTRAINT "orders_pkey", ADD PRIMARY KEY ("id", "v") and DROP INDEX "preview_scope_reporting"."i" all succeed, and the other schema's i is untouched.
  • DuckDB 1.5.4: INFORMATION_SCHEMA.TABLE_CONSTRAINTS reports a synthesized sales_id_pkey, and pg_constraint (the old query) reports PRIMARY KEY(id). Neither matters: ALTER TABLE ... DROP CONSTRAINT answers Not implemented Error: No support for that ALTER TABLE option yet! for any name, so DuckDB behaves as before.

Tests

verify.sh test on StructureSavePlanTests PrimaryKeyConstraintLookupTests CreateTableDraftCompositionTests SchemaCompositionGuardTests DatabaseManagerSchemaChangeRoutingTests StructureEditingSessionTests TabCloseProtectionTests SchemaOperationRefusalTests: 78 executed, 78 passed (after the follow-up commit).

  • StructureSavePlanTests (new, 12 cases): plan names the tab's schema on the pooled route and on a single-connection engine (PGlite) after pinning; Save runs exactly the planned statements; PK drop uses the server's name on PostgreSQL and PGlite and never contains /* unknown constraint */; a SQLite foreign key change plans the rebuild with the prepared script; Preview shows the ALTER statements; Preview shows a rebuild read-only with its caveats and runnability, for runnable and non-runnable plans, while Save's review offers the action only when runnable; a preview that lands after the tab left Structure, over another sheet, or for a session the selected tab no longer owns presents nothing.
  • PrimaryKeyConstraintLookupTests (new, 5): query shape and schema, literal escaping, no query without a drop or without schemas, a failed catalog read leaves the name unknown.
  • CreateTableDraftCompositionTests (new, 8): composes on the tab schema; the last statement stays until the next lands (session gate held across the recomposition); a failure keeps the last good statement; an unchanged draft is not composed again; an empty draft names what is missing with no connection; a grid edit through CreateTableGridDelegate.dataGridDidEditCell on the Columns, Indexes and Foreign Keys tabs publishes the draft and changes its composition key.
  • SchemaCompositionGuardTests (new, 2): no SchemaStatementGenerator(, CreateTableStatementComposer.compose( or generateCreateTableStatements( outside the scoped composers. SchemaSyncScriptBuilder is allow-listed as known unscoped collateral, not as a sanctioned composer.
  • DatabaseManagerSchemaChangeRoutingTests: the 7 existing save tests moved to compose-then-execute; 4 new cases for the composer's driver on both routes, Create Table on the scope schema running unchanged on its own connection, and the import CREATE on the execution route.
  • StructurePreviewSQLUITests (new UI test): on the SQLite sample, select Album's foreign key, remove it, press Cmd+Shift+P, and check the sheet offers Open in Query Editor with no run button and no "Unsupported schema operation". It compiles in the TableProUITests target; I did not run it locally because the lanes share one screen, so CI is its first run.

What turns each red, measured by mutating the source and rerunning:

  • Composing on driver(for:) instead of a scoped lease: 14 cases red, including both scope tests, composesOnTheTabSchema, keepsTheLastStatementUntilTheNextLands, composerDriverSitsOnTheScopeSchema, importCreateTableComposesOnTheExecutionRoute, schemaGroupedEngineKeepsTableSchema.
  • Opening the preview gate unconditionally: the three "presents nothing" cases red.
  • Giving the rebuild preview an action: previewShowsTheRebuildReadOnly red.
  • Dropping the unchanged-key check: unchangedDraftIsNotComposedAgain red.
  • An unscoped SchemaStatementGenerator( call site in TableStructureView+Schema: ddlIsComposedOnlyOnAScopedDriver red.
  • Skipping the lookup for PGlite (the old closed list): primaryKeyDropUsesTheServerName(type: .pglite) red, PostgreSQL still green.
  • Ignoring requiresRebuild: sqliteForeignKeyPlansTheRebuild and both previewShowsTheRebuildReadOnly arguments red.
  • Clearing composed before recomposing: keepsTheLastStatementUntilTheNextLands and failureKeepsTheLastStatement red.
  • Removing the change manager forwarding from CreateTableDraft.init: all three gridEditPublishesTheDraft arguments red.

Build PASS, lint 0 violations on all 20 changed Swift files, docs checks PASS.

Before / After

Screenshots to be added. States to capture:

  1. PostgreSQL, a structure tab on reporting.sales while the toolbar schema is public, one column staged, Preview SQL open: before names "public"."sales", after names "reporting"."sales".
  2. Same table after ALTER TABLE orders RENAME TO sales, primary key changed, Preview SQL open: before DROP CONSTRAINT /* unknown constraint */, after DROP CONSTRAINT "orders_pkey".
  3. SQLite sample, Album, foreign key removed, Preview SQL: before an -- Error generating SQL: Unsupported schema operation sheet, after the rebuild script with Open in Query Editor and Done.
  4. Create Table tab on PostgreSQL in schema reporting with the toolbar on public, SQL Preview segment: before CREATE TABLE "public"..., after CREATE TABLE "reporting"...; then switch to another tab and back, and the preview is still there with no spinner.

Critique points not taken

None. All six objections are resolved as described above. One note on the PK objection: the lookup is engine-agnostic through INFORMATION_SCHEMA.TABLE_CONSTRAINTS rather than a DatabaseType list, which also means SQL Server now gets a real constraint name where it used to get /* unknown constraint */ (Plugins/MSSQLDriverPlugin/MSSQLPluginDriver+DDL.swift:144). The query is written in uppercase so it matches catalog views under a case-sensitive collation. I could not run SQL Server here, so that path is unmeasured and is not claimed in the CHANGELOG.

Deliberately not fixed here

  • Compare's schema sync composes on the target driver outside a scoped lease and never resolves a primary key constraint name: TablePro/Core/Compare/SchemaSyncScriptBuilder.swift:112 and :143. Allow-listed in the guard as known collateral. PrimaryKeyConstraintLookup.constraintName(tableName:changes:driver:) is the function it can call. Object Copy's PK naming is the same follow-up.
  • The Create Table grid's foreign key reference menus read the toolbar schema and the browse database rather than the tab scope: TablePro/Views/Structure/CreateTableView.swift:321, :360, :361.
  • Preview of a draft whose staged rows are incomplete shows generated SQL, where Save refuses with Some Changes Are Incomplete: TablePro/Views/Structure/StructureEditingSession+Apply.swift:57 has the check previewStagedChanges does not make.
  • Open in Query Editor from a foreign key rebuild titles the tab "Reorder ": TablePro/Views/Main/Extensions/MainContentCoordinator+SQLPreview.swift:71.

@mintlify

mintlify Bot commented Sep 23, 2026

Copy link
Copy Markdown

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

Project Status Preview Updated
TablePro 🟢 Ready View Preview Sep 23, 2026, 3:53 PM

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

@datlechin
datlechin merged commit 16d56df into main Sep 23, 2026
8 of 9 checks passed
@datlechin
datlechin deleted the fix/structure-preview-scope branch September 23, 2026 19:19

This branch was successfully deployed

1 active deployment
staging - docs 3937125b Deployed Sep 23, 2026 by mintlify[bot]
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.

1 participant