Skip to content

fix(plugin-postgresql): follow pg_dump's index rule so invalid indexes stay out of dumps and copies - #3075

Merged
datlechin merged 5 commits into
mainfrom
fix/postgres-invalid-index-ddl
Sep 23, 2026
Merged

datlechin merged 5 commits into
mainfrom
fix/postgres-invalid-index-ddl

Conversation

@datlechin

Copy link
Copy Markdown
Member

Stacked on #3063

Summary

A PostgreSQL index left invalid by a failed or cancelled CREATE INDEX CONCURRENTLY or REINDEX CONCURRENTLY was written into SQL exports, the DDL tab, MCP get_table_ddl and the column reorder script as an ordinary CREATE INDEX, so replaying any of them failed on the duplicate rows the build had tripped over and rolled back. Compare & Sync and Copy To recreated it on the target. The same read dropped a plain unique index that a foreign key depends on, and exclusion constraints were in neither the table DDL nor the index DDL.

This PR gives PostgreSQL one index inclusion rule, pg_dump's own, and uses it everywhere a copy of a table is written. The Indexes tab keeps the row, because the index is still maintained on every write and still refuses duplicates, and names it in a line under the grid.

Root cause

Two hand-written queries decided which indexes stand outside a table's constraints, and neither implemented pg_dump's getIndexes rule.

  • fetchIndexDDL had no indisvalid or indisready predicate, and it excluded an index when any pg_constraint row named it in conindid. A foreign key's conindid names the unique index it references, so that index was dropped from the dump. pg_dump joins on conrelid = indrelid AND conindid = indexrelid AND contype IN ('p','u','x').
  • Column reorder read pg_indexes and excluded indexes whose name matched any constraint on the table, so it kept invalid indexes and dropped a valid index that shared a name with a CHECK constraint.
  • fetchTableDDL wrote contype IN ('p','u','c'), so an EXCLUDE constraint was in neither half.
  • PluginIndexInfo carried no validity, so the grid, Compare, Object Copy and MCP could not tell an invalid index from a working one.

Two ordering defects made the foreign-key case fail even with the index present: the SQL export wrote every deferred ADD FOREIGN KEY before the index phase, and the reorder script re-added foreign keys before the indexes. pg_dump writes indexes first.

What changed

  • PostgreSQLIndexQueries.restorableIndexPredicate is pg_dump REL_17's rule, (ix.indisvalid OR t.relkind = 'p') AND ix.indisready. standaloneIndexQuery projects it beside pg_get_indexdef and uses pg_dump's constraint join; standaloneIndexes(rows:) splits restorable definitions from invalid names. fetchIndexDDL (export, DDL tab, Copy DDL, MCP get_table_ddl, PGlite) and column reorder both read through it.
  • indexList projects the same predicate as is_valid, so the grid note, Compare, Object Copy, MCP and the export agree on one rule. An ON ONLY index on a partitioned table waiting for a partition's index to be attached is kept by pg_dump, so it reads as valid here too and gets no note.
  • PluginIndexInfo.isValid: Bool? arrives through a new full initializer; the previous full initializer is now @_disfavoredOverload and unchanged. nil means the driver does not report it and reads as valid. Synthesized Codable, so a payload without the key decodes to nil. Kit version stays at the pending 33, with a note beside the others.
  • IndexInfo.isValid (default true) carries it. TableStructureRead.sourceSnapshot drops invalid indexes; Compare's source side, the data compare's source side and Object Copy's source read use it, while the target keeps its invalid indexes. So an invalid source index is never created on a target, a target's invalid twin of a source index is left alone rather than re-added into an "already exists", and an orphan invalid index on the target is still offered for dropping.
  • SQL export: the index phase runs before the deferred foreign keys. Column reorder: indexes come back right after the table's own constraints, before any foreign key. The rebuild's statement order moved into PostgreSQLTableRebuild (pure, in the test target) so it can be tested; it also names the invalid indexes it leaves out in a caveat.
  • PostgreSQLSchemaQueries.tableDDLConstraintsQuery (moved out of fetchTableDDL) adds 'x'.
  • The Indexes tab stacks InvalidIndexNote over the concurrent-refresh note. ConcurrentRefreshNoteView became StructureTabNoteView(systemImage:text:identifier:); the refresh note keeps structure-concurrent-refresh-note, the new note is structure-invalid-index-note.
  • MCP index JSON gains is_valid, declared (required) in MCPToolSchema.indexDefinition and in docs/external-api/mcp-tools.mdx.
  • scripts/check-postgres-index-dump-parity.sh builds every shape below on a live server and diffs both the index names and the constraint names against pg_dump -s, after grepping the Swift predicates out of the source.

Measured

