Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
datlechin merged 5 commits into from
Sep 23, 2026
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5b2d6d0
feat(datagrid): show whether a materialized view can be refreshed con…
datlechin Sep 23, 2026
9389e84
fix(datagrid): let only the latest concurrent refresh check settle
datlechin Sep 23, 2026
818c728
fix(plugin-postgresql): follow pg_dump's index rule so invalid indexe…
datlechin Sep 23, 2026
3943937
Merge remote-tracking branch 'origin/main' into fix/postgres-invalid-…
datlechin Sep 23, 2026
6ebce55
Merge remote-tracking branch 'origin/main' into fix/postgres-invalid-…
datlechin Sep 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Version history for saved queries, with **Restore This Version**. (#2505)
- Git status letters, history and **Discard Changes…** for files in a linked SQL folder. (#2505)
- Whether a materialized view can be refreshed concurrently, on its **Indexes** tab. (#2522)
- Invalid PostgreSQL indexes named on the table's **Indexes** tab.

### Changed

Expand Down Expand Up @@ -370,6 +371,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Destination folder and the first database reading as one path in the backup result sheet. (#3046)
- Only the last line of a failed backup's error shown, which on `pg_dump` is the hint rather than the cause.
- Backup failure reported as an exit code alone when the tool wrote its message and exited at once.
- PostgreSQL export and column reorder script failing on an index a failed `CREATE INDEX CONCURRENTLY` left behind.
- PostgreSQL export and column reorder script failing on a foreign key that references a unique index.
- PostgreSQL exclusion constraints missing from exports and the DDL tab.
- PostgreSQL column reorder script dropping an index named like one of the table's check constraints.
- Invalid PostgreSQL index recreated on the target by Compare & Sync and **Copy To**.
- Indent and Outdent named the wrong way round for Command-[ and Command-] in Settings > Keyboard.
- Table, routine or type missing from the sidebar or Open Quickly when a period in its quoted name matched another's.
- Show Previous Tab and Show Next Tab listed twice in the Window menu.
Expand Down
52 changes: 50 additions & 2 deletions Plugins/PostgreSQLDriverPlugin/PostgreSQLIndexQueries.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,47 @@ import TableProPluginKit
nonisolated enum PostgreSQLIndexQueries {
private static let logger = Logger(subsystem: "com.TablePro.PostgreSQLDriver", category: "IndexQueries")

static let restorableIndexPredicate = "(ix.indisvalid OR t.relkind = 'p') AND ix.indisready"

static func standaloneIndexQuery(schema: String, table: String) -> String {
let schemaLiteral = PostgreSQLObjectQueries.quoteLiteral(schema)
let tableLiteral = PostgreSQLObjectQueries.quoteLiteral(table)
return """
SELECT
i.relname AS index_name,
pg_catalog.pg_get_indexdef(ix.indexrelid) AS definition,
(\(restorableIndexPredicate)) AS is_restorable
FROM pg_catalog.pg_index ix
JOIN pg_catalog.pg_class i ON i.oid = ix.indexrelid
JOIN pg_catalog.pg_class t ON t.oid = ix.indrelid
JOIN pg_catalog.pg_namespace n ON n.oid = t.relnamespace
WHERE n.nspname = \(schemaLiteral)
AND t.relname = \(tableLiteral)
AND NOT EXISTS (
SELECT 1 FROM pg_catalog.pg_constraint con
WHERE con.conrelid = ix.indrelid
AND con.conindid = ix.indexrelid
AND con.contype IN ('p', 'u', 'x')
)
ORDER BY i.relname
"""
}

static func standaloneIndexes(rows: [[PluginCellValue]]) -> PostgreSQLStandaloneIndexes {
var definitions: [String] = []
var invalidNames: [String] = []
for row in rows {
guard let name = row[safe: 0]?.asText,
let definition = row[safe: 1]?.asText?.nilIfEmpty else { continue }
if PostgreSQLCatalogBoolean.isTrue(row[safe: 2]?.asText) {
definitions.append(definition)
} else {
invalidNames.append(name)
}
}
return PostgreSQLStandaloneIndexes(definitions: definitions, invalidNames: invalidNames)
}

/// One row per index, with its key parts in key order.
///
/// A key part is read by position rather than by joining `pg_attribute` on `indkey`. An expression
Expand Down Expand Up @@ -56,7 +97,8 @@ nonisolated enum PostgreSQLIndexQueries {
JOIN pg_catalog.pg_attribute a
ON a.attrelid = ix.indrelid AND a.attnum = ix.indkey[k.n - 1]
ORDER BY k.n
)::text AS included_columns
)::text AS included_columns,
(\(restorableIndexPredicate)) AS is_valid
FROM pg_catalog.pg_index ix
JOIN pg_catalog.pg_class i ON i.oid = ix.indexrelid
JOIN pg_catalog.pg_class t ON t.oid = ix.indrelid
Expand Down Expand Up @@ -157,6 +199,11 @@ nonisolated struct PostgreSQLCatalogIndexDDL: Equatable {
let whereClause: String?
}

nonisolated struct PostgreSQLStandaloneIndexes: Equatable {
let definitions: [String]
let invalidNames: [String]
}

nonisolated enum PostgreSQLIndexRow {
static func index(
from row: [PluginCellValue],
Expand All @@ -176,7 +223,8 @@ nonisolated enum PostgreSQLIndexRow {
expressions: nonEmptyValues(row[safe: 7]?.asText),
includedColumns: nonEmptyValues(row[safe: 8]?.asText),
ddlMethodAndKeys: spelling?.methodAndKeys,
ddlWhereClause: spelling?.whereClause
ddlWhereClause: spelling?.whereClause,
isValid: row[safe: 9]?.asText.map { PostgreSQLCatalogBoolean.isTrue($0) }
)
return (table, index)
}
Expand Down
218 changes: 61 additions & 157 deletions Plugins/PostgreSQLDriverPlugin/PostgreSQLPluginDriver+ColumnReorder.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,163 +28,28 @@ extension PostgreSQLPluginDriver {
Set(parts.columnNames) == Set(desiredOrder),
parts.columnNames.count == desiredOrder.count else { return nil }

let qualified = "\(quoteIdentifier(resolvedSchema)).\(quoteIdentifier(table))"
let staging = "\(quoteIdentifier(resolvedSchema)).\(quoteIdentifier("\(table)_tablepro_reorder"))"
let copyList = parts.copyableColumns.map { quoteIdentifier($0) }.joined(separator: ", ")

let body = desiredOrder.compactMap { parts.columnDefinitions[$0] }

/// The order here is the whole difficulty, and every step of it was measured against
/// PostgreSQL 17. The old table is renamed rather than dropped, so a foreign key in another
/// table keeps pointing at real rows while the copy runs. But a rename moves nothing else:
/// the staging table still owns every index name and every constraint name the original
/// had, and both live in the schema rather than on the table. Declaring the constraints
/// inside the `CREATE TABLE` therefore silently renames them, which shipped as `x_pkey1`,
/// `x_a_b_key1` and `x_c_check1`; creating an index before the staging table goes fails
/// outright with "relation already exists". So nothing that carries a name is created until
/// the staging table is dropped, and the staging table cannot be dropped until every
/// inbound foreign key has let go of it.
var statements: [String] = []
statements.append("ALTER TABLE \(qualified) RENAME TO \(quoteIdentifier("\(table)_tablepro_reorder"))")
statements.append("CREATE TABLE \(qualified) (\n " + body.joined(separator: ",\n ") + "\n)")
statements.append(PostgreSQLVersionedStatements.copyRows(
into: qualified,
from: staging,
columnList: copyList,
capabilities: versionedCapabilities
))
statements.append(contentsOf: parts.identityResets(qualified: qualified, quote: quoteIdentifier))
statements.append(contentsOf: parts.inboundForeignKeyDrops)
/// A `serial` column's default still calls the sequence the staging table owns, so `DROP
/// TABLE` tries to take that sequence with it and PostgreSQL refuses, rolling the whole
/// script back. Measured: handing ownership to the rebuilt table first lets the drop
/// through, and the sequence keeps its original name.
statements.append(contentsOf: parts.serialSequenceHandovers)
statements.append("DROP TABLE \(staging)")
statements.append(contentsOf: parts.tableConstraints.map { "ALTER TABLE \(qualified) ADD \($0)" })
statements.append(contentsOf: parts.outboundForeignKeys.map { "ALTER TABLE \(qualified) ADD \($0)" })
statements.append(contentsOf: parts.inboundForeignKeyAdds)
statements.append(contentsOf: parts.indexes)
statements.append(contentsOf: parts.triggers)
statements.append(contentsOf: parts.triggerModes)
statements.append(contentsOf: parts.comments)

return PluginColumnReorderPlan(
statements: statements,
statements: parts.statements(
table: table,
schema: resolvedSchema,
desiredOrder: desiredOrder,
quote: quoteIdentifier,
capabilities: versionedCapabilities
),
isTransactional: true,
cost: .tableRebuild,
caveats: parts.dependentViewCaveat + [
String(localized: "Grants, row-level security policies, publications, extended statistics, partitioning and table inheritance are not carried over."),
String(localized: "An identity column keeps its value, but its sequence is recreated under a new name because the old table still holds the original name when the new one is created.")
],
caveats: parts.caveats,
isRunnable: false
)
}

private struct RebuildParts {
var columnNames: [String] = []
var columnDefinitions: [String: String] = [:]
var copyableColumns: [String] = []
var identityColumns: [String] = []
var tableConstraints: [String] = []
var outboundForeignKeys: [String] = []
var inboundForeignKeyDrops: [String] = []
var inboundForeignKeyAdds: [String] = []
var indexes: [String] = []
var triggers: [String] = []
var triggerModes: [String] = []
var comments: [String] = []
var dependentViews: [String] = []
var serialSequenceHandovers: [String] = []

/// PostgreSQL binds a view to the table's OID, not its name, so a view follows the rename
/// onto the staging table and then refuses to let it be dropped. Measured: the rebuild
/// stops at `DROP TABLE` with "cannot drop table … because other objects depend on it" and
/// the whole transaction rolls back. Naming them here is what stops that being discovered
/// three quarters of the way through the script.
var dependentViewCaveat: [String] {
guard !dependentViews.isEmpty else { return [] }
return [
String(
format: String(
localized: "Drop and recreate these views first, or the script stops when it drops the old table: %@."
),
dependentViews.joined(separator: ", ")
)
]
}

/// A new identity column starts its sequence at one, so it is wound forward to the rows the
/// copy just wrote. Without this the next insert collides with an existing key.
///
/// `qualified` is already a quoted identifier pair, and `pg_get_serial_sequence` takes the
/// whole pair as one literal, so it is composed first and quoted once. A schema, table or
/// column name may legally contain an apostrophe or a backslash, and both land inside a
/// literal here.
func identityResets(qualified: String, quote: (String) -> String) -> [String] {
let relationLiteral = PostgreSQLObjectQueries.quoteLiteral(qualified)
return identityColumns.map { column in
let columnLiteral = PostgreSQLObjectQueries.quoteLiteral(column)
return """
SELECT setval(
pg_get_serial_sequence(\(relationLiteral), \(columnLiteral)),
GREATEST(COALESCE((SELECT MAX(\(quote(column))) FROM \(qualified)), 0), 1),
true
)
"""
}
}
}

private func fetchRebuildParts(table: String, schema: String) async throws -> RebuildParts {
private func fetchRebuildParts(table: String, schema: String) async throws -> PostgreSQLTableRebuild {
let tableLiteral = PostgreSQLObjectQueries.quoteLiteral(table)
let schemaLiteral = PostgreSQLObjectQueries.quoteLiteral(schema)
let caps = versionedCapabilities
var parts = RebuildParts()
var parts = PostgreSQLTableRebuild()

let identityClause = caps.hasIdentityColumns ? """
CASE
WHEN a.attidentity = 'a' THEN ' GENERATED ALWAYS AS IDENTITY'
WHEN a.attidentity = 'd' THEN ' GENERATED BY DEFAULT AS IDENTITY'
ELSE ''
END ||
""" : ""
let generatedClause = caps.hasGeneratedColumns ? """
CASE
WHEN a.attgenerated = 's' THEN ' GENERATED ALWAYS AS (' || pg_get_expr(d.adbin, d.adrelid) || ') STORED'
WHEN a.attgenerated = 'v' THEN ' GENERATED ALWAYS AS (' || pg_get_expr(d.adbin, d.adrelid) || ') VIRTUAL'
ELSE ''
END ||
""" : ""
let defaultGuard = [
caps.hasIdentityColumns ? "AND a.attidentity = ''" : "",
caps.hasGeneratedColumns ? "AND a.attgenerated = ''" : ""
].filter { !$0.isEmpty }.joined(separator: " ")
let identityFlag = caps.hasIdentityColumns ? "a.attidentity <> ''" : "false"
let generatedFlag = caps.hasGeneratedColumns ? "a.attgenerated <> ''" : "false"

let columnRows = try await execute(query: """
SELECT
a.attname,
quote_ident(a.attname) || ' ' || format_type(a.atttypid, a.atttypmod) || \(PostgreSQLSchemaQueries.columnCollateClause) ||
\(identityClause)
\(generatedClause)
CASE WHEN a.attnotnull THEN ' NOT NULL' ELSE '' END ||
CASE
WHEN a.atthasdef \(defaultGuard)
THEN ' DEFAULT ' || pg_get_expr(d.adbin, d.adrelid)
ELSE ''
END,
\(identityFlag),
\(generatedFlag)
FROM pg_attribute a
JOIN pg_class c ON c.oid = a.attrelid
JOIN pg_namespace n ON n.oid = c.relnamespace
LEFT JOIN pg_attrdef d ON d.adrelid = c.oid AND d.adnum = a.attnum
WHERE c.relname = \(tableLiteral) AND n.nspname = \(schemaLiteral)
AND a.attnum > 0 AND NOT a.attisdropped
ORDER BY a.attnum
""").rows
let columnRows = try await rebuildColumnRows(tableLiteral: tableLiteral, schemaLiteral: schemaLiteral)

for row in columnRows {
guard let name = row[safe: 0]?.asText, let definition = row[safe: 1]?.asText else { continue }
Expand Down Expand Up @@ -242,17 +107,9 @@ extension PostgreSQLPluginDriver {

/// The indexes a constraint owns come back with the constraint, so listing them again would
/// fail on a duplicate name.
parts.indexes = try await textRows("""
SELECT indexdef FROM pg_indexes
WHERE tablename = \(tableLiteral) AND schemaname = \(schemaLiteral)
AND indexname NOT IN (
SELECT con.conname FROM pg_constraint con
JOIN pg_class c ON c.oid = con.conrelid
JOIN pg_namespace n ON n.oid = c.relnamespace
WHERE c.relname = \(tableLiteral) AND n.nspname = \(schemaLiteral)
)
ORDER BY indexname
""")
let standaloneIndexes = try await fetchStandaloneIndexes(table: table, schema: schema)
parts.indexes = standaloneIndexes.definitions
parts.invalidIndexes = standaloneIndexes.invalidNames

parts.triggers = try await textRows("""
SELECT pg_get_triggerdef(t.oid, true)
Expand Down Expand Up @@ -340,6 +197,53 @@ extension PostgreSQLPluginDriver {
return parts
}

private func rebuildColumnRows(tableLiteral: String, schemaLiteral: String) async throws -> [[PluginCellValue]] {
let caps = versionedCapabilities
let identityClause = caps.hasIdentityColumns ? """
CASE
WHEN a.attidentity = 'a' THEN ' GENERATED ALWAYS AS IDENTITY'
WHEN a.attidentity = 'd' THEN ' GENERATED BY DEFAULT AS IDENTITY'
ELSE ''
END ||
""" : ""
let generatedClause = caps.hasGeneratedColumns ? """
CASE
WHEN a.attgenerated = 's' THEN ' GENERATED ALWAYS AS (' || pg_get_expr(d.adbin, d.adrelid) || ') STORED'
WHEN a.attgenerated = 'v' THEN ' GENERATED ALWAYS AS (' || pg_get_expr(d.adbin, d.adrelid) || ') VIRTUAL'
ELSE ''
END ||
""" : ""
let defaultGuard = [
caps.hasIdentityColumns ? "AND a.attidentity = ''" : "",
caps.hasGeneratedColumns ? "AND a.attgenerated = ''" : ""
].filter { !$0.isEmpty }.joined(separator: " ")
let identityFlag = caps.hasIdentityColumns ? "a.attidentity <> ''" : "false"
let generatedFlag = caps.hasGeneratedColumns ? "a.attgenerated <> ''" : "false"

return try await execute(query: """
SELECT
a.attname,
quote_ident(a.attname) || ' ' || format_type(a.atttypid, a.atttypmod) || \(PostgreSQLSchemaQueries.columnCollateClause) ||
\(identityClause)
\(generatedClause)
CASE WHEN a.attnotnull THEN ' NOT NULL' ELSE '' END ||
CASE
WHEN a.atthasdef \(defaultGuard)
THEN ' DEFAULT ' || pg_get_expr(d.adbin, d.adrelid)
ELSE ''
END,
\(identityFlag),
\(generatedFlag)
FROM pg_attribute a
JOIN pg_class c ON c.oid = a.attrelid
JOIN pg_namespace n ON n.oid = c.relnamespace
LEFT JOIN pg_attrdef d ON d.adrelid = c.oid AND d.adnum = a.attnum
WHERE c.relname = \(tableLiteral) AND n.nspname = \(schemaLiteral)
AND a.attnum > 0 AND NOT a.attisdropped
ORDER BY a.attnum
""").rows
}

private func textRows(_ query: String) async throws -> [String] {
try await execute(query: query).rows.compactMap { $0[safe: 0]?.asText }
}
Expand Down
Loading
Loading