From 185b252f32e530adc8cc9862218a404554a0e816 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Wed, 2 Sep 2026 17:24:10 +0700 Subject: [PATCH] feat(plugins)!: order SQL exports by foreign key dependency and report what the order cannot fix (#2517) Claude-Session: https://claude.ai/code/session_01S9ckdzeugurfqNmDpGU2M7 --- CHANGELOG.md | 11 + Plugins/BeancountDriverPlugin/Info.plist | 2 +- Plugins/BigQueryDriverPlugin/Info.plist | 2 +- Plugins/CSVExportPlugin/Info.plist | 2 +- Plugins/CSVImportPlugin/Info.plist | 2 +- Plugins/CassandraDriverPlugin/Info.plist | 2 +- Plugins/ClickHouseDriverPlugin/Info.plist | 2 +- .../CloudflareD1PluginDriver.swift | 2 + Plugins/CloudflareD1DriverPlugin/Info.plist | 2 +- Plugins/DamengDriverPlugin/Info.plist | 2 +- Plugins/DuckDBDriverPlugin/DuckDBPlugin.swift | 2 + Plugins/DuckDBDriverPlugin/Info.plist | 2 +- Plugins/DynamoDBDriverPlugin/Info.plist | 2 +- Plugins/ElasticsearchDriverPlugin/Info.plist | 2 +- Plugins/EtcdDriverPlugin/Info.plist | 2 +- Plugins/JSONExportPlugin/Info.plist | 2 +- Plugins/JSONImportPlugin/Info.plist | 2 +- Plugins/KafkaDriverPlugin/Info.plist | 2 +- Plugins/LibSQLDriverPlugin/Info.plist | 2 +- .../LibSQLPluginDriver.swift | 2 + Plugins/MQLExportPlugin/Info.plist | 2 +- Plugins/MSSQLDriverPlugin/Info.plist | 2 +- .../MSSQLPluginDriver+Schema.swift | 2 + Plugins/MongoDBDriverPlugin/Info.plist | 2 +- Plugins/MySQLDriverPlugin/Info.plist | 2 +- .../MySQLDriverPlugin/MySQLPluginDriver.swift | 2 + Plugins/OracleDriverPlugin/Info.plist | 2 +- .../CockroachPluginDriver.swift | 2 + Plugins/PostgreSQLDriverPlugin/Info.plist | 2 +- .../RedshiftPluginDriver.swift | 33 ++- Plugins/RedisDriverPlugin/Info.plist | 2 +- Plugins/SQLExportPlugin/Info.plist | 2 +- Plugins/SQLExportPlugin/SQLExportPlugin.swift | 74 ++++-- Plugins/SQLImportPlugin/Info.plist | 2 +- Plugins/SQLiteDriverPlugin/Info.plist | 2 +- Plugins/SQLiteDriverPlugin/SQLitePlugin.swift | 2 + Plugins/SnowflakeDriverPlugin/Info.plist | 2 +- .../SnowflakePluginDriver.swift | 2 + Plugins/SurrealDBDriverPlugin/Info.plist | 2 +- .../ForeignKeyTopologicalSort.swift | 137 ++++++++-- .../PluginDatabaseDriver.swift | 11 + .../PluginExportDataSource.swift | 6 + Plugins/TeradataDriverPlugin/Info.plist | 2 +- Plugins/TrinoDriverPlugin/Info.plist | 2 +- Plugins/XLSXExportPlugin/Info.plist | 2 +- .../Plugins/ExportDataSourceAdapter.swift | 4 + TablePro/Core/Plugins/PluginManager.swift | 2 +- .../Core/Services/Export/ExportService.swift | 14 +- .../Components/TransferResultAlert.swift | 13 +- TablePro/Views/Export/ExportDialog.swift | 9 +- .../ForeignKeyTopologicalSortTests.swift | 43 ++++ .../Core/Services/ExportStateTests.swift | 6 +- .../SQLExportForeignKeyOrderTests.swift | 235 ++++++++++++++++++ docs/development/plugin-development.mdx | 2 +- docs/development/plugin-registry.mdx | 4 +- docs/features/import-export.mdx | 4 +- project.yml | 3 + 57 files changed, 598 insertions(+), 91 deletions(-) create mode 100644 TableProTests/Plugins/SQLExportForeignKeyOrderTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index e6bfdb2564..f0acd079d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- PluginKit ABI 21. Every registry plugin needs rebuilding before or with this release. +- Export summary reports the warnings an export produced, instead of a bare "Export completed". (#2517) + +### Fixed + +- Silent fallback order when foreign keys between the exported tables form a cycle. (#2517) +- Foreign keys declared twice in a SQL export on MySQL, SQL Server, DuckDB, Snowflake, CockroachDB and Redshift, and as an unsupported `ALTER TABLE` on SQLite, libSQL and Cloudflare D1. (#2517) +- Foreign keys missing from Redshift's reconstructed `CREATE TABLE`. + ## [0.71.0] - 2026-09-02 ### Added diff --git a/Plugins/BeancountDriverPlugin/Info.plist b/Plugins/BeancountDriverPlugin/Info.plist index 173ac7d820..8618e2f4d7 100644 --- a/Plugins/BeancountDriverPlugin/Info.plist +++ b/Plugins/BeancountDriverPlugin/Info.plist @@ -3,7 +3,7 @@ TableProPluginKitVersion - 20 + 21 TableProProvidesDatabaseTypeIds Beancount diff --git a/Plugins/BigQueryDriverPlugin/Info.plist b/Plugins/BigQueryDriverPlugin/Info.plist index d2cce4dc9e..4dafd54bc9 100644 --- a/Plugins/BigQueryDriverPlugin/Info.plist +++ b/Plugins/BigQueryDriverPlugin/Info.plist @@ -5,6 +5,6 @@ TableProMinAppVersion 0.42.0 TableProPluginKitVersion - 20 + 21 diff --git a/Plugins/CSVExportPlugin/Info.plist b/Plugins/CSVExportPlugin/Info.plist index aab4558fc9..b4a776c7b3 100644 --- a/Plugins/CSVExportPlugin/Info.plist +++ b/Plugins/CSVExportPlugin/Info.plist @@ -3,7 +3,7 @@ TableProPluginKitVersion - 20 + 21 TableProProvidesExportFormatIds csv diff --git a/Plugins/CSVImportPlugin/Info.plist b/Plugins/CSVImportPlugin/Info.plist index 121d712908..d6b8b25a3a 100644 --- a/Plugins/CSVImportPlugin/Info.plist +++ b/Plugins/CSVImportPlugin/Info.plist @@ -3,7 +3,7 @@ TableProPluginKitVersion - 20 + 21 TableProProvidesImportFormatIds csv diff --git a/Plugins/CassandraDriverPlugin/Info.plist b/Plugins/CassandraDriverPlugin/Info.plist index d73d6826ce..dd93807516 100644 --- a/Plugins/CassandraDriverPlugin/Info.plist +++ b/Plugins/CassandraDriverPlugin/Info.plist @@ -21,6 +21,6 @@ NSPrincipalClass $(PRODUCT_MODULE_NAME).CassandraPlugin TableProPluginKitVersion - 20 + 21 diff --git a/Plugins/ClickHouseDriverPlugin/Info.plist b/Plugins/ClickHouseDriverPlugin/Info.plist index a7fa58059e..a22345aad3 100644 --- a/Plugins/ClickHouseDriverPlugin/Info.plist +++ b/Plugins/ClickHouseDriverPlugin/Info.plist @@ -3,7 +3,7 @@ TableProPluginKitVersion - 20 + 21 TableProProvidesDatabaseTypeIds ClickHouse diff --git a/Plugins/CloudflareD1DriverPlugin/CloudflareD1PluginDriver.swift b/Plugins/CloudflareD1DriverPlugin/CloudflareD1PluginDriver.swift index 25ae5eeeb5..5c55649224 100644 --- a/Plugins/CloudflareD1DriverPlugin/CloudflareD1PluginDriver.swift +++ b/Plugins/CloudflareD1DriverPlugin/CloudflareD1PluginDriver.swift @@ -315,6 +315,8 @@ final class CloudflareD1PluginDriver: PluginDatabaseDriver, @unchecked Sendable var providesBulkForeignKeyFetch: Bool { true } + var tableDDLIncludesForeignKeys: Bool { true } + func fetchAllForeignKeys(schema: String?) async throws -> [String: [PluginForeignKeyInfo]] { let query = """ SELECT m.name AS table_name, p.id, p."table" AS referenced_table, diff --git a/Plugins/CloudflareD1DriverPlugin/Info.plist b/Plugins/CloudflareD1DriverPlugin/Info.plist index d2cce4dc9e..4dafd54bc9 100644 --- a/Plugins/CloudflareD1DriverPlugin/Info.plist +++ b/Plugins/CloudflareD1DriverPlugin/Info.plist @@ -5,6 +5,6 @@ TableProMinAppVersion 0.42.0 TableProPluginKitVersion - 20 + 21 diff --git a/Plugins/DamengDriverPlugin/Info.plist b/Plugins/DamengDriverPlugin/Info.plist index 0fdf6f0c92..6a412d916c 100644 --- a/Plugins/DamengDriverPlugin/Info.plist +++ b/Plugins/DamengDriverPlugin/Info.plist @@ -3,7 +3,7 @@ TableProPluginKitVersion - 20 + 21 TableProProvidesDatabaseTypeIds Dameng diff --git a/Plugins/DuckDBDriverPlugin/DuckDBPlugin.swift b/Plugins/DuckDBDriverPlugin/DuckDBPlugin.swift index 35bcaa2644..05f223616c 100644 --- a/Plugins/DuckDBDriverPlugin/DuckDBPlugin.swift +++ b/Plugins/DuckDBDriverPlugin/DuckDBPlugin.swift @@ -602,6 +602,8 @@ final class DuckDBPluginDriver: PluginDatabaseDriver, @unchecked Sendable { } } + var tableDDLIncludesForeignKeys: Bool { true } + func fetchForeignKeys(table: String, schema: String?) async throws -> [PluginForeignKeyInfo] { let schemaName = resolveSchema(schema) let catalog = try requireCatalog() diff --git a/Plugins/DuckDBDriverPlugin/Info.plist b/Plugins/DuckDBDriverPlugin/Info.plist index c271ca1f66..bb241b15a9 100644 --- a/Plugins/DuckDBDriverPlugin/Info.plist +++ b/Plugins/DuckDBDriverPlugin/Info.plist @@ -3,6 +3,6 @@ TableProPluginKitVersion - 20 + 21 diff --git a/Plugins/DynamoDBDriverPlugin/Info.plist b/Plugins/DynamoDBDriverPlugin/Info.plist index d2cce4dc9e..4dafd54bc9 100644 --- a/Plugins/DynamoDBDriverPlugin/Info.plist +++ b/Plugins/DynamoDBDriverPlugin/Info.plist @@ -5,6 +5,6 @@ TableProMinAppVersion 0.42.0 TableProPluginKitVersion - 20 + 21 diff --git a/Plugins/ElasticsearchDriverPlugin/Info.plist b/Plugins/ElasticsearchDriverPlugin/Info.plist index c679a35746..a631ed7e96 100644 --- a/Plugins/ElasticsearchDriverPlugin/Info.plist +++ b/Plugins/ElasticsearchDriverPlugin/Info.plist @@ -5,6 +5,6 @@ TableProMinAppVersion 0.53.0 TableProPluginKitVersion - 20 + 21 diff --git a/Plugins/EtcdDriverPlugin/Info.plist b/Plugins/EtcdDriverPlugin/Info.plist index d2cce4dc9e..4dafd54bc9 100644 --- a/Plugins/EtcdDriverPlugin/Info.plist +++ b/Plugins/EtcdDriverPlugin/Info.plist @@ -5,6 +5,6 @@ TableProMinAppVersion 0.42.0 TableProPluginKitVersion - 20 + 21 diff --git a/Plugins/JSONExportPlugin/Info.plist b/Plugins/JSONExportPlugin/Info.plist index aa0d8bbb0c..ce6a23c40e 100644 --- a/Plugins/JSONExportPlugin/Info.plist +++ b/Plugins/JSONExportPlugin/Info.plist @@ -3,7 +3,7 @@ TableProPluginKitVersion - 20 + 21 TableProProvidesExportFormatIds json diff --git a/Plugins/JSONImportPlugin/Info.plist b/Plugins/JSONImportPlugin/Info.plist index 39fca1807d..0cd60a2a64 100644 --- a/Plugins/JSONImportPlugin/Info.plist +++ b/Plugins/JSONImportPlugin/Info.plist @@ -3,7 +3,7 @@ TableProPluginKitVersion - 20 + 21 TableProProvidesImportFormatIds json diff --git a/Plugins/KafkaDriverPlugin/Info.plist b/Plugins/KafkaDriverPlugin/Info.plist index 16959124e2..536864492a 100644 --- a/Plugins/KafkaDriverPlugin/Info.plist +++ b/Plugins/KafkaDriverPlugin/Info.plist @@ -3,7 +3,7 @@ TableProPluginKitVersion - 20 + 21 TableProProvidesDatabaseTypeIds Kafka diff --git a/Plugins/LibSQLDriverPlugin/Info.plist b/Plugins/LibSQLDriverPlugin/Info.plist index d2cce4dc9e..4dafd54bc9 100644 --- a/Plugins/LibSQLDriverPlugin/Info.plist +++ b/Plugins/LibSQLDriverPlugin/Info.plist @@ -5,6 +5,6 @@ TableProMinAppVersion 0.42.0 TableProPluginKitVersion - 20 + 21 diff --git a/Plugins/LibSQLDriverPlugin/LibSQLPluginDriver.swift b/Plugins/LibSQLDriverPlugin/LibSQLPluginDriver.swift index 1e68dbe9f5..046b265917 100644 --- a/Plugins/LibSQLDriverPlugin/LibSQLPluginDriver.swift +++ b/Plugins/LibSQLDriverPlugin/LibSQLPluginDriver.swift @@ -389,6 +389,8 @@ final class LibSQLPluginDriver: PluginDatabaseDriver, @unchecked Sendable { var providesBulkForeignKeyFetch: Bool { true } + var tableDDLIncludesForeignKeys: Bool { true } + func fetchAllForeignKeys(schema: String?) async throws -> [String: [PluginForeignKeyInfo]] { let query = """ SELECT m.name AS table_name, p.id, p."table" AS referenced_table, diff --git a/Plugins/MQLExportPlugin/Info.plist b/Plugins/MQLExportPlugin/Info.plist index cb01349dac..a755e4d4ed 100644 --- a/Plugins/MQLExportPlugin/Info.plist +++ b/Plugins/MQLExportPlugin/Info.plist @@ -3,7 +3,7 @@ TableProPluginKitVersion - 20 + 21 TableProProvidesExportFormatIds mql diff --git a/Plugins/MSSQLDriverPlugin/Info.plist b/Plugins/MSSQLDriverPlugin/Info.plist index c271ca1f66..bb241b15a9 100644 --- a/Plugins/MSSQLDriverPlugin/Info.plist +++ b/Plugins/MSSQLDriverPlugin/Info.plist @@ -3,6 +3,6 @@ TableProPluginKitVersion - 20 + 21 diff --git a/Plugins/MSSQLDriverPlugin/MSSQLPluginDriver+Schema.swift b/Plugins/MSSQLDriverPlugin/MSSQLPluginDriver+Schema.swift index f3af787109..b10be71e3b 100644 --- a/Plugins/MSSQLDriverPlugin/MSSQLPluginDriver+Schema.swift +++ b/Plugins/MSSQLDriverPlugin/MSSQLPluginDriver+Schema.swift @@ -331,6 +331,8 @@ extension MSSQLPluginDriver { var providesBulkForeignKeyFetch: Bool { true } + var tableDDLIncludesForeignKeys: Bool { true } + func fetchAllForeignKeys(schema: String?) async throws -> [String: [PluginForeignKeyInfo]] { let esc = effectiveSchemaEscaped(schema) let sql = """ diff --git a/Plugins/MongoDBDriverPlugin/Info.plist b/Plugins/MongoDBDriverPlugin/Info.plist index c271ca1f66..bb241b15a9 100644 --- a/Plugins/MongoDBDriverPlugin/Info.plist +++ b/Plugins/MongoDBDriverPlugin/Info.plist @@ -3,6 +3,6 @@ TableProPluginKitVersion - 20 + 21 diff --git a/Plugins/MySQLDriverPlugin/Info.plist b/Plugins/MySQLDriverPlugin/Info.plist index 2c3d7b025b..f05c54ac77 100644 --- a/Plugins/MySQLDriverPlugin/Info.plist +++ b/Plugins/MySQLDriverPlugin/Info.plist @@ -3,7 +3,7 @@ TableProPluginKitVersion - 20 + 21 TableProProvidesDatabaseTypeIds MySQL diff --git a/Plugins/MySQLDriverPlugin/MySQLPluginDriver.swift b/Plugins/MySQLDriverPlugin/MySQLPluginDriver.swift index 5c1c89d4be..0c80c3a148 100644 --- a/Plugins/MySQLDriverPlugin/MySQLPluginDriver.swift +++ b/Plugins/MySQLDriverPlugin/MySQLPluginDriver.swift @@ -501,6 +501,8 @@ final class MySQLPluginDriver: PluginDatabaseDriver, @unchecked Sendable { var providesBulkForeignKeyFetch: Bool { true } + var tableDDLIncludesForeignKeys: Bool { true } + func fetchAllForeignKeys(schema: String?) async throws -> [String: [PluginForeignKeyInfo]] { let dbName = _activeDatabase let escapedDb = dbName.replacingOccurrences(of: "'", with: "''") diff --git a/Plugins/OracleDriverPlugin/Info.plist b/Plugins/OracleDriverPlugin/Info.plist index c271ca1f66..bb241b15a9 100644 --- a/Plugins/OracleDriverPlugin/Info.plist +++ b/Plugins/OracleDriverPlugin/Info.plist @@ -3,6 +3,6 @@ TableProPluginKitVersion - 20 + 21 diff --git a/Plugins/PostgreSQLDriverPlugin/CockroachPluginDriver.swift b/Plugins/PostgreSQLDriverPlugin/CockroachPluginDriver.swift index 8ab2086bfc..4bba73bc63 100644 --- a/Plugins/PostgreSQLDriverPlugin/CockroachPluginDriver.swift +++ b/Plugins/PostgreSQLDriverPlugin/CockroachPluginDriver.swift @@ -141,6 +141,8 @@ final class CockroachPluginDriver: LibPQBackedDriver, @unchecked Sendable { } } + var tableDDLIncludesForeignKeys: Bool { true } + func fetchForeignKeys(table: String, schema: String?) async throws -> [PluginForeignKeyInfo] { let safeTable = escapeLiteral(table) let schemaLiteral = escapeLiteral(schema ?? core.currentSchema) diff --git a/Plugins/PostgreSQLDriverPlugin/Info.plist b/Plugins/PostgreSQLDriverPlugin/Info.plist index d0b1fcf6b0..e3243ea198 100644 --- a/Plugins/PostgreSQLDriverPlugin/Info.plist +++ b/Plugins/PostgreSQLDriverPlugin/Info.plist @@ -3,7 +3,7 @@ TableProPluginKitVersion - 20 + 21 TableProProvidesDatabaseTypeIds PostgreSQL diff --git a/Plugins/PostgreSQLDriverPlugin/RedshiftPluginDriver.swift b/Plugins/PostgreSQLDriverPlugin/RedshiftPluginDriver.swift index 83587a7a05..a05d06cd87 100644 --- a/Plugins/PostgreSQLDriverPlugin/RedshiftPluginDriver.swift +++ b/Plugins/PostgreSQLDriverPlugin/RedshiftPluginDriver.swift @@ -362,6 +362,8 @@ final class RedshiftPluginDriver: LibPQBackedDriver, @unchecked Sendable { return indexes } + var tableDDLIncludesForeignKeys: Bool { true } + func fetchForeignKeys(table: String, schema: String?) async throws -> [PluginForeignKeyInfo] { let safeTable = escapeLiteral(table) let query = """ @@ -455,8 +457,11 @@ final class RedshiftPluginDriver: LibPQBackedDriver, @unchecked Sendable { throw LibPQPluginError(message: "Failed to fetch DDL for table '\(table)'", sqlState: nil, detail: nil) } + var parts = columnDefs + parts.append(contentsOf: try await foreignKeyClauses(table: table, schema: schema)) + let ddl = "CREATE TABLE \(quotedSchema).\(quotedTable) (\n " + - columnDefs.joined(separator: ",\n ") + + parts.joined(separator: ",\n ") + "\n);" do { @@ -479,6 +484,32 @@ final class RedshiftPluginDriver: LibPQBackedDriver, @unchecked Sendable { return ddl } + /// `SHOW TABLE` declares foreign keys inline, so the reconstruction below has to as well or + /// `tableDDLIncludesForeignKeys` would be true of one path and false of the other, and a SQL + /// export would drop every constraint whenever the fallback ran. A failed lookup therefore + /// throws rather than returning nothing, because nothing here is indistinguishable from a + /// table that has no foreign keys. + private func foreignKeyClauses(table: String, schema: String?) async throws -> [String] { + let foreignKeys = try await fetchForeignKeys(table: table, schema: schema) + var orderedNames: [String] = [] + var grouped: [String: [PluginForeignKeyInfo]] = [:] + for foreignKey in foreignKeys { + if grouped[foreignKey.name] == nil { orderedNames.append(foreignKey.name) } + grouped[foreignKey.name, default: []].append(foreignKey) + } + return orderedNames.compactMap { name in + guard let group = grouped[name], let first = group.first else { return nil } + let columns = group.map { quoteIdentifier($0.column) }.joined(separator: ", ") + let referencedColumns = group.map { quoteIdentifier($0.referencedColumn) }.joined(separator: ", ") + let referencedSchema = first.referencedSchema.flatMap { $0.isEmpty ? nil : $0 } + let referencedTable = referencedSchema.map { + "\(quoteIdentifier($0)).\(quoteIdentifier(first.referencedTable))" + } ?? quoteIdentifier(first.referencedTable) + return "CONSTRAINT \(quoteIdentifier(name)) FOREIGN KEY (\(columns))" + + " REFERENCES \(referencedTable) (\(referencedColumns))" + } + } + func fetchViewDefinition(view: String, schema: String?) async throws -> String { let safeView = escapeLiteral(view) let schemaLiteral = escapeLiteral(schema ?? core.currentSchema) diff --git a/Plugins/RedisDriverPlugin/Info.plist b/Plugins/RedisDriverPlugin/Info.plist index ebc80fdb94..747e5ba196 100644 --- a/Plugins/RedisDriverPlugin/Info.plist +++ b/Plugins/RedisDriverPlugin/Info.plist @@ -21,7 +21,7 @@ NSPrincipalClass $(PRODUCT_MODULE_NAME).RedisPlugin TableProPluginKitVersion - 20 + 21 TableProProvidesDatabaseTypeIds Redis diff --git a/Plugins/SQLExportPlugin/Info.plist b/Plugins/SQLExportPlugin/Info.plist index 77b02579e0..dcd0cf36bd 100644 --- a/Plugins/SQLExportPlugin/Info.plist +++ b/Plugins/SQLExportPlugin/Info.plist @@ -3,7 +3,7 @@ TableProPluginKitVersion - 20 + 21 TableProProvidesExportFormatIds sql diff --git a/Plugins/SQLExportPlugin/SQLExportPlugin.swift b/Plugins/SQLExportPlugin/SQLExportPlugin.swift index 3c22f4ac4d..30e76a247a 100644 --- a/Plugins/SQLExportPlugin/SQLExportPlugin.swift +++ b/Plugins/SQLExportPlugin/SQLExportPlugin.swift @@ -35,6 +35,11 @@ final class SQLExportPlugin: ExportFormatPlugin, SettablePlugin, @unchecked Send var ddlFailures: [String] = [] var metadataWarnings: [String] = [] + /// The tables a foreign key cycle left the ordering unable to place. They keep the order the + /// export tree gave them, which is the only order left once no parent-first one exists, and + /// the dump says so rather than reading as if it were restorable with the checks on. + var tablesUnorderedByCycle: [String] = [] + /// A dump refers to its tables unqualified whenever every selected table lives in one /// container, which is what makes it restorable into any database. Qualifying became necessary /// only once an export could span two containers holding the same table name: unqualified there @@ -77,6 +82,7 @@ final class SQLExportPlugin: ExportFormatPlugin, SettablePlugin, @unchecked Send ddlFailures = [] metadataWarnings = [] exportSpansContainers = false + tablesUnorderedByCycle = [] let actualDestination: URL let gzipTempURL: URL? @@ -105,6 +111,7 @@ final class SQLExportPlugin: ExportFormatPlugin, SettablePlugin, @unchecked Send let fkMap = await prefetchForeignKeys(tables: tables, dataSource: dataSource) let sortedTables = topologicallySort(tables, fkMap: fkMap) noteContainerSpan(of: sortedTables) + try writeDependencyCycleNote(to: fileHandle) try writeDropPhase(sortedTables: sortedTables, dataSource: dataSource, to: fileHandle) try await writeDependentTypesAndSequences( @@ -146,7 +153,8 @@ final class SQLExportPlugin: ExportFormatPlugin, SettablePlugin, @unchecked Send var warnings: [String] = [] if !ddlFailures.isEmpty { let failedTables = ddlFailures.joined(separator: ", ") - warnings.append("Could not fetch table structure for: \(failedTables)") + warnings.append(String( + format: String(localized: "Could not fetch table structure for: %@"), failedTables)) } warnings.append(contentsOf: metadataWarnings) return ExportFormatResult(warnings: warnings) @@ -200,8 +208,8 @@ final class SQLExportPlugin: ExportFormatPlugin, SettablePlugin, @unchecked Send } } if anyGroupFailed { - metadataWarnings.append( - "Could not fetch foreign keys; FK constraints may be missing from the export.") + metadataWarnings.append(String(localized: + "Could not fetch foreign keys, so foreign key constraints may be missing from the export.")) } return merged } @@ -224,8 +232,8 @@ final class SQLExportPlugin: ExportFormatPlugin, SettablePlugin, @unchecked Send } } if anyGroupFailed { - metadataWarnings.append( - "Could not fetch column metadata; identity columns and generated columns may not round-trip correctly.") + metadataWarnings.append(String(localized: + "Could not fetch column metadata, so identity and generated columns may not round-trip correctly.")) } return merged } @@ -237,8 +245,28 @@ final class SQLExportPlugin: ExportFormatPlugin, SettablePlugin, @unchecked Send let byIdentifier = Dictionary( tables.map { (node(for: $0).identifier, $0) }, uniquingKeysWith: { first, _ in first }) - let ordered = ForeignKeyTopologicalSort.ordered(tables.map { node(for: $0) }, foreignKeysByTable: fkMap) - return ordered.compactMap { byIdentifier[$0.identifier] } + let ordering = ForeignKeyTopologicalSort.order(tables.map { node(for: $0) }, foreignKeysByTable: fkMap) + tablesUnorderedByCycle = ordering.unorderedByCycle.map { $0.identifier } + return ordering.tables.compactMap { byIdentifier[$0.identifier] } + } + + /// The warning states what the file is rather than prescribing a remedy, because the remedy is + /// not the same everywhere: `foreignKeyDisableStatements` is nil on SQL Server, Oracle, + /// Snowflake and DuckDB, so telling every user to import with the checks off would be wrong on + /// the engines that cannot turn them off. + private func writeDependencyCycleNote(to fileHandle: FileHandle) throws { + guard !tablesUnorderedByCycle.isEmpty else { return } + let names = tablesUnorderedByCycle.joined(separator: ", ") + metadataWarnings.append(String( + format: String(localized: """ + Foreign keys between %@ reference each other, so no order puts every parent before \ + its children. Those tables are written in the order they were listed, and the dump \ + cannot be restored while foreign keys are enforced. + """), + names)) + let note = "-- Warning: \(PluginExportUtilities.sanitizeForSQLComment(names)) reference each other.\n" + + "-- No parent-first order exists, so they are written in the order they were listed.\n\n" + try fileHandle.write(contentsOf: note.toUTF8Data()) } private func writeDropPhase( @@ -361,13 +389,19 @@ final class SQLExportPlugin: ExportFormatPlugin, SettablePlugin, @unchecked Send to fileHandle: FileHandle ) throws { var emittedAnything = false - for table in sortedTables where optionValue(table, at: 0) { - let fks = fkMap[node(for: table).identifier] ?? [] - let grouped = groupForeignKeysByConstraint(fks) - for group in grouped { - let alter = renderAddConstraintFK(table: table, group: group, dataSource: dataSource) - try fileHandle.write(contentsOf: "\(alter)\n".toUTF8Data()) - emittedAnything = true + /// A driver that hands back the server's own CREATE statement has already declared these + /// constraints inline, so adding them again names each one twice: MySQL and SQL Server + /// reject the duplicate, and SQLite has no ADD CONSTRAINT to reject it with. The phase + /// exists for the drivers whose DDL leaves foreign keys out, PostgreSQL and Oracle. + if !dataSource.tableDDLIncludesForeignKeys { + for table in sortedTables where optionValue(table, at: 0) { + let fks = fkMap[node(for: table).identifier] ?? [] + let grouped = groupForeignKeysByConstraint(fks) + for group in grouped { + let alter = renderAddConstraintFK(table: table, group: group, dataSource: dataSource) + try fileHandle.write(contentsOf: "\(alter)\n".toUTF8Data()) + emittedAnything = true + } } } @@ -425,11 +459,13 @@ final class SQLExportPlugin: ExportFormatPlugin, SettablePlugin, @unchecked Send let containers = Set(tables.map { $0.containerName ?? "" }) exportSpansContainers = containers.count > 1 guard exportSpansContainers else { return } - metadataWarnings.append( - "Warning: this export spans \(containers.count) databases or schemas. Table references are " - + "qualified, but CREATE TABLE comes from the server unqualified, so restore it into the " - + "matching database or schema." - ) + metadataWarnings.append(String( + format: String(localized: """ + This export spans %lld databases or schemas. Table references are qualified, but \ + CREATE TABLE comes from the server unqualified, so restore it into the matching \ + database or schema. + """), + Int64(containers.count))) } private func qualifiedRef( diff --git a/Plugins/SQLImportPlugin/Info.plist b/Plugins/SQLImportPlugin/Info.plist index 64945bd719..632b66b796 100644 --- a/Plugins/SQLImportPlugin/Info.plist +++ b/Plugins/SQLImportPlugin/Info.plist @@ -3,7 +3,7 @@ TableProPluginKitVersion - 20 + 21 TableProProvidesImportFormatIds sql diff --git a/Plugins/SQLiteDriverPlugin/Info.plist b/Plugins/SQLiteDriverPlugin/Info.plist index 426f9a62fa..3121d1beb3 100644 --- a/Plugins/SQLiteDriverPlugin/Info.plist +++ b/Plugins/SQLiteDriverPlugin/Info.plist @@ -3,7 +3,7 @@ TableProPluginKitVersion - 20 + 21 TableProProvidesDatabaseTypeIds SQLite diff --git a/Plugins/SQLiteDriverPlugin/SQLitePlugin.swift b/Plugins/SQLiteDriverPlugin/SQLitePlugin.swift index a6278447e0..9434d40928 100644 --- a/Plugins/SQLiteDriverPlugin/SQLitePlugin.swift +++ b/Plugins/SQLiteDriverPlugin/SQLitePlugin.swift @@ -873,6 +873,8 @@ final class SQLitePluginDriver: PluginDatabaseDriver, @unchecked Sendable { var providesBulkForeignKeyFetch: Bool { true } + var tableDDLIncludesForeignKeys: Bool { true } + func fetchAllForeignKeys(schema: String?) async throws -> [String: [PluginForeignKeyInfo]] { let query = """ SELECT m.name AS table_name, p.id, p."table" AS referenced_table, diff --git a/Plugins/SnowflakeDriverPlugin/Info.plist b/Plugins/SnowflakeDriverPlugin/Info.plist index 7eb4de381f..698e9fa1e2 100644 --- a/Plugins/SnowflakeDriverPlugin/Info.plist +++ b/Plugins/SnowflakeDriverPlugin/Info.plist @@ -5,6 +5,6 @@ TableProMinAppVersion 0.48.0 TableProPluginKitVersion - 20 + 21 diff --git a/Plugins/SnowflakeDriverPlugin/SnowflakePluginDriver.swift b/Plugins/SnowflakeDriverPlugin/SnowflakePluginDriver.swift index af68785f0f..fbab8db84d 100644 --- a/Plugins/SnowflakeDriverPlugin/SnowflakePluginDriver.swift +++ b/Plugins/SnowflakeDriverPlugin/SnowflakePluginDriver.swift @@ -450,6 +450,8 @@ final class SnowflakePluginDriver: PluginDatabaseDriver, @unchecked Sendable { return SnowflakeSchemaQueries.parseClusterBy(clusterBy) } + var tableDDLIncludesForeignKeys: Bool { true } + func fetchForeignKeys(table: String, schema: String?) async throws -> [PluginForeignKeyInfo] { let database = connection?.currentDatabase let targetSchema = schema ?? connection?.currentSchema diff --git a/Plugins/SurrealDBDriverPlugin/Info.plist b/Plugins/SurrealDBDriverPlugin/Info.plist index 535b56f87c..86b0bf7baf 100644 --- a/Plugins/SurrealDBDriverPlugin/Info.plist +++ b/Plugins/SurrealDBDriverPlugin/Info.plist @@ -3,7 +3,7 @@ TableProPluginKitVersion - 20 + 21 TableProProvidesDatabaseTypeIds SurrealDB diff --git a/Plugins/TableProPluginKit/ForeignKeyTopologicalSort.swift b/Plugins/TableProPluginKit/ForeignKeyTopologicalSort.swift index f77beb8bbf..8f2201f932 100644 --- a/Plugins/TableProPluginKit/ForeignKeyTopologicalSort.swift +++ b/Plugins/TableProPluginKit/ForeignKeyTopologicalSort.swift @@ -18,23 +18,46 @@ public enum ForeignKeyTopologicalSort { } } + /// The ordering, and the tables a dependency cycle left it unable to place. A caller that + /// writes a script the order has to be right for reads `unorderedByCycle` to say so, because + /// `tables` alone cannot distinguish a parent-first order from a partial one. + public struct Ordering: Sendable { + public let tables: [Table] + public let unorderedByCycle: [Table] + + public init(tables: [Table], unorderedByCycle: [Table]) { + self.tables = tables + self.unorderedByCycle = unorderedByCycle + } + } + /// Orders `tables` so a parent precedes every child that references it. Identity is /// `Table.identifier` throughout: `foreignKeysByTable` is keyed by it, and a foreign key /// that names no `referencedSchema` points inside the referencing table's own schema. - /// A dependency cycle falls back to the tables the traversal could not place, in identifier - /// order, so the result holds every distinct input table exactly once. + /// Tables caught in a cycle keep the order they were given, and everything outside the cycle + /// is still ordered around them, so the result holds every distinct input table exactly once. public static func ordered( _ tables: [Table], foreignKeysByTable: [String: [PluginForeignKeyInfo]], childrenFirst: Bool = false ) -> [Table] { + order(tables, foreignKeysByTable: foreignKeysByTable, childrenFirst: childrenFirst).tables + } + + /// `ordered`, plus the tables that sit inside a foreign key cycle. Tables are grouped into + /// strongly connected components and the component graph is ordered, so a table that merely + /// descends from a cycle still lands after it and is not reported as part of it. Only a + /// component holding more than one table is unorderable, and its members keep their input order. + public static func order( + _ tables: [Table], + foreignKeysByTable: [String: [PluginForeignKeyInfo]], + childrenFirst: Bool = false + ) -> Ordering { let nodes = distinct(tables) - guard nodes.count > 1 else { return nodes } + guard nodes.count > 1 else { return Ordering(tables: nodes, unorderedByCycle: []) } let byIdentifier = Dictionary(nodes.map { ($0.identifier, $0) }, uniquingKeysWith: { first, _ in first }) - var indegree: [String: Int] = [:] var children: [String: Set] = [:] - for node in nodes { indegree[node.identifier] = 0 } for node in nodes { let identifier = node.identifier @@ -48,30 +71,112 @@ public enum ForeignKeyTopologicalSort { byIdentifier[parent] != nil, seenParents.insert(parent).inserted else { continue } children[parent, default: []].insert(identifier) - indegree[identifier, default: 0] += 1 } } - var queue = nodes.map { $0.identifier }.filter { (indegree[$0] ?? 0) == 0 }.sorted() + let components = stronglyConnectedComponents(of: nodes, children: children) + let componentOf = Dictionary( + components.enumerated().flatMap { index, members in members.map { ($0, index) } }, + uniquingKeysWith: { first, _ in first }) + + var componentIndegree = [Int](repeating: 0, count: components.count) + var componentChildren: [Set] = Array(repeating: [], count: components.count) + for (parent, kids) in children { + guard let parentComponent = componentOf[parent] else { continue } + for kid in kids { + guard let kidComponent = componentOf[kid], + kidComponent != parentComponent, + componentChildren[parentComponent].insert(kidComponent).inserted else { continue } + componentIndegree[kidComponent] += 1 + } + } + + let componentKey = components.map { $0.min() ?? "" } + var queue = (0 ..< components.count) + .filter { componentIndegree[$0] == 0 } + .sorted { componentKey[$0] < componentKey[$1] } var placed: [String] = [] + var unordered: [String] = [] while !queue.isEmpty { let head = queue.removeFirst() - placed.append(head) - for child in (children[head] ?? []).sorted() { - indegree[child] = (indegree[child] ?? 0) - 1 - if indegree[child] == 0 { + placed.append(contentsOf: components[head]) + if components[head].count > 1 { + unordered.append(contentsOf: components[head]) + } + for child in componentChildren[head].sorted(by: { componentKey[$0] < componentKey[$1] }) { + componentIndegree[child] -= 1 + if componentIndegree[child] == 0 { queue.append(child) } } } - if placed.count < nodes.count { - let settled = Set(placed) - placed += nodes.map { $0.identifier }.filter { !settled.contains($0) }.sorted() + let resolved = placed.compactMap { byIdentifier[$0] } + return Ordering( + tables: childrenFirst ? resolved.reversed() : resolved, + unorderedByCycle: unordered.compactMap { byIdentifier[$0] } + ) + } + + /// Tarjan, iterative so a deep dependency chain cannot overflow the stack. Members come back + /// in the order `nodes` gave them, and a component of more than one table is a cycle: nothing + /// in it can be written before the rest. A table that merely descends from one is its own + /// component and still lands after it, which is why the two are reported separately. + private static func stronglyConnectedComponents( + of nodes: [Table], + children: [String: Set] + ) -> [[String]] { + let order = Dictionary(uniqueKeysWithValues: nodes.enumerated().map { ($1.identifier, $0) }) + var index: [String: Int] = [:] + var lowLink: [String: Int] = [:] + var onStack: Set = [] + var stack: [String] = [] + var nextIndex = 0 + var components: [[String]] = [] + + for root in nodes.map({ $0.identifier }) where index[root] == nil { + var work: [(node: String, next: Int, neighbours: [String])] = [ + (root, 0, (children[root] ?? []).sorted()) + ] + index[root] = nextIndex + lowLink[root] = nextIndex + nextIndex += 1 + stack.append(root) + onStack.insert(root) + + while let frame = work.last { + if frame.next < frame.neighbours.count { + work[work.count - 1].next += 1 + let neighbour = frame.neighbours[frame.next] + if index[neighbour] == nil { + index[neighbour] = nextIndex + lowLink[neighbour] = nextIndex + nextIndex += 1 + stack.append(neighbour) + onStack.insert(neighbour) + work.append((neighbour, 0, (children[neighbour] ?? []).sorted())) + } else if onStack.contains(neighbour) { + lowLink[frame.node] = min(lowLink[frame.node] ?? 0, index[neighbour] ?? 0) + } + continue + } + + work.removeLast() + if let parent = work.last?.node { + lowLink[parent] = min(lowLink[parent] ?? 0, lowLink[frame.node] ?? 0) + } + guard lowLink[frame.node] == index[frame.node] else { continue } + var members: [String] = [] + while let member = stack.popLast() { + onStack.remove(member) + members.append(member) + if member == frame.node { break } + } + components.append(members.sorted { (order[$0] ?? 0) < (order[$1] ?? 0) }) + } } - let resolved = placed.compactMap { byIdentifier[$0] } - return childrenFirst ? resolved.reversed() : resolved + return components } private static func distinct(_ tables: [Table]) -> [Table] { diff --git a/Plugins/TableProPluginKit/PluginDatabaseDriver.swift b/Plugins/TableProPluginKit/PluginDatabaseDriver.swift index 753000029e..071c269c29 100644 --- a/Plugins/TableProPluginKit/PluginDatabaseDriver.swift +++ b/Plugins/TableProPluginKit/PluginDatabaseDriver.swift @@ -139,6 +139,7 @@ public protocol PluginDatabaseDriver: AnyObject, Sendable { func sampleFieldPaths(table: String, schema: String?, limit: Int) async throws -> [PluginFieldPath] func fetchAllForeignKeys(schema: String?) async throws -> [String: [PluginForeignKeyInfo]] var providesBulkForeignKeyFetch: Bool { get } + var tableDDLIncludesForeignKeys: Bool { get } func fetchAllIndexes(schema: String?) async throws -> [String: [PluginIndexInfo]] var providesBulkIndexFetch: Bool { get } func fetchAllTableMetadata(schema: String?) async throws -> [String: PluginTableMetadata] @@ -458,6 +459,16 @@ public extension PluginDatabaseDriver { [] } + /// Answers whether `fetchTableDDL` already carries the table's FOREIGN KEY constraints, which + /// every driver returning the server's own CREATE statement does. A SQL export defers foreign + /// keys to `ALTER TABLE ... ADD CONSTRAINT` after the data, so it must skip that for a driver + /// answering `true` or the dump declares each constraint twice, and SQLite has no such + /// statement to declare it with at all. + /// + /// Defaults to `false`, which is the behaviour every driver shipped before this existed: the + /// export adds the foreign keys itself. A driver whose DDL carries them overrides it. + var tableDDLIncludesForeignKeys: Bool { false } + /// Answers whether `fetchAllForeignKeys` is a single query rather than the N+1 default below. /// The app reads this before fetching a whole schema's foreign keys up front, so a driver that /// has not overridden the default is never asked to make one round trip per table. It belongs diff --git a/Plugins/TableProPluginKit/PluginExportDataSource.swift b/Plugins/TableProPluginKit/PluginExportDataSource.swift index db23ef0b7b..23dd0fece8 100644 --- a/Plugins/TableProPluginKit/PluginExportDataSource.swift +++ b/Plugins/TableProPluginKit/PluginExportDataSource.swift @@ -14,6 +14,7 @@ public protocol PluginExportDataSource: AnyObject, Sendable { func fetchAllColumns(databaseName: String) async throws -> [String: [PluginColumnInfo]] func fetchForeignKeys(table: String, databaseName: String) async throws -> [PluginForeignKeyInfo] func fetchAllForeignKeys(databaseName: String) async throws -> [String: [PluginForeignKeyInfo]] + var tableDDLIncludesForeignKeys: Bool { get } } public extension PluginExportDataSource { @@ -23,4 +24,9 @@ public extension PluginExportDataSource { func fetchAllColumns(databaseName: String) async throws -> [String: [PluginColumnInfo]] { [:] } func fetchForeignKeys(table: String, databaseName: String) async throws -> [PluginForeignKeyInfo] { [] } func fetchAllForeignKeys(databaseName: String) async throws -> [String: [PluginForeignKeyInfo]] { [:] } + + /// Mirrors `PluginDatabaseDriver.tableDDLIncludesForeignKeys` for the export side: `true` means + /// `fetchTableDDL` already declares them, so a format that defers foreign keys must not add + /// them a second time. + var tableDDLIncludesForeignKeys: Bool { false } } diff --git a/Plugins/TeradataDriverPlugin/Info.plist b/Plugins/TeradataDriverPlugin/Info.plist index c271ca1f66..bb241b15a9 100644 --- a/Plugins/TeradataDriverPlugin/Info.plist +++ b/Plugins/TeradataDriverPlugin/Info.plist @@ -3,6 +3,6 @@ TableProPluginKitVersion - 20 + 21 diff --git a/Plugins/TrinoDriverPlugin/Info.plist b/Plugins/TrinoDriverPlugin/Info.plist index c271ca1f66..bb241b15a9 100644 --- a/Plugins/TrinoDriverPlugin/Info.plist +++ b/Plugins/TrinoDriverPlugin/Info.plist @@ -3,6 +3,6 @@ TableProPluginKitVersion - 20 + 21 diff --git a/Plugins/XLSXExportPlugin/Info.plist b/Plugins/XLSXExportPlugin/Info.plist index 4b81c4c361..3477527ac7 100644 --- a/Plugins/XLSXExportPlugin/Info.plist +++ b/Plugins/XLSXExportPlugin/Info.plist @@ -3,7 +3,7 @@ TableProPluginKitVersion - 20 + 21 TableProProvidesExportFormatIds xlsx diff --git a/TablePro/Core/Plugins/ExportDataSourceAdapter.swift b/TablePro/Core/Plugins/ExportDataSourceAdapter.swift index a65a656017..dc7ea9e290 100644 --- a/TablePro/Core/Plugins/ExportDataSourceAdapter.swift +++ b/TablePro/Core/Plugins/ExportDataSourceAdapter.swift @@ -113,6 +113,10 @@ final class ExportDataSourceAdapter: PluginExportDataSource, @unchecked Sendable return try await pluginDriver.fetchAllForeignKeys(schema: exportSchema(for: databaseName)) } + var tableDDLIncludesForeignKeys: Bool { + pluginDriver?.tableDDLIncludesForeignKeys ?? false + } + // MARK: - Helpers /// The export tree names every group after a schema on a schema-aware engine and after a diff --git a/TablePro/Core/Plugins/PluginManager.swift b/TablePro/Core/Plugins/PluginManager.swift index a76f4fe727..118d9335ea 100644 --- a/TablePro/Core/Plugins/PluginManager.swift +++ b/TablePro/Core/Plugins/PluginManager.swift @@ -23,7 +23,7 @@ final class PluginManager { /// v19 host. Measured on a rebuilt CassandraDriver, which implements none of them and imports /// all six. Left at 19, such a plugin passes `validateBundleVersions` in a shipped v19 app and /// then fails `Bundle.loadAndReturnError`; at 20 that app refuses it and says to update. - nonisolated static let currentPluginKitVersion = 20 + nonisolated static let currentPluginKitVersion = 21 /// Still 19, so every plugin already published for the previous release keeps loading. nonisolated static let minimumCompatiblePluginKitVersion = 19 diff --git a/TablePro/Core/Services/Export/ExportService.swift b/TablePro/Core/Services/Export/ExportService.swift index d8d097a798..d9d63b71b3 100644 --- a/TablePro/Core/Services/Export/ExportService.swift +++ b/TablePro/Core/Services/Export/ExportService.swift @@ -50,7 +50,7 @@ struct ExportState { var totalRows: Int = 0 var statusMessage: String = "" var errorMessage: String? - var warningMessage: String? + var warnings: [String] = [] } // MARK: - Export Service @@ -171,9 +171,7 @@ final class ExportService { state.processedRows = progress.processedRows - if !result.warnings.isEmpty { - state.warningMessage = result.warnings.joined(separator: "\n") - } + state.warnings = result.warnings } // MARK: - Statement Timeout @@ -269,9 +267,7 @@ final class ExportService { state.processedRows = progress.processedRows - if !result.warnings.isEmpty { - state.warningMessage = result.warnings.joined(separator: "\n") - } + state.warnings = result.warnings } func exportStreamingQuery( @@ -341,9 +337,7 @@ final class ExportService { state.processedRows = progress.processedRows - if !result.warnings.isEmpty { - state.warningMessage = result.warnings.joined(separator: "\n") - } + state.warnings = result.warnings } // MARK: - Row Count Fetching diff --git a/TablePro/Views/Components/TransferResultAlert.swift b/TablePro/Views/Components/TransferResultAlert.swift index 20f24e5eb7..ac53469505 100644 --- a/TablePro/Views/Components/TransferResultAlert.swift +++ b/TablePro/Views/Components/TransferResultAlert.swift @@ -18,16 +18,23 @@ internal enum TransferResultAlert { case close } + /// An export that finished with something to report says so here, the way an import already + /// does. The suppression checkbox is offered only on a clean run: the alert the user turned + /// off is the routine one, and hiding a warning behind that switch loses it for good. internal static func presentExportSuccess( + warnings: [String], window: NSWindow?, completion: @escaping @MainActor (ExportChoice) -> Void ) { let alert = NSAlert() - alert.messageText = String(localized: "Export completed") - alert.alertStyle = .informational + alert.messageText = warnings.isEmpty + ? String(localized: "Export completed") + : String(localized: "Export completed with warnings") + alert.alertStyle = warnings.isEmpty ? .informational : .warning + alert.informativeText = warnings.joined(separator: "\n\n") alert.addButton(withTitle: String(localized: "Open in Finder")) alert.addButton(withTitle: String(localized: "Done")) - alert.showsSuppressionButton = true + alert.showsSuppressionButton = warnings.isEmpty alert.suppressionButton?.title = String(localized: "Do not show this again") let deliver: @MainActor (NSApplication.ModalResponse) -> Void = { response in diff --git a/TablePro/Views/Export/ExportDialog.swift b/TablePro/Views/Export/ExportDialog.swift index f7805f07f9..abae2d35a5 100644 --- a/TablePro/Views/Export/ExportDialog.swift +++ b/TablePro/Views/Export/ExportDialog.swift @@ -161,7 +161,10 @@ struct ExportDialog: View { } .onChange(of: showSuccessDialog) { _, isShowing in guard isShowing else { return } - TransferResultAlert.presentExportSuccess(window: hostWindow) { choice in + TransferResultAlert.presentExportSuccess( + warnings: exportService?.state.warnings ?? [], + window: hostWindow + ) { choice in showSuccessDialog = false if choice == .openFolder { openContainingFolder() @@ -838,7 +841,7 @@ struct ExportDialog: View { isExporting = false recordSuccessfulExport() - if hideSuccessDialog { + if hideSuccessDialog, exportService?.state.warnings.isEmpty ?? true { isPresented = false } else { showSuccessDialog = true @@ -905,7 +908,7 @@ struct ExportDialog: View { isExporting = false recordSuccessfulExport() - if hideSuccessDialog { + if hideSuccessDialog, exportService?.state.warnings.isEmpty ?? true { isPresented = false } else { showSuccessDialog = true diff --git a/TableProTests/Core/Compare/ForeignKeyTopologicalSortTests.swift b/TableProTests/Core/Compare/ForeignKeyTopologicalSortTests.swift index 524fdc8bc6..65e6c70951 100644 --- a/TableProTests/Core/Compare/ForeignKeyTopologicalSortTests.swift +++ b/TableProTests/Core/Compare/ForeignKeyTopologicalSortTests.swift @@ -110,6 +110,49 @@ struct ForeignKeyTopologicalSortTests { #expect(Set(identifiers) == ["public.orders", "public.customers", "sales.orders"]) } + @Test("A cycle names its own members, in the order they were given") + func cycleNamesItsMembersInInputOrder() { + let ordering = ForeignKeyTopologicalSort.order( + [table("orders", "public"), table("regions", "public"), table("customers", "public")], + foreignKeysByTable: [ + "public.orders": [foreignKey(to: "customers", schema: "public")], + "public.customers": [foreignKey(to: "orders", schema: "public")] + ] + ) + + #expect(ordering.unorderedByCycle.map { $0.identifier } == ["public.orders", "public.customers"]) + #expect(Set(ordering.tables.map { $0.identifier }) + == ["public.orders", "public.customers", "public.regions"]) + } + + @Test("A table that only descends from a cycle is ordered after it, not reported inside it") + func descendantOfACycleIsOrderedAfterIt() { + let ordering = ForeignKeyTopologicalSort.order( + [table("audit", "public"), table("orders", "public"), table("customers", "public")], + foreignKeysByTable: [ + "public.orders": [foreignKey(to: "customers", schema: "public")], + "public.customers": [foreignKey(to: "orders", schema: "public")], + "public.audit": [foreignKey(to: "orders", schema: "public")] + ] + ) + let identifiers = ordering.tables.map { $0.identifier } + + #expect(ordering.unorderedByCycle.map { $0.identifier } == ["public.orders", "public.customers"]) + #expect(identifiers.last == "public.audit") + #expect(identifiers.count == 3) + } + + @Test("An acyclic graph strands nothing") + func acyclicGraphStrandsNothing() { + let ordering = ForeignKeyTopologicalSort.order( + [table("orders", "public"), table("customers", "public")], + foreignKeysByTable: ["public.orders": [foreignKey(to: "customers", schema: "public")]] + ) + + #expect(ordering.unorderedByCycle.isEmpty) + #expect(ordering.tables.map { $0.identifier } == ["public.customers", "public.orders"]) + } + @Test("A self-referencing foreign key does not strand its table") func selfReferenceDoesNotStrandTheTable() { let ordered = ForeignKeyTopologicalSort.ordered( diff --git a/TableProTests/Core/Services/ExportStateTests.swift b/TableProTests/Core/Services/ExportStateTests.swift index eca8f96e63..06a68c00fb 100644 --- a/TableProTests/Core/Services/ExportStateTests.swift +++ b/TableProTests/Core/Services/ExportStateTests.swift @@ -23,7 +23,7 @@ struct ExportStateTests { #expect(state.totalRows == 0) #expect(state.statusMessage == "") #expect(state.errorMessage == nil) - #expect(state.warningMessage == nil) + #expect(state.warnings.isEmpty) } @Test("Value semantics — copy is independent") @@ -83,7 +83,7 @@ struct ExportStateTests { state.errorMessage = "Some error" #expect(state.errorMessage == "Some error") - state.warningMessage = "Some warning" - #expect(state.warningMessage == "Some warning") + state.warnings = ["Some warning"] + #expect(state.warnings == ["Some warning"]) } } diff --git a/TableProTests/Plugins/SQLExportForeignKeyOrderTests.swift b/TableProTests/Plugins/SQLExportForeignKeyOrderTests.swift new file mode 100644 index 0000000000..cafa2ecd7e --- /dev/null +++ b/TableProTests/Plugins/SQLExportForeignKeyOrderTests.swift @@ -0,0 +1,235 @@ +// +// SQLExportForeignKeyOrderTests.swift +// TableProTests +// + +import Foundation +import TableProPluginKit +import Testing + +@Suite("SQL export foreign key ordering") +struct SQLExportForeignKeyOrderTests { + private final class StubExportDataSource: PluginExportDataSource, @unchecked Sendable { + let databaseTypeId: String + let tableDDLIncludesForeignKeys: Bool + let foreignKeys: [String: [PluginForeignKeyInfo]] + let ddlByTable: [String: String] + + init( + databaseTypeId: String, + tableDDLIncludesForeignKeys: Bool, + foreignKeys: [String: [PluginForeignKeyInfo]] = [:], + ddlByTable: [String: String] = [:] + ) { + self.databaseTypeId = databaseTypeId + self.tableDDLIncludesForeignKeys = tableDDLIncludesForeignKeys + self.foreignKeys = foreignKeys + self.ddlByTable = ddlByTable + } + + func streamRows(table: String, databaseName: String) -> AsyncThrowingStream { + AsyncThrowingStream { continuation in + continuation.yield(.header(PluginStreamHeader(columns: ["id"], columnTypeNames: ["INTEGER"]))) + continuation.yield(.rows([[.text("1")]])) + continuation.finish() + } + } + + func fetchTableDDL(table: String, databaseName: String) async throws -> String { + ddlByTable[table] ?? "CREATE TABLE \(table) (id INTEGER)" + } + + func execute(query: String) async throws -> PluginQueryResult { + PluginQueryResult(columns: [], columnTypeNames: [], rows: [], rowsAffected: 0, executionTime: 0) + } + + func quoteIdentifier(_ identifier: String) -> String { + "\"\(identifier.replacingOccurrences(of: "\"", with: "\"\""))\"" + } + + func escapeStringLiteral(_ value: String) -> String { + value.replacingOccurrences(of: "'", with: "''") + } + + func fetchApproximateRowCount(table: String, databaseName: String) async throws -> Int? { nil } + + func fetchAllForeignKeys(databaseName: String) async throws -> [String: [PluginForeignKeyInfo]] { + foreignKeys + } + } + + private func table(_ name: String) -> PluginExportTable { + PluginExportTable(name: name, databaseName: "", tableType: "table", optionValues: [true, true, true], schema: nil) + } + + private func foreignKey(_ name: String, from column: String, to referencedTable: String) -> PluginForeignKeyInfo { + PluginForeignKeyInfo( + name: name, + column: column, + referencedTable: referencedTable, + referencedColumn: "id" + ) + } + + private func runExport( + tables: [PluginExportTable], + dataSource: StubExportDataSource + ) async throws -> (dump: String, result: ExportFormatResult) { + let plugin = SQLExportPlugin() + /// The plugin loads its settings from the app's own defaults, so a developer who has + /// turned gzip on would otherwise get a compressed file the assertions cannot read. + let storedSettings = plugin.settings + plugin.settings = SQLExportOptions() + let destination = FileManager.default.temporaryDirectory + .appendingPathComponent("\(UUID().uuidString).sql") + defer { + plugin.settings = storedSettings + try? FileManager.default.removeItem(at: destination) + } + + let result = try await plugin.export( + tables: tables, + dataSource: dataSource, + destination: destination, + progress: PluginExportProgress(progress: Progress(totalUnitCount: 1)) + ) + let dump = try String(contentsOf: destination, encoding: .utf8) + return (dump, result) + } + + private func createOrder(in dump: String, of tables: [String]) -> [String] { + tables + .compactMap { name -> (String, Int)? in + guard let range = dump.range(of: "-- Table: \(name)\n") else { return nil } + return (name, dump.distance(from: dump.startIndex, to: range.lowerBound)) + } + .sorted { $0.1 < $1.1 } + .map(\.0) + } + + @Test("A parent's CREATE precedes its child's however the tree listed them") + func parentIsCreatedBeforeChild() async throws { + let source = StubExportDataSource( + databaseTypeId: "PostgreSQL", + tableDDLIncludesForeignKeys: false, + foreignKeys: ["orders": [foreignKey("fk_orders_customer", from: "customer_id", to: "customers")]] + ) + + let (dump, _) = try await runExport(tables: [table("orders"), table("customers")], dataSource: source) + + #expect(createOrder(in: dump, of: ["orders", "customers"]) == ["customers", "orders"]) + } + + @Test("A child is dropped before the parent it references") + func childIsDroppedBeforeParent() async throws { + let source = StubExportDataSource( + databaseTypeId: "PostgreSQL", + tableDDLIncludesForeignKeys: false, + foreignKeys: ["orders": [foreignKey("fk_orders_customer", from: "customer_id", to: "customers")]] + ) + + let (dump, _) = try await runExport(tables: [table("customers"), table("orders")], dataSource: source) + + let dropOrders = try #require(dump.range(of: "DROP TABLE IF EXISTS \"orders\"")) + let dropCustomers = try #require(dump.range(of: "DROP TABLE IF EXISTS \"customers\"")) + #expect(dropOrders.lowerBound < dropCustomers.lowerBound) + } + + @Test("A dependency cycle is reported in the warnings and in the dump") + func cycleIsReported() async throws { + let source = StubExportDataSource( + databaseTypeId: "PostgreSQL", + tableDDLIncludesForeignKeys: false, + foreignKeys: [ + "orders": [foreignKey("fk_orders_customer", from: "customer_id", to: "customers")], + "customers": [foreignKey("fk_customers_order", from: "last_order_id", to: "orders")] + ] + ) + + let (dump, result) = try await runExport(tables: [table("orders"), table("customers")], dataSource: source) + + #expect(result.warnings.contains { $0.contains("orders") && $0.contains("customers") }) + #expect(dump.contains("-- Warning: orders, customers reference each other.")) + } + + @Test("A cycle keeps the tables in the order the export listed them") + func cycleFallsBackToTheGivenOrder() async throws { + let source = StubExportDataSource( + databaseTypeId: "PostgreSQL", + tableDDLIncludesForeignKeys: false, + foreignKeys: [ + "orders": [foreignKey("fk_orders_customer", from: "customer_id", to: "customers")], + "customers": [foreignKey("fk_customers_order", from: "last_order_id", to: "orders")] + ] + ) + + let (dump, _) = try await runExport(tables: [table("orders"), table("customers")], dataSource: source) + + #expect(createOrder(in: dump, of: ["orders", "customers"]) == ["orders", "customers"]) + } + + @Test("An acyclic export reports no warnings") + func acyclicExportIsSilent() async throws { + let source = StubExportDataSource( + databaseTypeId: "PostgreSQL", + tableDDLIncludesForeignKeys: false, + foreignKeys: ["orders": [foreignKey("fk_orders_customer", from: "customer_id", to: "customers")]] + ) + + let (dump, result) = try await runExport(tables: [table("orders"), table("customers")], dataSource: source) + + #expect(result.warnings.isEmpty) + #expect(!dump.contains("-- Warning:")) + } + + @Test("A table that only descends from a cycle is not reported as part of it") + func descendantOfACycleIsNotReportedAsACycleMember() async throws { + let source = StubExportDataSource( + databaseTypeId: "PostgreSQL", + tableDDLIncludesForeignKeys: false, + foreignKeys: [ + "orders": [foreignKey("fk_orders_customer", from: "customer_id", to: "customers")], + "customers": [foreignKey("fk_customers_order", from: "last_order_id", to: "orders")], + "audit": [foreignKey("fk_audit_order", from: "order_id", to: "orders")] + ] + ) + + let (dump, result) = try await runExport( + tables: [table("audit"), table("orders"), table("customers")], dataSource: source) + + #expect(result.warnings.contains { $0.contains("orders") && !$0.contains("audit") }) + #expect(createOrder(in: dump, of: ["audit", "orders", "customers"]).last == "audit") + } + + @Test("A driver whose DDL omits foreign keys gets them back as ALTER TABLE") + func deferredForeignKeysAreAdded() async throws { + let source = StubExportDataSource( + databaseTypeId: "PostgreSQL", + tableDDLIncludesForeignKeys: false, + foreignKeys: ["orders": [foreignKey("fk_orders_customer", from: "customer_id", to: "customers")]] + ) + + let (dump, _) = try await runExport(tables: [table("orders"), table("customers")], dataSource: source) + + #expect(dump.contains( + "ALTER TABLE \"orders\" ADD CONSTRAINT \"fk_orders_customer\" " + + "FOREIGN KEY (\"customer_id\") REFERENCES \"customers\" (\"id\");")) + } + + @Test("A driver whose DDL carries foreign keys does not declare them a second time") + func inlineForeignKeysAreNotDuplicated() async throws { + let source = StubExportDataSource( + databaseTypeId: "SQLite", + tableDDLIncludesForeignKeys: true, + foreignKeys: ["orders": [foreignKey("fk_orders_0", from: "customer_id", to: "customers")]], + ddlByTable: [ + "orders": "CREATE TABLE orders (id INTEGER, customer_id INTEGER REFERENCES customers(id))" + ] + ) + + let (dump, _) = try await runExport(tables: [table("orders"), table("customers")], dataSource: source) + + #expect(!dump.contains("ADD CONSTRAINT")) + #expect(dump.contains("REFERENCES customers(id)")) + } +} diff --git a/docs/development/plugin-development.mdx b/docs/development/plugin-development.mdx index 5755f33b89..1d4f7fd3a8 100644 --- a/docs/development/plugin-development.mdx +++ b/docs/development/plugin-development.mdx @@ -13,7 +13,7 @@ A plugin is a macOS loadable bundle target with `WRAPPER_EXTENSION = tableplugin | Key | Type | Required | Purpose | |-----|------|----------|---------| -| `TableProPluginKitVersion` | integer | Yes | The PluginKit ABI the plugin was built against. Current value: 20 | +| `TableProPluginKitVersion` | integer | Yes | The PluginKit ABI the plugin was built against. Current value: 21 | | `TableProProvidesDatabaseTypeIds` | array of strings | Recommended | Database type IDs the plugin serves, which is what makes lazy loading possible | | `CFBundleShortVersionString` | string | Yes | Plugin version, read by registry update checks | | `TableProMinAppVersion` | string | No | The loader rejects the plugin on an older app | diff --git a/docs/development/plugin-registry.mdx b/docs/development/plugin-registry.mdx index bbc54b2a82..8f21729f59 100644 --- a/docs/development/plugin-registry.mdx +++ b/docs/development/plugin-registry.mdx @@ -68,13 +68,13 @@ Themes carry no native code, so they match on architecture alone. "binaries": [ { "architecture": "arm64", - "pluginKitVersion": 20, + "pluginKitVersion": 21, "downloadURL": "https://github.com/TableProApp/TablePro/releases/download/plugin-oracle-v1.0.26/OracleDriver-arm64.zip", "sha256": "" }, { "architecture": "x86_64", - "pluginKitVersion": 20, + "pluginKitVersion": 21, "downloadURL": "https://github.com/TableProApp/TablePro/releases/download/plugin-oracle-v1.0.26/OracleDriver-x86_64.zip", "sha256": "" } diff --git a/docs/features/import-export.mdx b/docs/features/import-export.mdx index 95edbe63e7..07ef9f722b 100644 --- a/docs/features/import-export.mdx +++ b/docs/features/import-export.mdx @@ -70,6 +70,8 @@ A whole-table export streams from the database at constant memory, with no row-c All three are on by default, so an SQL export carries `DROP TABLE IF EXISTS` unless you untick **Drop**. Run that file against the wrong database and it drops the tables first. + A multi-table export orders the tables by their foreign keys, so a parent is created and filled before the rows that reference it and dropped after them. Foreign keys between two tables that reference each other leave no such order: those tables keep the order the export listed them in, the file says so in a comment, and the summary repeats it. Import that one with **Disable foreign key checks** ticked. + Not available on MongoDB or Redis. @@ -147,7 +149,7 @@ The checkbox runs a different statement per engine, and one of them needs a priv | SQLite, libSQL, Cloudflare D1 | `PRAGMA foreign_keys = OFF` | Nothing | | Everything else | Nothing. The option is ignored | | -A server that rejects the statement stops the import with that error, so clear the checkbox and run it again. A dump TablePro exported needs no privilege: it adds foreign keys with `ALTER TABLE … ADD CONSTRAINT` after the data. +A server that rejects the statement stops the import with that error, so clear the checkbox and run it again. A dump TablePro exported usually needs no privilege, because its tables are already ordered parents first. On PostgreSQL and Oracle, whose `CREATE TABLE` leaves foreign keys out, the dump adds them with `ALTER TABLE … ADD CONSTRAINT` after the data instead. ### Import JSON diff --git a/project.yml b/project.yml index cd83991c4d..1bc7baa277 100644 --- a/project.yml +++ b/project.yml @@ -484,6 +484,9 @@ targets: - Plugins/RedisDriverPlugin/RedisSentinelResolver.swift - Plugins/RedisDriverPlugin/RedisStatementGenerator.swift - Plugins/RedisDriverPlugin/RedisTopologyDiagnostics.swift + - Plugins/SQLExportPlugin/SQLExportModels.swift + - Plugins/SQLExportPlugin/SQLExportOptionsView.swift + - Plugins/SQLExportPlugin/SQLExportPlugin.swift - Plugins/SQLImportPlugin/SQLImportFailure.swift - Plugins/SQLImportPlugin/SQLImportOptions.swift - Plugins/SQLImportPlugin/SQLImportOptionsView.swift