PostgreSQL 17.11 and pg_dump 17.11, throwaway server on 127.0.0.1:54329.

Shapes: t with a failed unique CIC over duplicates (t_email_key, valid f / ready f), a unique CIC cancelled while waiting out an older snapshot (t_code_key, f/t), a cancelled REINDEX CONCURRENTLY (t_code_idx_ccnew, f/t); parent_code_idx, a plain unique index child_code_fkey references; s_v_idx, an index sharing its name with a CHECK constraint; ex, an EXCLUDE USING gist; a partitioned p with an ON ONLY index attached on one of two partitions.

scripts/check-postgres-index-dump-parity.sh:

Before (base predicates) After
t indexes plugin t_code_idx t_code_idx_ccnew t_code_key t_email_key, pg_dump t_code_idx agree
parent indexes plugin none, pg_dump parent_code_idx agree
ex constraints plugin none, pg_dump ex_r_excl agree
other 13 comparisons agree agree

The real driver, built with swiftc against the worktree's TableProPluginKit.framework and run against the same schema, base branch then this branch:

Base This branch
fetchIndexDDL(t) t_code_idx, t_code_key, t_email_key t_code_idx
fetchIndexDDL(parent) nothing parent_code_idx
fetchTableDDL(ex) r int4range only adds EXCLUDE USING gist (r WITH &&)
reorder t, run in a transaction could not create unique index "t_email_key", Key (email)=(a@x) is duplicated runs; caveat "Invalid indexes are not recreated: t_code_key, t_email_key."
reorder parent, run in a transaction there is no unique constraint matching given keys for referenced table "parent" runs
reorder s s_v_idx never recreated recreated
fetchIndexes(t) isValid n/a t_code_key and t_email_key false, others true
fetchIndexes(p) isValid n/a p_a_idx (ON ONLY, waiting) true, as pg_dump keeps it

Replaying the export's old order by hand: ALTER TABLE child ADD CONSTRAINT child_code_fkey ... REFERENCES parent (code) before CREATE UNIQUE INDEX parent_code_idx fails with there is no unique constraint matching given keys for referenced table "parent"; the new order runs.

PluginKit ABI: scripts/check-pluginkit-abi.sh against the merge base with origin/main: additive only. One stored property (isValid: Bool?) and one initializer are added, and the previous full initializer gains @_disfavoredOverload with its signature unchanged; nothing is removed. nm on the built framework still exports the 7-argument and the 11-argument initializers beside the new 12-argument one. currentPluginKitVersion was already raised to 33 this cycle (v0.75.0 ships 32), so it is reused and no Info.plist changes.

Tests

verify.sh test on every suite that owns a changed type, after the final edit: 356 executed, 356 passed.

New suites: PostgreSQLStandaloneIndexQueryTests (5), PostgreSQLTableDDLConstraintsQueryTests (1), PostgreSQLTableRebuildTests (5), TableStructureSnapshotIndexValidityTests (5), InvalidIndexNoteTests (3). New cases in existing suites: PostgreSQLIndexKeyPartTests (3), PluginIndexInfoCodableTests (extended), PluginIndexMappingCoverageTests (1), MCPIndexEncodingTests (2), SQLExportIndexPhaseTests (1, replacing a case whose name claimed indexes follow the foreign keys). PluginStructureFixtures now varies isValid, so PluginStructureMappingTests catches a mapper that drops it. The new builders are registered in PostgreSQLLegacyCatalogQueryTests (9.1 portability and hostile names) and PostgreSQLLiteralQuotingTests.

Also: verify.sh build (TablePro, PostgreSQLDriver, SQLExport) pass; verify.sh docs pass; verify.sh lint on every changed Swift file shows no violation on a changed line (two pre-existing public_error_text_in_log hits at PostgreSQLPluginDriver.swift:812 and :837 are untouched lines; the pre-existing function_body_length on fetchRebuildParts is fixed by moving its column read out); shellcheck --severity=warning on the new script is clean.

Each new test was run red by undoing its fix in one pass (all mutations applied together, then restored):

Fix undone Goes red
Export index phase moved back after the foreign keys SQLExportIndexPhaseTests.indexesPrecedeDeferredForeignKeys
Reorder indexes moved back after the foreign keys PostgreSQLTableRebuildTests.indexesPrecedeForeignKeys
Reorder invalid-index caveat removed PostgreSQLTableRebuildTests.invalidIndexesAreNamed
sourceSnapshot stops filtering TableStructureSnapshotIndexValidityTests sourceSnapshotDropsInvalidIndexes, invalidSourceIndexIsNotSynced, objectCopyLeavesInvalidIndexOut
IndexInfo(_:) maps isValid to a constant PluginStructureMappingTests.indexCarriesEveryField
MCP stops encoding is_valid MCPIndexEncodingTests.validityIsEncoded
Predicate reduced to ix.indisready PostgreSQLStandaloneIndexQueryTests.restorableFollowsPgDump, PostgreSQLIndexKeyPartTests.validityUsesTheDumpRule
'x' removed from the table DDL constraints PostgreSQLTableDDLConstraintsQueryTests.exclusionConstraintsAreIncluded
Note stops selecting invalid indexes InvalidIndexNoteTests namesOnlyTheInvalidIndex, listsEveryInvalidIndex

13 cases red, all expected, and green again after restoring. targetInvalidTwinIsNotReAdded and orphanTargetInvalidIndexIsDropped guard the other direction: they go red if the target side is filtered too.

No UI test: an invalid index needs a live PostgreSQL server and a concurrent build made to fail, which the UI suite does not provide.

Before / After

Screenshots to be added. States to capture, light and dark:

  1. Indexes tab of a PostgreSQL table with one invalid index (t_code_key): the grid lists it and the line under the grid reads "t_code_key is invalid, so queries skip it and exports leave it out. Drop it, or rebuild it with REINDEX."
  2. The same table with two invalid indexes (t_code_key, t_email_key): the plural line.
  3. A materialized view with an invalid unique index: the invalid-index line stacked over the concurrent-refresh line.
  4. The DDL tab of t: before lists CREATE UNIQUE INDEX t_code_key and t_email_key, after does not.

Critique points not taken

None. Every objection in the review was applied:

  • Index phase before the foreign keys in the export, indexes right after table constraints in the reorder, with order tests.
  • 'x' in the table DDL constraint read, and the parity script diffs constraints as well as indexes.
  • Compare and Object Copy drop invalid indexes from source reads only, under the same predicate the export uses; tests for a target's invalid twin and an orphan invalid target index.
  • No "drop and recreate" for an ON ONLY partitioned index waiting on ATTACH: the predicate reads it as valid, so the note leaves it out.
  • isValid is Bool? through a new overload with synthesized Codable.
  • is_valid declared in the MCP schema and docs; the new builders registered in the 9.1-portability and hostile-name suites.

Deviation from the design: the design projected raw ix.indisvalid. This projects pg_dump's rule instead, so one predicate decides the note, Compare, Object Copy, MCP and the export, and the partitioned case needs no table kind in the app (a structure tab's objectKind falls back to .table when the tab was opened without one, which would have worded the partitioned case wrongly).

Deliberately not fixed here

  • No ALTER INDEX ... ATTACH PARTITION is ever written, so a partitioned table's ON ONLY index restores invalid even when the source's was valid. Plugins/PostgreSQLDriverPlugin/PostgreSQLPluginDriver.swift:453 (fetchIndexDDL) and the export's index phase would need the attach statements pg_dump writes.
  • Table constraints are written without their names, because pg_get_constraintdef returns the body alone: a primary key, unique, check or exclusion constraint restores under PostgreSQL's default name, and a standalone index already holding that default name then fails the restore. Plugins/PostgreSQLDriverPlugin/PostgreSQLSchemaQueries.swift:478.
  • A target whose only copy of an index is invalid compares as identical to the source's valid one, so the sync never rebuilds it. Rebuilding needs validity on EditableIndexDefinition and a DROP plus CREATE in the script. TablePro/Core/Compare/CompareRunner.swift:305.
  • Other engines' unusable indexes are not reported: SQL Server is_disabled, Oracle UNUSABLE, MySQL INVISIBLE. Each driver's index read would set isValid.
  • An export that leaves an invalid index out says nothing, as pg_dump says nothing. fetchIndexDDL returns [String], so a note would need the export data source to carry the skipped names.

@datlechin
datlechin deleted the branch main September 23, 2026 19:19
@datlechin datlechin closed this Sep 23, 2026
@datlechin
datlechin deleted the fix/postgres-invalid-index-ddl branch September 23, 2026 19:19
@datlechin
datlechin restored the fix/postgres-invalid-index-ddl branch September 23, 2026 19:20
@datlechin datlechin reopened this Sep 23, 2026
@datlechin
datlechin changed the base branch from feat/2522-matview-indexes to main September 23, 2026 19:21
@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, 7:27 PM

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

…index-ddl

# Conflicts:
#	TablePro/Resources/Localizable.xcstrings
#	docs/databases/postgresql.mdx
@datlechin
datlechin merged commit 4cf524b into main Sep 23, 2026
7 of 8 checks passed
@datlechin
datlechin deleted the fix/postgres-invalid-index-ddl branch September 23, 2026 19:29

This branch was successfully deployed

1 active deployment
staging - docs 6ebce556 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