diff --git a/CHANGELOG.md b/CHANGELOG.md index 32cb85929..117d3d098 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,31 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Comparisons button in the Compare & Sync toolbar, listing every saved comparison. +- Save Comparison… in the Compare & Sync toolbar and under Database > Compare. +- Select > All and Select > None in the structure results pane. +- Whole-schema index and table metadata reads on the driver protocol. +- Script that checks the SQLite whole-schema reads against the per-table ones. + +### Changed + +- Compare & Sync reads a whole schema in a few queries, and reads both sides at once. +- Data mode lists the tables both sides share as soon as a pair is chosen. +- Apply… builds the script when there is not one. +- Saved comparisons set both endpoints, and list whatever pair is on screen. +- Compare & Sync reopens on the source, target, mode and options it last held. +- Table collation on MySQL, previously never read. +- PluginKit ABI 20. Every registry plugin needs rebuilding before or with this release. + +### Fixed + +- Generated column expressions missing from MySQL's whole-schema column read. +- Generated columns missing entirely from SQLite's whole-schema column read. +- PostgreSQL index reads matching a table name in every schema rather than the one asked for. +- Unreachable hazard allowances in the Apply sheet. +- Compare & Sync toolbar naming the old pair after a saved comparison was loaded from Options. +- A failed whole-schema trigger read counting as a schema with no triggers. +- Compare & Sync publishing one pair's results after the pickers moved to another. - Middle-click on a tab to close it. (#2595) ### Fixed diff --git a/Plugins/BeancountDriverPlugin/Info.plist b/Plugins/BeancountDriverPlugin/Info.plist index 8487d1977..173ac7d82 100644 --- a/Plugins/BeancountDriverPlugin/Info.plist +++ b/Plugins/BeancountDriverPlugin/Info.plist @@ -3,7 +3,7 @@ TableProPluginKitVersion - 19 + 20 TableProProvidesDatabaseTypeIds Beancount diff --git a/Plugins/BigQueryDriverPlugin/Info.plist b/Plugins/BigQueryDriverPlugin/Info.plist index 2ea7e5581..d2cce4dc9 100644 --- a/Plugins/BigQueryDriverPlugin/Info.plist +++ b/Plugins/BigQueryDriverPlugin/Info.plist @@ -5,6 +5,6 @@ TableProMinAppVersion 0.42.0 TableProPluginKitVersion - 19 + 20 diff --git a/Plugins/CSVExportPlugin/Info.plist b/Plugins/CSVExportPlugin/Info.plist index 4729f4543..aab4558fc 100644 --- a/Plugins/CSVExportPlugin/Info.plist +++ b/Plugins/CSVExportPlugin/Info.plist @@ -3,7 +3,7 @@ TableProPluginKitVersion - 19 + 20 TableProProvidesExportFormatIds csv diff --git a/Plugins/CSVImportPlugin/Info.plist b/Plugins/CSVImportPlugin/Info.plist index 87fdc215a..121d71290 100644 --- a/Plugins/CSVImportPlugin/Info.plist +++ b/Plugins/CSVImportPlugin/Info.plist @@ -3,7 +3,7 @@ TableProPluginKitVersion - 19 + 20 TableProProvidesImportFormatIds csv diff --git a/Plugins/CassandraDriverPlugin/CassandraPluginDriver+Routines.swift b/Plugins/CassandraDriverPlugin/CassandraPluginDriver+Routines.swift index 9823a1ab2..009fa4057 100644 --- a/Plugins/CassandraDriverPlugin/CassandraPluginDriver+Routines.swift +++ b/Plugins/CassandraDriverPlugin/CassandraPluginDriver+Routines.swift @@ -96,6 +96,12 @@ extension CassandraPluginDriver { return definition } + /// False until the whole-schema scope is checked against a live server. This driver has no + /// per-table trigger read, so the protocol default answers with nothing and a comparison has + /// never listed its triggers; opting in here would change what is compared rather than only + /// how fast it is read. + var providesBulkTriggerFetch: Bool { false } + func fetchAllTriggers(schema: String?) async throws -> [PluginTriggerInfo] { try await cassandraTriggerList(keyspace: resolveKeyspace(schema), table: nil) } diff --git a/Plugins/CassandraDriverPlugin/Info.plist b/Plugins/CassandraDriverPlugin/Info.plist index d0dc3c30a..d73d6826c 100644 --- a/Plugins/CassandraDriverPlugin/Info.plist +++ b/Plugins/CassandraDriverPlugin/Info.plist @@ -21,6 +21,6 @@ NSPrincipalClass $(PRODUCT_MODULE_NAME).CassandraPlugin TableProPluginKitVersion - 19 + 20 diff --git a/Plugins/ClickHouseDriverPlugin/Info.plist b/Plugins/ClickHouseDriverPlugin/Info.plist index 0aac7f94b..a7fa58059 100644 --- a/Plugins/ClickHouseDriverPlugin/Info.plist +++ b/Plugins/ClickHouseDriverPlugin/Info.plist @@ -3,7 +3,7 @@ TableProPluginKitVersion - 19 + 20 TableProProvidesDatabaseTypeIds ClickHouse diff --git a/Plugins/CloudflareD1DriverPlugin/CloudflareD1PluginDriver+Triggers.swift b/Plugins/CloudflareD1DriverPlugin/CloudflareD1PluginDriver+Triggers.swift index 4e75643c9..437eff6e3 100644 --- a/Plugins/CloudflareD1DriverPlugin/CloudflareD1PluginDriver+Triggers.swift +++ b/Plugins/CloudflareD1DriverPlugin/CloudflareD1PluginDriver+Triggers.swift @@ -7,6 +7,8 @@ import Foundation import TableProPluginKit extension CloudflareD1PluginDriver { + var providesBulkTriggerFetch: Bool { true } + func fetchAllTriggers(schema: String?) async throws -> [PluginTriggerInfo] { try await sqliteTriggerList(table: nil) } diff --git a/Plugins/CloudflareD1DriverPlugin/Info.plist b/Plugins/CloudflareD1DriverPlugin/Info.plist index 2ea7e5581..d2cce4dc9 100644 --- a/Plugins/CloudflareD1DriverPlugin/Info.plist +++ b/Plugins/CloudflareD1DriverPlugin/Info.plist @@ -5,6 +5,6 @@ TableProMinAppVersion 0.42.0 TableProPluginKitVersion - 19 + 20 diff --git a/Plugins/DamengDriverPlugin/DamengPluginDriver+Routines.swift b/Plugins/DamengDriverPlugin/DamengPluginDriver+Routines.swift index 44d1411f7..4fc2c45ad 100644 --- a/Plugins/DamengDriverPlugin/DamengPluginDriver+Routines.swift +++ b/Plugins/DamengDriverPlugin/DamengPluginDriver+Routines.swift @@ -65,6 +65,12 @@ extension DamengPluginDriver { return body.uppercased().hasPrefix("CREATE") ? body : "CREATE OR REPLACE \(body)" } + /// False until the whole-schema scope is checked against a live server. This driver has no + /// per-table trigger read, so the protocol default answers with nothing and a comparison has + /// never listed its triggers; opting in here would change what is compared rather than only + /// how fast it is read. + var providesBulkTriggerFetch: Bool { false } + func fetchAllTriggers(schema: String?) async throws -> [PluginTriggerInfo] { try await damengTriggerList(schema: schema, table: nil) } diff --git a/Plugins/DamengDriverPlugin/Info.plist b/Plugins/DamengDriverPlugin/Info.plist index c3f02778f..0fdf6f0c9 100644 --- a/Plugins/DamengDriverPlugin/Info.plist +++ b/Plugins/DamengDriverPlugin/Info.plist @@ -3,7 +3,7 @@ TableProPluginKitVersion - 19 + 20 TableProProvidesDatabaseTypeIds Dameng diff --git a/Plugins/DuckDBDriverPlugin/Info.plist b/Plugins/DuckDBDriverPlugin/Info.plist index 0811e81d5..c271ca1f6 100644 --- a/Plugins/DuckDBDriverPlugin/Info.plist +++ b/Plugins/DuckDBDriverPlugin/Info.plist @@ -3,6 +3,6 @@ TableProPluginKitVersion - 19 + 20 diff --git a/Plugins/DynamoDBDriverPlugin/Info.plist b/Plugins/DynamoDBDriverPlugin/Info.plist index 2ea7e5581..d2cce4dc9 100644 --- a/Plugins/DynamoDBDriverPlugin/Info.plist +++ b/Plugins/DynamoDBDriverPlugin/Info.plist @@ -5,6 +5,6 @@ TableProMinAppVersion 0.42.0 TableProPluginKitVersion - 19 + 20 diff --git a/Plugins/ElasticsearchDriverPlugin/Info.plist b/Plugins/ElasticsearchDriverPlugin/Info.plist index 4aabbb7b7..c679a3574 100644 --- a/Plugins/ElasticsearchDriverPlugin/Info.plist +++ b/Plugins/ElasticsearchDriverPlugin/Info.plist @@ -5,6 +5,6 @@ TableProMinAppVersion 0.53.0 TableProPluginKitVersion - 19 + 20 diff --git a/Plugins/EtcdDriverPlugin/Info.plist b/Plugins/EtcdDriverPlugin/Info.plist index 2ea7e5581..d2cce4dc9 100644 --- a/Plugins/EtcdDriverPlugin/Info.plist +++ b/Plugins/EtcdDriverPlugin/Info.plist @@ -5,6 +5,6 @@ TableProMinAppVersion 0.42.0 TableProPluginKitVersion - 19 + 20 diff --git a/Plugins/JSONExportPlugin/Info.plist b/Plugins/JSONExportPlugin/Info.plist index c05a94aa4..aa0d8bbb0 100644 --- a/Plugins/JSONExportPlugin/Info.plist +++ b/Plugins/JSONExportPlugin/Info.plist @@ -3,7 +3,7 @@ TableProPluginKitVersion - 19 + 20 TableProProvidesExportFormatIds json diff --git a/Plugins/JSONImportPlugin/Info.plist b/Plugins/JSONImportPlugin/Info.plist index b22c84da2..39fca1807 100644 --- a/Plugins/JSONImportPlugin/Info.plist +++ b/Plugins/JSONImportPlugin/Info.plist @@ -3,7 +3,7 @@ TableProPluginKitVersion - 19 + 20 TableProProvidesImportFormatIds json diff --git a/Plugins/KafkaDriverPlugin/Info.plist b/Plugins/KafkaDriverPlugin/Info.plist index 243b16227..16959124e 100644 --- a/Plugins/KafkaDriverPlugin/Info.plist +++ b/Plugins/KafkaDriverPlugin/Info.plist @@ -3,7 +3,7 @@ TableProPluginKitVersion - 19 + 20 TableProProvidesDatabaseTypeIds Kafka diff --git a/Plugins/LibSQLDriverPlugin/Info.plist b/Plugins/LibSQLDriverPlugin/Info.plist index 2ea7e5581..d2cce4dc9 100644 --- a/Plugins/LibSQLDriverPlugin/Info.plist +++ b/Plugins/LibSQLDriverPlugin/Info.plist @@ -5,6 +5,6 @@ TableProMinAppVersion 0.42.0 TableProPluginKitVersion - 19 + 20 diff --git a/Plugins/LibSQLDriverPlugin/LibSQLPluginDriver+Triggers.swift b/Plugins/LibSQLDriverPlugin/LibSQLPluginDriver+Triggers.swift index fc921c963..a72680eb9 100644 --- a/Plugins/LibSQLDriverPlugin/LibSQLPluginDriver+Triggers.swift +++ b/Plugins/LibSQLDriverPlugin/LibSQLPluginDriver+Triggers.swift @@ -7,6 +7,8 @@ import Foundation import TableProPluginKit extension LibSQLPluginDriver { + var providesBulkTriggerFetch: Bool { true } + func fetchAllTriggers(schema: String?) async throws -> [PluginTriggerInfo] { try await sqliteTriggerList(table: nil) } diff --git a/Plugins/MQLExportPlugin/Info.plist b/Plugins/MQLExportPlugin/Info.plist index 11a843bb0..cb01349da 100644 --- a/Plugins/MQLExportPlugin/Info.plist +++ b/Plugins/MQLExportPlugin/Info.plist @@ -3,7 +3,7 @@ TableProPluginKitVersion - 19 + 20 TableProProvidesExportFormatIds mql diff --git a/Plugins/MSSQLDriverPlugin/Info.plist b/Plugins/MSSQLDriverPlugin/Info.plist index 0811e81d5..c271ca1f6 100644 --- a/Plugins/MSSQLDriverPlugin/Info.plist +++ b/Plugins/MSSQLDriverPlugin/Info.plist @@ -3,6 +3,6 @@ TableProPluginKitVersion - 19 + 20 diff --git a/Plugins/MSSQLDriverPlugin/MSSQLPluginDriver+Routines.swift b/Plugins/MSSQLDriverPlugin/MSSQLPluginDriver+Routines.swift index d84cea2a1..f67799cd6 100644 --- a/Plugins/MSSQLDriverPlugin/MSSQLPluginDriver+Routines.swift +++ b/Plugins/MSSQLDriverPlugin/MSSQLPluginDriver+Routines.swift @@ -48,6 +48,8 @@ extension MSSQLPluginDriver { return definition } + var providesBulkTriggerFetch: Bool { true } + func fetchAllTriggers(schema: String?) async throws -> [PluginTriggerInfo] { try await triggerList(schema: effectiveSchema(schema), table: nil) } diff --git a/Plugins/MongoDBDriverPlugin/Info.plist b/Plugins/MongoDBDriverPlugin/Info.plist index 0811e81d5..c271ca1f6 100644 --- a/Plugins/MongoDBDriverPlugin/Info.plist +++ b/Plugins/MongoDBDriverPlugin/Info.plist @@ -3,6 +3,6 @@ TableProPluginKitVersion - 19 + 20 diff --git a/Plugins/MySQLDriverPlugin/Info.plist b/Plugins/MySQLDriverPlugin/Info.plist index 7d1d32067..2c3d7b025 100644 --- a/Plugins/MySQLDriverPlugin/Info.plist +++ b/Plugins/MySQLDriverPlugin/Info.plist @@ -3,7 +3,7 @@ TableProPluginKitVersion - 19 + 20 TableProProvidesDatabaseTypeIds MySQL diff --git a/Plugins/MySQLDriverPlugin/MySQLPluginDriver+BulkMetadata.swift b/Plugins/MySQLDriverPlugin/MySQLPluginDriver+BulkMetadata.swift new file mode 100644 index 000000000..1bbe673d8 --- /dev/null +++ b/Plugins/MySQLDriverPlugin/MySQLPluginDriver+BulkMetadata.swift @@ -0,0 +1,147 @@ +// +// MySQLPluginDriver+BulkMetadata.swift +// MySQLDriverPlugin +// +// Whole-schema reads of the metadata that otherwise costs one round trip per +// table. +// +// A caller comparing two schemas pays four reads per table without these, so a +// 200-table database is 800 round trips per side. Both queries here are the +// unfiltered form of the per-table statement, so they answer the same question +// for every table at once. +// +// The shaping is shared with the per-table reads rather than written twice. Two +// copies of the same grouping is how a bulk read drifts from the read it stands +// in for, and a comparison built on the drifted one reports a real difference as +// no difference. +// + +import Foundation +import TableProPluginKit + +/// One row of `SHOW INDEX` or `INFORMATION_SCHEMA.STATISTICS`, in the fields both spell the same. +struct MySQLIndexRow { + let table: String + let index: String + let column: String + let isNonUnique: Bool + let type: String + let prefixLength: Int? +} + +enum MySQLIndexGrouping { + /// Rows must arrive in index-position order: a composite index takes its column order from the + /// order they are appended, which is what the caller's `ORDER BY … SEQ_IN_INDEX` provides. + static func group(_ rows: [MySQLIndexRow]) -> [String: [PluginIndexInfo]] { + var byTable: [String: [String: (columns: [String], isUnique: Bool, type: String, prefixes: [String: Int])]] = [:] + + for row in rows { + var indexes = byTable[row.table] ?? [:] + if var existing = indexes[row.index] { + existing.columns.append(row.column) + if let prefix = row.prefixLength { + existing.prefixes[row.column] = prefix + } + indexes[row.index] = existing + } else { + var prefixes: [String: Int] = [:] + if let prefix = row.prefixLength { + prefixes[row.column] = prefix + } + indexes[row.index] = ( + columns: [row.column], isUnique: !row.isNonUnique, type: row.type, prefixes: prefixes + ) + } + byTable[row.table] = indexes + } + + return byTable.mapValues { indexes in + indexes + .map { name, info in + PluginIndexInfo( + name: name, columns: info.columns, isUnique: info.isUnique, + isPrimary: name == "PRIMARY", type: info.type, + columnPrefixes: info.prefixes.isEmpty ? nil : info.prefixes + ) + } + .sorted { $0.isPrimary && !$1.isPrimary } + } + } +} + +extension MySQLPluginDriver { + var providesBulkIndexFetch: Bool { true } + + /// `INFORMATION_SCHEMA.STATISTICS` is `SHOW INDEX` for every table at once, and reports the + /// same fields under the same names. `NON_UNIQUE` and `SUB_PART` are integers here where + /// `SHOW INDEX` returns text, so both are cast rather than read through a text accessor that + /// would depend on how the driver rendered an integer cell. + func fetchAllIndexes(schema: String?) async throws -> [String: [PluginIndexInfo]] { + let escapedDb = activeDatabaseName.replacingOccurrences(of: "'", with: "''") + let query = """ + SELECT + TABLE_NAME, INDEX_NAME, COLUMN_NAME, + CAST(NON_UNIQUE AS CHAR), INDEX_TYPE, CAST(SUB_PART AS CHAR) + FROM INFORMATION_SCHEMA.STATISTICS + WHERE TABLE_SCHEMA = '\(escapedDb)' + ORDER BY TABLE_NAME, INDEX_NAME, SEQ_IN_INDEX + """ + + let result = try await execute(query: query) + let rows = result.rows.compactMap { row -> MySQLIndexRow? in + guard let table = row[safe: 0]?.asText, + let index = row[safe: 1]?.asText, + let column = row[safe: 2]?.asText + else { return nil } + return MySQLIndexRow( + table: table, + index: index, + column: column, + isNonUnique: (row[safe: 3]?.asText) == "1", + type: (row[safe: 4]?.asText) ?? "BTREE", + prefixLength: (row[safe: 5]?.asText).flatMap { Int($0) } + ) + } + return MySQLIndexGrouping.group(rows) + } + + var providesBulkTableMetadataFetch: Bool { true } + + /// `SHOW TABLE STATUS` with no `WHERE` is the whole schema, in the same column order the + /// per-table read indexes into. + func fetchAllTableMetadata(schema: String?) async throws -> [String: PluginTableMetadata] { + let result = try await execute(query: "SHOW TABLE STATUS") + var metadata: [String: PluginTableMetadata] = [:] + for row in result.rows { + guard let name = row[safe: 0]?.asText else { continue } + metadata[name] = MySQLTableStatusRow.metadata(from: row, tableName: name) + } + return metadata + } +} + +enum MySQLTableStatusRow { + /// The positions `SHOW TABLE STATUS` documents, read in one place so the per-table and + /// whole-schema reads cannot index the same row differently. + static func metadata(from row: [PluginCellValue], tableName: String) -> PluginTableMetadata { + let dataSize = (row[safe: 6]?.asText).flatMap { Int64($0) } + let indexSize = (row[safe: 8]?.asText).flatMap { Int64($0) } + let comment = row[safe: 17]?.asText + + let totalSize: Int64? = { + guard let data = dataSize, let index = indexSize else { return nil } + return data + index + }() + + return PluginTableMetadata( + tableName: tableName, + dataSize: dataSize, + indexSize: indexSize, + totalSize: totalSize, + rowCount: (row[safe: 4]?.asText).flatMap { Int64($0) }, + comment: comment?.isEmpty == true ? nil : comment, + engine: row[safe: 1]?.asText, + collation: row[safe: 14]?.asText?.nilIfEmpty + ) + } +} diff --git a/Plugins/MySQLDriverPlugin/MySQLPluginDriver+Routines.swift b/Plugins/MySQLDriverPlugin/MySQLPluginDriver+Routines.swift index af025757f..49005733a 100644 --- a/Plugins/MySQLDriverPlugin/MySQLPluginDriver+Routines.swift +++ b/Plugins/MySQLDriverPlugin/MySQLPluginDriver+Routines.swift @@ -60,6 +60,8 @@ extension MySQLPluginDriver { return ddl } + var providesBulkTriggerFetch: Bool { true } + func fetchAllTriggers(schema: String?) async throws -> [PluginTriggerInfo] { try await triggerList(schema: routineSchema(schema), table: nil) } diff --git a/Plugins/MySQLDriverPlugin/MySQLPluginDriver.swift b/Plugins/MySQLDriverPlugin/MySQLPluginDriver.swift index 91c4606ed..5c1c89d4b 100644 --- a/Plugins/MySQLDriverPlugin/MySQLPluginDriver.swift +++ b/Plugins/MySQLDriverPlugin/MySQLPluginDriver.swift @@ -340,13 +340,24 @@ final class MySQLPluginDriver: PluginDatabaseDriver, @unchecked Sendable { } } + var providesBulkColumnFetch: Bool { true } + + /// `GENERATION_EXPRESSION` is projected here rather than looked up per table, because a caller + /// that takes the bulk list has to receive what `fetchColumns` would have given it. Without the + /// column the two reads disagree on generated columns alone, and a schema comparison built on + /// the bulk read reports a changed generation expression as no difference at all. func fetchAllColumns(schema: String?) async throws -> [String: [PluginColumnInfo]] { let dbName = _activeDatabase let escapedDb = dbName.replacingOccurrences(of: "'", with: "''") + let hasGenerationExpression = MySQLServerVersion.hasGenerationExpression( + banner: _serverVersion, isMariaDB: isMariaDB + ) + let generationProjection = hasGenerationExpression ? "GENERATION_EXPRESSION" : "NULL" let query = """ SELECT TABLE_NAME, COLUMN_NAME, COLUMN_TYPE, COLLATION_NAME, - IS_NULLABLE, COLUMN_KEY, COLUMN_DEFAULT, EXTRA, COLUMN_COMMENT + IS_NULLABLE, COLUMN_KEY, COLUMN_DEFAULT, EXTRA, COLUMN_COMMENT, + \(generationProjection) FROM INFORMATION_SCHEMA.COLUMNS WHERE TABLE_SCHEMA = '\(escapedDb)' ORDER BY TABLE_NAME, ORDINAL_POSITION @@ -390,7 +401,9 @@ final class MySQLPluginDriver: PluginDatabaseDriver, @unchecked Sendable { comment: comment?.isEmpty == false ? comment : nil, identityKind: mysqlIdentityKind(extra: extra), isGenerated: mysqlColumnIsGenerated(extra: extra), - allowedValues: allowedValues + allowedValues: allowedValues, + generationExpression: row[safe: 9]?.asText?.nilIfEmpty, + generationKind: mysqlGenerationKind(extra: extra) ) allColumns[tableName, default: []].append(column) @@ -403,41 +416,20 @@ final class MySQLPluginDriver: PluginDatabaseDriver, @unchecked Sendable { let safeTable = table.replacingOccurrences(of: "`", with: "``") let result = try await execute(query: "SHOW INDEX FROM `\(safeTable)`") - var indexMap: [String: (columns: [String], isUnique: Bool, type: String, prefixes: [String: Int])] = [:] - - for row in result.rows { + let rows = result.rows.compactMap { row -> MySQLIndexRow? in guard let indexName = row[safe: 2]?.asText, let columnName = row[safe: 4]?.asText - else { continue } - - let nonUnique = (row[safe: 1]?.asText) == "1" - let indexType = (row[safe: 10]?.asText) ?? "BTREE" - let subPart = (row[safe: 7]?.asText).flatMap { Int($0) } - - if var existing = indexMap[indexName] { - existing.columns.append(columnName) - if let subPart { - existing.prefixes[columnName] = subPart - } - indexMap[indexName] = existing - } else { - var prefixes: [String: Int] = [:] - if let subPart { - prefixes[columnName] = subPart - } - indexMap[indexName] = (columns: [columnName], isUnique: !nonUnique, type: indexType, prefixes: prefixes) - } + else { return nil } + return MySQLIndexRow( + table: table, + index: indexName, + column: columnName, + isNonUnique: (row[safe: 1]?.asText) == "1", + type: (row[safe: 10]?.asText) ?? "BTREE", + prefixLength: (row[safe: 7]?.asText).flatMap { Int($0) } + ) } - - return indexMap - .map { name, info in - PluginIndexInfo( - name: name, columns: info.columns, isUnique: info.isUnique, - isPrimary: name == "PRIMARY", type: info.type, - columnPrefixes: info.prefixes.isEmpty ? nil : info.prefixes - ) - } - .sorted { $0.isPrimary && !$1.isPrimary } + return MySQLIndexGrouping.group(rows)[table] ?? [] } func fetchForeignKeys(table: String, schema: String?) async throws -> [PluginForeignKeyInfo] { @@ -608,27 +600,7 @@ final class MySQLPluginDriver: PluginDatabaseDriver, @unchecked Sendable { guard let row = result.rows.first else { return PluginTableMetadata(tableName: table) } - - let engine = row[safe: 1]?.asText - let rowCount = (row[safe: 4]?.asText).flatMap { Int64($0) } - let dataSize = (row[safe: 6]?.asText).flatMap { Int64($0) } - let indexSize = (row[safe: 8]?.asText).flatMap { Int64($0) } - let comment = row[safe: 17]?.asText - - let totalSize: Int64? = { - guard let data = dataSize, let index = indexSize else { return nil } - return data + index - }() - - return PluginTableMetadata( - tableName: table, - dataSize: dataSize, - indexSize: indexSize, - totalSize: totalSize, - rowCount: rowCount, - comment: comment?.isEmpty == true ? nil : comment, - engine: engine - ) + return MySQLTableStatusRow.metadata(from: row, tableName: table) } // MARK: - Streaming diff --git a/Plugins/OracleDriverPlugin/Info.plist b/Plugins/OracleDriverPlugin/Info.plist index 0811e81d5..c271ca1f6 100644 --- a/Plugins/OracleDriverPlugin/Info.plist +++ b/Plugins/OracleDriverPlugin/Info.plist @@ -3,6 +3,6 @@ TableProPluginKitVersion - 19 + 20 diff --git a/Plugins/OracleDriverPlugin/OraclePluginDriver+Routines.swift b/Plugins/OracleDriverPlugin/OraclePluginDriver+Routines.swift index 54452901c..8b5ea7853 100644 --- a/Plugins/OracleDriverPlugin/OraclePluginDriver+Routines.swift +++ b/Plugins/OracleDriverPlugin/OraclePluginDriver+Routines.swift @@ -54,6 +54,13 @@ extension OraclePluginDriver { return body.uppercased().hasPrefix("CREATE") ? body : "CREATE OR REPLACE \(body)" } + /// False on purpose. `triggerList` scopes a whole-schema read on OWNER, the trigger's own + /// schema, and a per-table read on TABLE_OWNER, the subject table's. Those are different + /// questions for a trigger one schema owns on another's table, and a caller holding a bare + /// table name cannot tell them apart, so a comparison would accept a trigger from outside its + /// scope and miss one inside it. + var providesBulkTriggerFetch: Bool { false } + func fetchAllTriggers(schema: String?) async throws -> [PluginTriggerInfo] { try await triggerList(schema: routineSchema(schema), table: nil) } diff --git a/Plugins/PostgreSQLDriverPlugin/Info.plist b/Plugins/PostgreSQLDriverPlugin/Info.plist index ba0cdb6ae..d0b1fcf6b 100644 --- a/Plugins/PostgreSQLDriverPlugin/Info.plist +++ b/Plugins/PostgreSQLDriverPlugin/Info.plist @@ -3,7 +3,7 @@ TableProPluginKitVersion - 19 + 20 TableProProvidesDatabaseTypeIds PostgreSQL diff --git a/Plugins/PostgreSQLDriverPlugin/PostgreSQLPluginDriver+BulkMetadata.swift b/Plugins/PostgreSQLDriverPlugin/PostgreSQLPluginDriver+BulkMetadata.swift new file mode 100644 index 000000000..6ed075d85 --- /dev/null +++ b/Plugins/PostgreSQLDriverPlugin/PostgreSQLPluginDriver+BulkMetadata.swift @@ -0,0 +1,114 @@ +// +// PostgreSQLPluginDriver+BulkMetadata.swift +// PostgreSQLDriverPlugin +// +// Whole-schema reads of the metadata that otherwise costs one round trip per +// table. +// +// Each query here is the per-table statement with its `relname` predicate +// traded for a namespace predicate and the table name added to the projection, +// so the two forms answer with the same fields from the same catalogs. +// + +import Foundation +import TableProPluginKit + +extension PostgreSQLPluginDriver { + /// The bulk column read shares its projection builder with `fetchColumns`, so it reports + /// generated columns and their expressions exactly as the per-table read does. + var providesBulkColumnFetch: Bool { true } + + var providesBulkIndexFetch: Bool { true } + + func fetchAllIndexes(schema: String?) async throws -> [String: [PluginIndexInfo]] { + let schemaLiteral = escapeLiteral(schema ?? core.currentSchema) + let columnOrdering = versionedCapabilities.hasArrayPosition + ? "ORDER BY array_position(ix.indkey, a.attnum)" + : "ORDER BY a.attnum" + let query = """ + SELECT + t.relname AS table_name, + i.relname AS index_name, + ARRAY_AGG(a.attname \(columnOrdering)) AS columns, + ix.indisunique AS is_unique, + ix.indisprimary AS is_primary, + am.amname AS index_type, + pg_get_expr(ix.indpred, ix.indrelid) AS predicate + FROM pg_index ix + JOIN pg_class i ON i.oid = ix.indexrelid + JOIN pg_class t ON t.oid = ix.indrelid + JOIN pg_namespace n ON n.oid = t.relnamespace + JOIN pg_am am ON am.oid = i.relam + JOIN pg_attribute a ON a.attrelid = t.oid AND a.attnum = ANY(ix.indkey) + WHERE n.nspname = '\(schemaLiteral)' + GROUP BY t.relname, i.relname, ix.indisunique, ix.indisprimary, am.amname, ix.indpred, ix.indrelid + ORDER BY t.relname, ix.indisprimary DESC, i.relname + """ + let result = try await execute(query: query) + + var indexes: [String: [PluginIndexInfo]] = [:] + for row in result.rows { + guard row.count >= 6, let table = row[0].asText, + let index = PostgreSQLIndexRow.index(from: row) else { continue } + indexes[table, default: []].append(index) + } + return indexes + } + + var providesBulkTableMetadataFetch: Bool { true } + + func fetchAllTableMetadata(schema: String?) async throws -> [String: PluginTableMetadata] { + let schemaLiteral = escapeLiteral(schema ?? core.currentSchema) + let query = """ + SELECT + c.relname AS table_name, + pg_total_relation_size(c.oid) AS total_size, + pg_table_size(c.oid) AS data_size, + pg_indexes_size(c.oid) AS index_size, + c.reltuples::bigint AS row_count, + obj_description(c.oid, 'pg_class') AS comment + FROM pg_class c + JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE n.nspname = '\(schemaLiteral)' AND c.relkind IN ('r', 'p', 'm', 'f') + ORDER BY c.relname + """ + let result = try await execute(query: query) + + var metadata: [String: PluginTableMetadata] = [:] + for row in result.rows { + guard let name = row[safe: 0]?.asText else { continue } + let comment = row[safe: 5]?.asText + metadata[name] = PluginTableMetadata( + tableName: name, + dataSize: (row[safe: 2]?.asText).flatMap { Int64($0) }, + indexSize: (row[safe: 3]?.asText).flatMap { Int64($0) }, + totalSize: (row[safe: 1]?.asText).flatMap { Int64($0) }, + rowCount: (row[safe: 4]?.asText).flatMap { Int64($0) }, + comment: comment?.isEmpty == true ? nil : comment, + engine: "PostgreSQL" + ) + } + return metadata + } +} + +enum PostgreSQLIndexRow { + /// The shared shaping for a `pg_index` row, so the per-table and whole-schema reads cannot + /// disagree about how an index is named, ordered or typed. The row's first field is the table + /// name in the bulk form and the index name in the per-table form, so the offset is passed in. + static func index(from row: [PluginCellValue], offset: Int = 1) -> PluginIndexInfo? { + guard let name = row[safe: offset]?.asText, + let columnsText = row[safe: offset + 1]?.asText else { return nil } + let columns = columnsText + .trimmingCharacters(in: CharacterSet(charactersIn: "{}")) + .components(separatedBy: ",") + return PluginIndexInfo( + name: name, + columns: columns, + isUnique: row[safe: offset + 2]?.asText == "t", + isPrimary: row[safe: offset + 3]?.asText == "t", + type: row[safe: offset + 4]?.asText?.uppercased() ?? "BTREE", + whereClause: row[safe: offset + 5]?.asText + ) + } +} diff --git a/Plugins/PostgreSQLDriverPlugin/PostgreSQLPluginDriver+Routines.swift b/Plugins/PostgreSQLDriverPlugin/PostgreSQLPluginDriver+Routines.swift index 3e7c7d702..d2d49a526 100644 --- a/Plugins/PostgreSQLDriverPlugin/PostgreSQLPluginDriver+Routines.swift +++ b/Plugins/PostgreSQLDriverPlugin/PostgreSQLPluginDriver+Routines.swift @@ -49,6 +49,8 @@ extension PostgreSQLPluginDriver { return ddl } + var providesBulkTriggerFetch: Bool { true } + func fetchAllTriggers(schema: String?) async throws -> [PluginTriggerInfo] { let resolvedSchema = schema ?? currentSchema ?? "public" let query = PostgreSQLObjectQueries.triggerList(schema: resolvedSchema, table: nil) diff --git a/Plugins/PostgreSQLDriverPlugin/PostgreSQLPluginDriver.swift b/Plugins/PostgreSQLDriverPlugin/PostgreSQLPluginDriver.swift index a216a22f6..332b42e4e 100644 --- a/Plugins/PostgreSQLDriverPlugin/PostgreSQLPluginDriver.swift +++ b/Plugins/PostgreSQLDriverPlugin/PostgreSQLPluginDriver.swift @@ -225,7 +225,11 @@ class PostgreSQLPluginDriver: LibPQBackedDriver, @unchecked Sendable { } } + /// The namespace predicate is not optional. Without it the read matched `relname` alone, so two + /// schemas holding a table of the same name returned each other's indexes merged into one list, + /// which a comparison between those two schemas reports as neither side differing. func fetchIndexes(table: String, schema: String?) async throws -> [PluginIndexInfo] { + let schemaLiteral = escapeLiteral(schema ?? core.currentSchema) let columnOrdering = versionedCapabilities.hasArrayPosition ? "ORDER BY array_position(ix.indkey, a.attnum)" : "ORDER BY a.attnum" @@ -240,28 +244,15 @@ class PostgreSQLPluginDriver: LibPQBackedDriver, @unchecked Sendable { FROM pg_index ix JOIN pg_class i ON i.oid = ix.indexrelid JOIN pg_class t ON t.oid = ix.indrelid + JOIN pg_namespace n ON n.oid = t.relnamespace JOIN pg_am am ON am.oid = i.relam JOIN pg_attribute a ON a.attrelid = t.oid AND a.attnum = ANY(ix.indkey) - WHERE t.relname = '\(escapeLiteral(table))' + WHERE t.relname = '\(escapeLiteral(table))' AND n.nspname = '\(schemaLiteral)' GROUP BY i.relname, ix.indisunique, ix.indisprimary, am.amname, ix.indpred, ix.indrelid ORDER BY ix.indisprimary DESC, i.relname """ let result = try await execute(query: query) - return result.rows.compactMap { row -> PluginIndexInfo? in - guard row.count >= 5, let name = row[0].asText, let columnsStr = row[1].asText else { return nil } - let columns = columnsStr - .trimmingCharacters(in: CharacterSet(charactersIn: "{}")) - .components(separatedBy: ",") - let whereClause = row.count > 5 ? row[5].asText : nil - return PluginIndexInfo( - name: name, - columns: columns, - isUnique: row[2].asText == "t", - isPrimary: row[3].asText == "t", - type: row[4].asText?.uppercased() ?? "BTREE", - whereClause: whereClause - ) - } + return result.rows.compactMap { PostgreSQLIndexRow.index(from: $0, offset: 0) } } func fetchForeignKeys(table: String, schema: String?) async throws -> [PluginForeignKeyInfo] { diff --git a/Plugins/RedisDriverPlugin/Info.plist b/Plugins/RedisDriverPlugin/Info.plist index d1491ce46..ebc80fdb9 100644 --- a/Plugins/RedisDriverPlugin/Info.plist +++ b/Plugins/RedisDriverPlugin/Info.plist @@ -21,7 +21,7 @@ NSPrincipalClass $(PRODUCT_MODULE_NAME).RedisPlugin TableProPluginKitVersion - 19 + 20 TableProProvidesDatabaseTypeIds Redis diff --git a/Plugins/SQLExportPlugin/Info.plist b/Plugins/SQLExportPlugin/Info.plist index c6c8119ca..77b02579e 100644 --- a/Plugins/SQLExportPlugin/Info.plist +++ b/Plugins/SQLExportPlugin/Info.plist @@ -3,7 +3,7 @@ TableProPluginKitVersion - 19 + 20 TableProProvidesExportFormatIds sql diff --git a/Plugins/SQLImportPlugin/Info.plist b/Plugins/SQLImportPlugin/Info.plist index 69b0001d3..64945bd71 100644 --- a/Plugins/SQLImportPlugin/Info.plist +++ b/Plugins/SQLImportPlugin/Info.plist @@ -3,7 +3,7 @@ TableProPluginKitVersion - 19 + 20 TableProProvidesImportFormatIds sql diff --git a/Plugins/SQLiteDriverPlugin/Info.plist b/Plugins/SQLiteDriverPlugin/Info.plist index 9467f73c2..426f9a62f 100644 --- a/Plugins/SQLiteDriverPlugin/Info.plist +++ b/Plugins/SQLiteDriverPlugin/Info.plist @@ -3,7 +3,7 @@ TableProPluginKitVersion - 19 + 20 TableProProvidesDatabaseTypeIds SQLite diff --git a/Plugins/SQLiteDriverPlugin/SQLitePlugin.swift b/Plugins/SQLiteDriverPlugin/SQLitePlugin.swift index 3a4cb15f6..a6278447e 100644 --- a/Plugins/SQLiteDriverPlugin/SQLitePlugin.swift +++ b/Plugins/SQLiteDriverPlugin/SQLitePlugin.swift @@ -807,37 +807,62 @@ final class SQLitePluginDriver: PluginDatabaseDriver, @unchecked Sendable { } } + var providesBulkColumnFetch: Bool { true } + + /// `pragma_table_xinfo`, not `pragma_table_info`, for the same reason `fetchColumns` uses it: + /// `table_info` omits generated columns entirely, so the bulk read used to answer with a + /// shorter column list than the per-table read for the same table. A caller comparing two + /// schemas through the bulk read saw neither side's generated columns and reported them as + /// matching. `m.sql` rides along so the generation expressions are parsed from the CREATE + /// statement without a second round trip per table. func fetchAllColumns(schema: String?) async throws -> [String: [PluginColumnInfo]] { let query = """ - SELECT m.name AS tbl, p.cid, p.name, p.type, p."notnull", p.dflt_value, p.pk - FROM sqlite_master m, pragma_table_info(m.name) p + SELECT m.name AS tbl, p.cid, p.name, p.type, p."notnull", p.dflt_value, p.pk, + p.hidden, m.sql + FROM sqlite_master m, pragma_table_xinfo(m.name) p WHERE m.type = 'table' AND m.name NOT LIKE 'sqlite_%' ORDER BY m.name, p.cid """ let result = try await execute(query: query) var allColumns: [String: [PluginColumnInfo]] = [:] + var expressionsByTable: [String: [String: String]] = [:] for row in result.rows { - guard row.count >= 7, + guard row.count >= 9, let tableName = row[0].asText, let columnName = row[2].asText, let dataType = row[3].asText else { continue } + // hidden: 0 normal, 1 a virtual table's hidden column, 2 VIRTUAL generated, + // 3 STORED generated. + let hidden = row[7].asText.flatMap { Int($0) } ?? 0 + guard hidden != 1 else { continue } + let isNullable = row[4].asText == "0" let defaultValue = row[5].asText - // PRAGMA table_info pk column: 0 = not PK, 1+ = position in composite PK + // PRAGMA table_xinfo pk column: 0 = not PK, 1+ = position in composite PK let pkText = row[6].asText let isPrimaryKey = pkText != nil && pkText != "0" + let generationKind: GenerationKind? = hidden == 2 ? .virtual : (hidden == 3 ? .stored : nil) + + if generationKind != nil, expressionsByTable[tableName] == nil { + expressionsByTable[tableName] = SQLiteCheckConstraintParser.generationExpressions( + inCreateStatement: row[8].asText ?? "" + ) + } let column = PluginColumnInfo( name: columnName, dataType: dataType, isNullable: isNullable, isPrimaryKey: isPrimaryKey, - defaultValue: defaultValue + defaultValue: defaultValue, + isGenerated: generationKind != nil, + generationExpression: generationKind == nil ? nil : expressionsByTable[tableName]?[columnName], + generationKind: generationKind ) allColumns[tableName, default: []].append(column) @@ -899,41 +924,17 @@ final class SQLitePluginDriver: PluginDatabaseDriver, @unchecked Sendable { """ let result = try await execute(query: query) - var indexMap: [(name: String, isUnique: Bool, isPrimary: Bool, columns: [String])] = [] - var indexLookup: [String: Int] = [:] - - for row in result.rows { - guard row.count >= 4, - let indexName = row[0].asText else { continue } - - let isUnique = row[1].asText == "1" - let origin = row[2].asText ?? "c" - - if let idx = indexLookup[indexName] { - if let colName = row[3].asText { - indexMap[idx].columns.append(colName) - } - } else { - let columns: [String] = row[3].asText.map { [$0] } ?? [] - indexLookup[indexName] = indexMap.count - indexMap.append(( - name: indexName, - isUnique: isUnique, - isPrimary: origin == "pk", - columns: columns - )) - } - } - - return indexMap.map { entry in - PluginIndexInfo( - name: entry.name, - columns: entry.columns, - isUnique: entry.isUnique, - isPrimary: entry.isPrimary, - type: "BTREE" + let rows = result.rows.compactMap { row -> SQLiteIndexRow? in + guard row.count >= 4, let indexName = row[0].asText else { return nil } + return SQLiteIndexRow( + table: table, + index: indexName, + column: row[3].asText, + isUnique: row[1].asText == "1", + origin: row[2].asText ?? "c" ) - }.sorted { $0.isPrimary && !$1.isPrimary } + } + return SQLiteIndexGrouping.group(rows)[table] ?? [] } func fetchForeignKeys(table: String, schema: String?) async throws -> [PluginForeignKeyInfo] { diff --git a/Plugins/SQLiteDriverPlugin/SQLitePluginDriver+BulkMetadata.swift b/Plugins/SQLiteDriverPlugin/SQLitePluginDriver+BulkMetadata.swift new file mode 100644 index 000000000..5d347e5fa --- /dev/null +++ b/Plugins/SQLiteDriverPlugin/SQLitePluginDriver+BulkMetadata.swift @@ -0,0 +1,120 @@ +// +// SQLitePluginDriver+BulkMetadata.swift +// SQLiteDriverPlugin +// +// Whole-schema reads of the metadata that otherwise costs one round trip per +// table. +// +// SQLite reaches these through its table-valued pragma functions, so the +// per-table `PRAGMA index_list` becomes one join against `sqlite_master`. The +// shaping is shared with the per-table read rather than written twice, because +// two copies of one grouping is how a bulk read drifts from the read it stands +// in for. +// + +import Foundation +import TableProPluginKit + +/// One row of `pragma_index_list` joined to `pragma_index_info`, in the fields both forms carry. +struct SQLiteIndexRow { + let table: String + let index: String + let column: String? + let isUnique: Bool + let origin: String +} + +enum SQLiteIndexGrouping { + /// Rows must arrive in index-position order, which is what the caller's + /// `ORDER BY il.seq, ii.seqno` provides: a composite index takes its column order from the + /// order they are appended. + static func group(_ rows: [SQLiteIndexRow]) -> [String: [PluginIndexInfo]] { + var order: [String: [String]] = [:] + var entries: [String: [String: (isUnique: Bool, isPrimary: Bool, columns: [String])]] = [:] + + for row in rows { + var tableEntries = entries[row.table] ?? [:] + if var existing = tableEntries[row.index] { + if let column = row.column { + existing.columns.append(column) + } + tableEntries[row.index] = existing + } else { + tableEntries[row.index] = ( + isUnique: row.isUnique, + isPrimary: row.origin == "pk", + columns: row.column.map { [$0] } ?? [] + ) + order[row.table, default: []].append(row.index) + } + entries[row.table] = tableEntries + } + + var result: [String: [PluginIndexInfo]] = [:] + for (table, names) in order { + result[table] = names.compactMap { name -> PluginIndexInfo? in + guard let entry = entries[table]?[name] else { return nil } + return PluginIndexInfo( + name: name, + columns: entry.columns, + isUnique: entry.isUnique, + isPrimary: entry.isPrimary, + type: "BTREE" + ) + } + .sorted { $0.isPrimary && !$1.isPrimary } + } + return result + } +} + +extension SQLitePluginDriver { + var providesBulkIndexFetch: Bool { true } + + func fetchAllIndexes(schema: String?) async throws -> [String: [PluginIndexInfo]] { + let query = """ + SELECT m.name AS tbl, il.name, il."unique", il.origin, ii.name AS col_name + FROM sqlite_master m + JOIN pragma_index_list(m.name) il + LEFT JOIN pragma_index_info(il.name) ii ON 1=1 + WHERE m.type = 'table' AND m.name NOT LIKE 'sqlite_%' + ORDER BY m.name, il.seq, ii.seqno + """ + let result = try await execute(query: query) + + let rows = result.rows.compactMap { row -> SQLiteIndexRow? in + guard row.count >= 5, + let table = row[0].asText, + let index = row[1].asText else { return nil } + return SQLiteIndexRow( + table: table, + index: index, + column: row[4].asText, + isUnique: row[2].asText == "1", + origin: row[3].asText ?? "c" + ) + } + return SQLiteIndexGrouping.group(rows) + } + + var providesBulkTableMetadataFetch: Bool { true } + + /// `rowCount` is deliberately absent. SQLite stores no row count, so the per-table read counts + /// rows with a capped scan, and doing that once per table is the cost this whole-schema read + /// exists to remove. Everything the metadata says about a table's *structure* is here; a caller + /// that wants a count asks `fetchTableMetadata` for the one table it cares about. + func fetchAllTableMetadata(schema: String?) async throws -> [String: PluginTableMetadata] { + let query = """ + SELECT name FROM sqlite_master + WHERE type = 'table' AND name NOT LIKE 'sqlite_%' + ORDER BY name + """ + let result = try await execute(query: query) + var metadata: [String: PluginTableMetadata] = [:] + for row in result.rows { + guard let name = row[safe: 0]?.asText else { continue } + metadata[name] = PluginTableMetadata(tableName: name, engine: "SQLite") + } + return metadata + } +} diff --git a/Plugins/SQLiteDriverPlugin/SQLitePluginDriver+Triggers.swift b/Plugins/SQLiteDriverPlugin/SQLitePluginDriver+Triggers.swift index 7a7564b6f..5ab772be2 100644 --- a/Plugins/SQLiteDriverPlugin/SQLitePluginDriver+Triggers.swift +++ b/Plugins/SQLiteDriverPlugin/SQLitePluginDriver+Triggers.swift @@ -7,6 +7,8 @@ import Foundation import TableProPluginKit extension SQLitePluginDriver { + var providesBulkTriggerFetch: Bool { true } + func fetchAllTriggers(schema: String?) async throws -> [PluginTriggerInfo] { try await sqliteTriggerList(table: nil) } diff --git a/Plugins/SnowflakeDriverPlugin/Info.plist b/Plugins/SnowflakeDriverPlugin/Info.plist index 34dbfb803..7eb4de381 100644 --- a/Plugins/SnowflakeDriverPlugin/Info.plist +++ b/Plugins/SnowflakeDriverPlugin/Info.plist @@ -5,6 +5,6 @@ TableProMinAppVersion 0.48.0 TableProPluginKitVersion - 19 + 20 diff --git a/Plugins/SurrealDBDriverPlugin/Info.plist b/Plugins/SurrealDBDriverPlugin/Info.plist index aa408a600..535b56f87 100644 --- a/Plugins/SurrealDBDriverPlugin/Info.plist +++ b/Plugins/SurrealDBDriverPlugin/Info.plist @@ -3,7 +3,7 @@ TableProPluginKitVersion - 19 + 20 TableProProvidesDatabaseTypeIds SurrealDB diff --git a/Plugins/TableProPluginKit/PluginDatabaseDriver.swift b/Plugins/TableProPluginKit/PluginDatabaseDriver.swift index b1d1caeeb..753000029 100644 --- a/Plugins/TableProPluginKit/PluginDatabaseDriver.swift +++ b/Plugins/TableProPluginKit/PluginDatabaseDriver.swift @@ -100,6 +100,7 @@ public protocol PluginDatabaseDriver: AnyObject, Sendable { func fetchTriggers(table: String, schema: String?) async throws -> [PluginTriggerInfo] func fetchCheckConstraints(table: String, schema: String?) async throws -> [PluginCheckConstraintInfo] func fetchAllTriggers(schema: String?) async throws -> [PluginTriggerInfo] + var providesBulkTriggerFetch: Bool { get } func fetchTriggerDDL(_ trigger: PluginTriggerInfo) async throws -> String func fetchRoutines(schema: String?) async throws -> [PluginRoutineInfo] func fetchRoutineDDL(_ routine: PluginRoutineInfo) async throws -> String @@ -134,9 +135,14 @@ public protocol PluginDatabaseDriver: AnyObject, Sendable { func fetchApproximateRowCount(table: String, schema: String?) async throws -> Int? func fetchAllColumns(schema: String?) async throws -> [String: [PluginColumnInfo]] + var providesBulkColumnFetch: Bool { get } func sampleFieldPaths(table: String, schema: String?, limit: Int) async throws -> [PluginFieldPath] func fetchAllForeignKeys(schema: String?) async throws -> [String: [PluginForeignKeyInfo]] var providesBulkForeignKeyFetch: Bool { get } + func fetchAllIndexes(schema: String?) async throws -> [String: [PluginIndexInfo]] + var providesBulkIndexFetch: Bool { get } + func fetchAllTableMetadata(schema: String?) async throws -> [String: PluginTableMetadata] + var providesBulkTableMetadataFetch: Bool { get } func fetchAllDatabaseMetadata() async throws -> [PluginDatabaseMetadata] func fetchDependentTypes(table: String, schema: String?) async throws -> [(name: String, labels: [String])] func fetchDependentSequences(table: String, schema: String?) async throws -> [(name: String, ddl: String)] @@ -308,6 +314,12 @@ public extension PluginDatabaseDriver { func fetchAllTriggers(schema: String?) async throws -> [PluginTriggerInfo] { [] } + /// Answers whether `fetchAllTriggers` lists a whole schema's triggers. The default above + /// returns nothing rather than looping, so a caller that wants triggers has to know whether + /// this driver answers at all before it decides to ask per table. False is the safe answer: it + /// costs a round trip per table and reports every trigger, where a wrong true reports none. + var providesBulkTriggerFetch: Bool { false } + func fetchTriggerDDL(_ trigger: PluginTriggerInfo) async throws -> String { if let definition = trigger.definition, !definition.isEmpty { return definition } guard let table = trigger.table else { @@ -422,6 +434,13 @@ public extension PluginDatabaseDriver { func fetchApproximateRowCount(table: String, schema: String?) async throws -> Int? { nil } + /// Answers whether `fetchAllColumns` is a single query rather than the N+1 default below, and + /// whether it reports every column `fetchColumns` reports. Both halves matter: a bulk query + /// that omits generated columns or their expressions is not a substitute for the per-table + /// read, and a caller that compares two schemas would report the missing detail as no + /// difference at all. + var providesBulkColumnFetch: Bool { false } + /// Default: fetches columns per-table sequentially (N+1 round-trips). /// SQL drivers should override with a single bulk query (e.g. INFORMATION_SCHEMA.COLUMNS). func fetchAllColumns(schema: String?) async throws -> [String: [PluginColumnInfo]] { @@ -458,6 +477,39 @@ public extension PluginDatabaseDriver { return result } + /// Answers whether `fetchAllIndexes` is a single query rather than the N+1 default below. + var providesBulkIndexFetch: Bool { false } + + /// Default: fetches indexes per-table sequentially (N+1 round-trips). + /// SQL drivers should override with a single bulk query (e.g. INFORMATION_SCHEMA.STATISTICS). + func fetchAllIndexes(schema: String?) async throws -> [String: [PluginIndexInfo]] { + let tables = try await fetchTables(schema: schema) + var result: [String: [PluginIndexInfo]] = [:] + for table in tables { + let indexes = try await fetchIndexes(table: table.name, schema: schema) + if !indexes.isEmpty { result[table.name] = indexes } + } + return result + } + + /// Answers whether `fetchAllTableMetadata` is a single query rather than the N+1 default below. + var providesBulkTableMetadataFetch: Bool { false } + + /// Default: fetches metadata per-table sequentially (N+1 round-trips). + /// SQL drivers should override with a single bulk query (e.g. SHOW TABLE STATUS with no filter). + /// + /// A table whose metadata cannot be read is left out rather than throwing. The caller wants + /// the descriptive fields, and one unreadable table is not a reason to lose the other 199. + func fetchAllTableMetadata(schema: String?) async throws -> [String: PluginTableMetadata] { + let tables = try await fetchTables(schema: schema) + var result: [String: PluginTableMetadata] = [:] + for table in tables { + guard let metadata = try? await fetchTableMetadata(table: table.name, schema: schema) else { continue } + result[table.name] = metadata + } + return result + } + func fetchAllDatabaseMetadata() async throws -> [PluginDatabaseMetadata] { let dbs = try await fetchDatabases() var result: [PluginDatabaseMetadata] = [] diff --git a/Plugins/TeradataDriverPlugin/Info.plist b/Plugins/TeradataDriverPlugin/Info.plist index 0811e81d5..c271ca1f6 100644 --- a/Plugins/TeradataDriverPlugin/Info.plist +++ b/Plugins/TeradataDriverPlugin/Info.plist @@ -3,6 +3,6 @@ TableProPluginKitVersion - 19 + 20 diff --git a/Plugins/TeradataDriverPlugin/TeradataPluginDriver+Routines.swift b/Plugins/TeradataDriverPlugin/TeradataPluginDriver+Routines.swift index a899b11ca..680fd3fda 100644 --- a/Plugins/TeradataDriverPlugin/TeradataPluginDriver+Routines.swift +++ b/Plugins/TeradataDriverPlugin/TeradataPluginDriver+Routines.swift @@ -63,6 +63,12 @@ extension TeradataPluginDriver { return text } + /// False until the whole-schema scope is checked against a live server. This driver has no + /// per-table trigger read, so the protocol default answers with nothing and a comparison has + /// never listed its triggers; opting in here would change what is compared rather than only + /// how fast it is read. + var providesBulkTriggerFetch: Bool { false } + func fetchAllTriggers(schema: String?) async throws -> [PluginTriggerInfo] { try await teradataTriggerList(schema: schema, table: nil) } diff --git a/Plugins/TrinoDriverPlugin/Info.plist b/Plugins/TrinoDriverPlugin/Info.plist index 0811e81d5..c271ca1f6 100644 --- a/Plugins/TrinoDriverPlugin/Info.plist +++ b/Plugins/TrinoDriverPlugin/Info.plist @@ -3,6 +3,6 @@ TableProPluginKitVersion - 19 + 20 diff --git a/Plugins/XLSXExportPlugin/Info.plist b/Plugins/XLSXExportPlugin/Info.plist index 7194edd0f..4b81c4c36 100644 --- a/Plugins/XLSXExportPlugin/Info.plist +++ b/Plugins/XLSXExportPlugin/Info.plist @@ -3,7 +3,7 @@ TableProPluginKitVersion - 19 + 20 TableProProvidesExportFormatIds xlsx diff --git a/TablePro/Core/Compare/CompareMetadataService.swift b/TablePro/Core/Compare/CompareMetadataService.swift index 298b4e315..ef6c4b7fe 100644 --- a/TablePro/Core/Compare/CompareMetadataService.swift +++ b/TablePro/Core/Compare/CompareMetadataService.swift @@ -104,15 +104,20 @@ internal struct CompareMetadataService { /// regard to case because engines disagree on identifier folding. A comparison passes nil and /// reads the whole scope; a copy of one table would otherwise pay four round trips for every /// other table in the database. + /// + /// `profile` says which of the four reads this caller actually looks at. A data comparison + /// pairs tables and reads their rows, so the indexes and the table metadata it used to fetch + /// for every table were two round trips per table spent on fields it never reads. internal func tableReads( for endpoint: DatabaseEndpoint, connection: DatabaseConnection, includeViews: Bool, + profile: TableReadProfile = .structure, names: Set? = nil ) async throws -> [TableStructureRead] { try await manager.ensureConnected(connection) let schema = endpoint.schema - let concurrency = Self.metadataConcurrency(for: endpoint.databaseType) + let databaseType = endpoint.databaseType let wanted = names.map { Set($0.map { $0.lowercased() }) } return try await manager.withMetadataDriver(scope: endpoint.scope) { driver in @@ -122,13 +127,34 @@ internal struct CompareMetadataService { let kind = CompareTableKindClassifier.kind(of: table) return kind == .table || includeViews } - - return try await Self.map(tables, concurrency: concurrency) { table in - await Self.read(table: table, schema: table.schema ?? schema, using: plugin) - } + return try await Self.read( + tables: tables, schema: schema, profile: profile, + narrowed: wanted != nil, databaseType: databaseType, using: plugin + ) } } + /// Both sides at once. + /// + /// A comparison is two independent reads and used to run them one after the other, so the wall + /// clock was the sum of the two. `SessionDriverGate` is a FIFO queue per connection rather than + /// a lock a task can deadlock itself on, and these two reads are concurrent rather than nested, + /// so the worst case where both sides route to one connection's shared driver is that they + /// serialise, which is what they did before. + internal func bothSideTableReads( + context: CompareRunner.Context, + includeViews: Bool, + profile: TableReadProfile + ) async throws -> (source: [TableStructureRead], target: [TableStructureRead]) { + async let source = tableReads( + for: context.source, connection: context.sourceConnection, includeViews: includeViews, profile: profile + ) + async let target = tableReads( + for: context.target, connection: context.targetConnection, includeViews: includeViews, profile: profile + ) + return try await (source, target) + } + /// `fetchRoutines` supersedes the old per-kind pair and carries `identity`, which is what /// `fetchRoutineDDL` needs to address an overloaded routine again. A routine whose DDL cannot /// be read is still listed, with an empty definition, so it shows as present rather than @@ -161,9 +187,12 @@ internal struct CompareMetadataService { } } - /// There is no schema-wide trigger fetch on the driver protocol, so the tables the structure - /// read already listed are the ones asked. A trigger on a table that is not in scope is not in - /// scope either. + /// A trigger on a table that is not in scope is not in scope either, so the tables the + /// structure read already listed are the ones kept. + /// + /// The whole-schema read is one query where the driver has one. Where it does not, the + /// protocol's default answers with nothing rather than looping, so the per-table read is the + /// only correct fallback and `providesBulkTriggerFetch` is what tells the two apart. internal func triggerReads( for endpoint: DatabaseEndpoint, connection: DatabaseConnection, @@ -171,24 +200,61 @@ internal struct CompareMetadataService { ) async throws -> [RoutineSourceRead] { try await manager.ensureConnected(connection) let schema = endpoint.schema + let inScope = Set(tables.map { $0.lowercased() }) return try await manager.withMetadataDriver(scope: endpoint.scope) { driver in guard let plugin = Self.pluginDriver(from: driver) else { return [] } - var reads: [RoutineSourceRead] = [] - for table in tables { - try Task.checkCancellation() - guard let triggers = try? await plugin.fetchTriggers(table: table, schema: schema) else { continue } - reads += triggers.map { trigger in - RoutineSourceRead( - name: trigger.name, - kind: .trigger, - schema: trigger.schema ?? schema, - signature: trigger.table ?? table, - source: trigger.definition ?? trigger.statement - ) - } + guard plugin.providesBulkTriggerFetch else { + return try await Self.perTableTriggerReads(tables: tables, schema: schema, using: plugin) } - return reads + /// A failed whole-schema query is not an answer of "no triggers". Swallowing it made an + /// empty set authoritative on one side, so every trigger on the other side read as a + /// real difference and the script offered to drop or create all of them. + let triggers: [PluginTriggerInfo] + do { + triggers = try await plugin.fetchAllTriggers(schema: schema) + } catch is CancellationError { + throw CancellationError() + } catch { + Self.logger.warning( + "Whole-schema trigger read failed, falling back per table: \(error.localizedDescription, privacy: .public)" + ) + return try await Self.perTableTriggerReads(tables: tables, schema: schema, using: plugin) + } + return triggers + .filter { trigger in + guard let table = trigger.table?.lowercased() else { return true } + return inScope.contains(table) + } + .map { Self.read($0, schema: schema, fallbackTable: nil) } + } + } + + nonisolated private static func perTableTriggerReads( + tables: [String], + schema: String?, + using plugin: any PluginDatabaseDriver + ) async throws -> [RoutineSourceRead] { + var reads: [RoutineSourceRead] = [] + for table in tables { + try Task.checkCancellation() + guard let triggers = try? await plugin.fetchTriggers(table: table, schema: schema) else { continue } + reads += triggers.map { read($0, schema: schema, fallbackTable: table) } } + return reads + } + + nonisolated private static func read( + _ trigger: PluginTriggerInfo, + schema: String?, + fallbackTable: String? + ) -> RoutineSourceRead { + RoutineSourceRead( + name: trigger.name, + kind: .trigger, + schema: trigger.schema ?? schema, + signature: trigger.table ?? fallbackTable, + source: trigger.definition ?? trigger.statement + ) } internal func viewDefinitions( @@ -221,16 +287,169 @@ internal struct CompareMetadataService { // MARK: - Helpers + /// Reads a whole scope, asking each driver for the cheapest form of every read it needs. + /// + /// The per-table form costs four round trips per table, which is what made a 200-table + /// comparison 800 round trips a side. `PluginDatabaseDriver` already answers three of the four + /// for a whole schema in one query, and now answers the fourth, so a driver that declares them + /// is read in a handful of statements no matter how many tables it holds. + /// + /// The fan-out this replaces bought nothing. `withMetadataDriver` yields one driver, and a + /// driver dispatches its statements onto its own serial queue, so four concurrent reads of one + /// connection queue behind each other. Its gate was `supportsConnectionPooling`, which answers + /// whether a *second* connection is safe, not whether one connection can run two statements. + /// + /// `narrowed` says the caller asked for specific tables, so the whole-schema queries would read + /// the rest of the database to throw it away. Those callers keep the per-table reads. + nonisolated internal static func read( + tables: [PluginTableInfo], + schema: String?, + profile: TableReadProfile, + narrowed: Bool, + databaseType: DatabaseType, + using plugin: any PluginDatabaseDriver + ) async throws -> [TableStructureRead] { + let bulk = narrowed + ? BulkMetadata() + : await BulkMetadata(schema: schema, profile: profile, tables: tables, plugin: plugin) + + /// Whatever the whole-schema reads did not answer is still one statement per table, and a + /// driver whose statements are independent requests rather than one serialised socket can + /// overlap them. Cloudflare D1 is the case that matters: it has only the bulk foreign-key + /// read, so everything else would otherwise be three remote calls per table in a row. + /// A driver that cannot take a second connection stays serial, which is the gate the + /// previous fan-out used and the conservative answer for a driver with no queue of its own. + let concurrency = bulk.answersEveryRead(for: profile) || !databaseType.supportsConnectionPooling + ? 1 + : Self.fallbackConcurrency + + return try await map(tables, concurrency: concurrency) { table in + await read(table: table, schema: table.schema ?? schema, profile: profile, bulk: bulk, using: plugin) + } + } + + nonisolated private static let fallbackConcurrency = 4 + + nonisolated private static func map( + _ tables: [PluginTableInfo], + concurrency: Int, + _ transform: @escaping @Sendable (PluginTableInfo) async -> TableStructureRead + ) async throws -> [TableStructureRead] { + guard concurrency > 1, tables.count > 1 else { + var results: [TableStructureRead] = [] + results.reserveCapacity(tables.count) + for table in tables { + try Task.checkCancellation() + results.append(await transform(table)) + } + return results + } + + return try await withThrowingTaskGroup(of: (Int, TableStructureRead).self) { group in + var results = [TableStructureRead?](repeating: nil, count: tables.count) + var next = 0 + + while next < tables.count, next < concurrency { + let index = next + group.addTask { (index, await transform(tables[index])) } + next += 1 + } + while let (index, result) = try await group.next() { + results[index] = result + try Task.checkCancellation() + guard next < tables.count else { continue } + let queued = next + group.addTask { (queued, await transform(tables[queued])) } + next += 1 + } + return results.compactMap { $0 } + } + } + + /// What a whole-schema read produced, or nothing where the driver has no single-query form for + /// it and the per-table read still has to run. + private struct BulkMetadata: Sendable { + var columns: [String: [PluginColumnInfo]]? + var indexes: [String: [PluginIndexInfo]]? + var foreignKeys: [String: [PluginForeignKeyInfo]]? + var tableMetadata: [String: PluginTableMetadata]? + + /// The folded spellings that name exactly one table in this scope. A folded fallback is + /// only safe for those. + private var unambiguousFolded: Set = [] + + init() {} + + /// A whole-schema query that fails takes nothing with it: the read falls back to the + /// per-table form, which reports a failure against the one table it belongs to rather than + /// losing the comparison. That is the same rule the per-table read already followed. + init( + schema: String?, + profile: TableReadProfile, + tables: [PluginTableInfo], + plugin: any PluginDatabaseDriver + ) async { + var counts: [String: Int] = [:] + for table in tables { + counts[table.name.lowercased(), default: 0] += 1 + } + unambiguousFolded = Set(counts.filter { $0.value == 1 }.keys) + + if plugin.providesBulkColumnFetch { + columns = try? await plugin.fetchAllColumns(schema: schema) + } + if profile.wantsIndexes, plugin.providesBulkIndexFetch { + indexes = try? await plugin.fetchAllIndexes(schema: schema) + } + if profile.wantsForeignKeys, plugin.providesBulkForeignKeyFetch { + foreignKeys = try? await plugin.fetchAllForeignKeys(schema: schema) + } + if profile.wantsTableMetadata, plugin.providesBulkTableMetadataFetch { + tableMetadata = try? await plugin.fetchAllTableMetadata(schema: schema) + } + } + + /// True when nothing is left for the per-table path, so the fan-out over it is not worth + /// starting. + func answersEveryRead(for profile: TableReadProfile) -> Bool { + guard columns != nil else { return false } + if profile.wantsIndexes, indexes == nil { return false } + if profile.wantsForeignKeys, foreignKeys == nil { return false } + if profile.wantsTableMetadata, tableMetadata == nil { return false } + return true + } + + /// Engines disagree on identifier folding, so a name that was stored one way and listed + /// another still has to find its entry. + /// + /// The folded fallback is refused where two tables in this scope fold to the same + /// spelling. The index and foreign key maps are sparse, so a table with none has no exact + /// entry, and PostgreSQL allows `"Foo"` beside `"foo"`: a folded match there handed one + /// table's indexes to the other, and a DROP INDEX generated from that names the index + /// alone, so it would have dropped the real one. + func lookup(_ map: [String: Value]?, _ name: String) -> Value? { + guard let map else { return nil } + if let exact = map[name] { return exact } + let folded = name.lowercased() + guard unambiguousFolded.contains(folded) else { return nil } + return map.first { $0.key.lowercased() == folded }?.value + } + } + nonisolated private static func read( table: PluginTableInfo, schema: String?, + profile: TableReadProfile, + bulk: BulkMetadata, using plugin: any PluginDatabaseDriver ) async -> TableStructureRead { do { - let columns = try await plugin.fetchColumns(table: table.name, schema: schema) - let indexes = (try? await plugin.fetchIndexes(table: table.name, schema: schema)) ?? [] - let foreignKeys = (try? await plugin.fetchForeignKeys(table: table.name, schema: schema)) ?? [] - let metadata = try? await plugin.fetchTableMetadata(table: table.name, schema: schema) + let columns = try await columns(of: table, schema: schema, bulk: bulk, using: plugin) + let indexes = await indexes(of: table, schema: schema, profile: profile, bulk: bulk, using: plugin) + let foreignKeys = await foreignKeys( + of: table, schema: schema, profile: profile, bulk: bulk, using: plugin + ) + let metadata = await metadata(of: table, schema: schema, profile: profile, bulk: bulk, using: plugin) return TableStructureRead( table: table, columns: columns, indexes: indexes, foreignKeys: foreignKeys, metadata: metadata, failure: nil @@ -246,55 +465,76 @@ internal struct CompareMetadataService { } } - /// A driver that cannot be pooled reaches a different database on a second connection, so its - /// reads stay serial. Everything else fans out, because the old code paid one round trip per - /// table for columns, indexes, foreign keys and metadata in strict sequence. - nonisolated private static func metadataConcurrency(for databaseType: DatabaseType) -> Int { - databaseType.supportsConnectionPooling ? 4 : 1 + /// A table with no columns is not a real answer, so a name missing from the whole-schema list + /// is read on its own. That keeps the per-table failure reporting, which an absent dictionary + /// entry cannot express. + nonisolated private static func columns( + of table: PluginTableInfo, + schema: String?, + bulk: BulkMetadata, + using plugin: any PluginDatabaseDriver + ) async throws -> [PluginColumnInfo] { + if let columns = bulk.lookup(bulk.columns, table.name), !columns.isEmpty { return columns } + return try await plugin.fetchColumns(table: table.name, schema: schema) } - nonisolated private static func map( - _ elements: [Element], - concurrency: Int, - _ transform: @escaping @Sendable (Element) async -> Result - ) async throws -> [Result] { - guard concurrency > 1, elements.count > 1 else { - var results: [Result] = [] - for element in elements { - try Task.checkCancellation() - results.append(await transform(element)) - } - return results - } - - return try await withThrowingTaskGroup(of: (Int, Result).self) { group in - var results = [Result?](repeating: nil, count: elements.count) - var next = 0 - var running = 0 + /// An empty index list is a real answer, unlike an empty column list, so a table absent from a + /// whole-schema read has no indexes rather than needing a read of its own. + nonisolated private static func indexes( + of table: PluginTableInfo, + schema: String?, + profile: TableReadProfile, + bulk: BulkMetadata, + using plugin: any PluginDatabaseDriver + ) async -> [PluginIndexInfo] { + guard profile.wantsIndexes else { return [] } + guard bulk.indexes == nil else { return bulk.lookup(bulk.indexes, table.name) ?? [] } + return (try? await plugin.fetchIndexes(table: table.name, schema: schema)) ?? [] + } - while next < elements.count && running < concurrency { - let index = next - group.addTask { (index, await transform(elements[index])) } - next += 1 - running += 1 - } + nonisolated private static func foreignKeys( + of table: PluginTableInfo, + schema: String?, + profile: TableReadProfile, + bulk: BulkMetadata, + using plugin: any PluginDatabaseDriver + ) async -> [PluginForeignKeyInfo] { + guard profile.wantsForeignKeys else { return [] } + guard bulk.foreignKeys == nil else { return bulk.lookup(bulk.foreignKeys, table.name) ?? [] } + return (try? await plugin.fetchForeignKeys(table: table.name, schema: schema)) ?? [] + } - while let (index, result) = try await group.next() { - results[index] = result - running -= 1 - try Task.checkCancellation() - if next < elements.count { - let queued = next - group.addTask { (queued, await transform(elements[queued])) } - next += 1 - running += 1 - } - } - return results.compactMap { $0 } - } + nonisolated private static func metadata( + of table: PluginTableInfo, + schema: String?, + profile: TableReadProfile, + bulk: BulkMetadata, + using plugin: any PluginDatabaseDriver + ) async -> PluginTableMetadata? { + guard profile.wantsTableMetadata else { return nil } + guard bulk.tableMetadata == nil else { return bulk.lookup(bulk.tableMetadata, table.name) } + return try? await plugin.fetchTableMetadata(table: table.name, schema: schema) } nonisolated internal static func pluginDriver(from driver: DatabaseDriver) -> (any PluginDatabaseDriver)? { (driver as? PluginDriverAdapter)?.schemaPluginDriver } } + +/// Which of the four per-table reads a caller actually looks at. +internal struct TableReadProfile: Sendable { + internal let wantsIndexes: Bool + internal let wantsForeignKeys: Bool + internal let wantsTableMetadata: Bool + + internal static let structure = TableReadProfile( + wantsIndexes: true, wantsForeignKeys: true, wantsTableMetadata: true + ) + + /// A data comparison pairs tables by name, reads the columns they share and walks their rows. + /// It reads foreign keys to order the statements it writes, and it never looks at an index or + /// at a storage engine. + internal static let data = TableReadProfile( + wantsIndexes: false, wantsForeignKeys: true, wantsTableMetadata: false + ) +} diff --git a/TablePro/Core/Compare/CompareRunner+Data.swift b/TablePro/Core/Compare/CompareRunner+Data.swift index f4948e1c7..78a87ac31 100644 --- a/TablePro/Core/Compare/CompareRunner+Data.swift +++ b/TablePro/Core/Compare/CompareRunner+Data.swift @@ -17,18 +17,57 @@ import Foundation import TableProPluginKit internal extension CompareRunner { - func runDataCompare(_ context: Context) async throws { + /// Lists the tables the two sides share, without reading a row. + /// + /// Data mode used to spend its first Compare on this list, because plans arrive unticked and a + /// comparison of nothing reads nothing, so a data sync cost two Compares and paid the whole + /// metadata read twice. + /// + /// The guard is `hasLoadedDataPlans` rather than an empty list, because two sides that share no + /// table produce an empty list from a load that did happen, and a caller on a validation pass + /// would reload it forever. + func loadDataPlans() { + guard session.mode == .data, session.canCompare, !session.hasLoadedDataPlans else { return } + session.errorMessage = nil + + let claim = session.currentClaim + session.runTask = Task { [session] in + session.activity = .connecting + defer { session.activity = .idle } + do { + let context = try resolveContext() + if let refusal = try await capabilityRefusal(context) { + guard session.owns(claim) else { return } + session.errorMessage = refusal + return + } + let plans = try await buildPlans(context) + /// The pair may have moved while this was reading. Publishing now would put one + /// pair's tables, columns and snapshots behind another pair's Compare. + guard session.owns(claim) else { return } + session.adoptDataPlans(plans) + } catch is CancellationError { + } catch { + guard session.owns(claim) else { return } + session.errorMessage = error.localizedDescription + } + } + } + + func runDataCompare(_ context: Context, claim: CompareSyncSession.RunClaim) async throws { if let refusal = rowService.concurrentReadRefusal(source: context.source, target: context.target) { throw CompareSyncError.unsupportedOperation(refusal) } - var plans = try await buildPlans(context) - if !session.pendingSelection.isEmpty { - for index in plans.indices { - plans[index].isEnabled = session.pendingSelection.contains(plans[index].id) - } - session.pendingSelection = [] - } + /// The metadata is read again on every explicit Compare, never reused from the preload. + /// The list on screen can be minutes old, and a table's key can have been dropped since: + /// a stale key still merges the two row streams and still addresses the UPDATE and DELETE + /// it generates, so one reviewed row's statement can reach every row sharing that value. + /// `buildPlans` carries the user's ticks, keys and row exclusions onto the fresh list. + let built = try await buildPlans(context) + guard session.owns(claim) else { throw CancellationError() } + session.adoptDataPlans(built) + var plans = session.dataPlans for index in plans.indices where plans[index].isEnabled && plans[index].isComparable { try Task.checkCancellation() @@ -48,19 +87,22 @@ internal extension CompareRunner { } } + try Task.checkCancellation() + guard session.owns(claim) else { throw CancellationError() } session.dataPlans = plans + session.hasLoadedDataPlans = true session.selectedPlanId = plans.first { $0.isEnabled && $0.isComparable }?.id ?? plans.first?.id session.detailPane = .rows session.invalidateScript() - /// Plans start unchecked so a first Compare cannot stream every row of every table, which - /// means a first run legitimately reads nothing. Recording that as "0 differences" invited - /// the reader to conclude the two databases matched. + /// Plans start unchecked so a Compare cannot stream every row of every table by accident, + /// which means a run with nothing ticked legitimately reads nothing. Recording that as + /// "0 differences" invited the reader to conclude the two databases matched. let comparedAny = plans.contains { $0.isEnabled && $0.isComparable && $0.summary != nil } guard comparedAny else { session.lastAction = .none session.informationalMessage = String( - localized: "No tables were compared. Choose the tables to compare, then press Compare." + localized: "Nothing was compared. Tick the tables to compare, then press Compare." ) return } @@ -113,14 +155,11 @@ internal extension CompareRunner { // MARK: - Plans private func buildPlans(_ context: Context) async throws -> [DataComparePlan] { - let sourceReads = try await metadataService.tableReads( - for: context.source, connection: context.sourceConnection, includeViews: false - ) - try Task.checkCancellation() - let targetReads = try await metadataService.tableReads( - for: context.target, connection: context.targetConnection, includeViews: false + let (sourceReads, targetReads) = try await metadataService.bothSideTableReads( + context: context, includeViews: false, profile: .data ) try Task.checkCancellation() + session.unreadableTableCount = (sourceReads + targetReads).filter { $0.failure != nil }.count /// Keyed on schema and name, not name alone: two schemas of one database can hold the same /// table, and pairing on the bare name took the shared column set from the wrong diff --git a/TablePro/Core/Compare/CompareRunner.swift b/TablePro/Core/Compare/CompareRunner.swift index b23de99ac..c0c6e8b74 100644 --- a/TablePro/Core/Compare/CompareRunner.swift +++ b/TablePro/Core/Compare/CompareRunner.swift @@ -35,37 +35,61 @@ internal struct CompareRunner { session.errorMessage = nil session.informationalMessage = nil + let claim = session.currentClaim session.runTask = Task { [session] in session.activity = .connecting defer { session.activity = .idle } do { let context = try resolveContext() if let refusal = try await capabilityRefusal(context) { + guard session.owns(claim) else { return } session.errorMessage = refusal return } session.activity = .comparing - switch session.mode { + switch claim.mode { case .structure: - try await runStructureCompare(context) + try await runStructureCompare(context, claim: claim) case .data: - try await runDataCompare(context) + try await runDataCompare(context, claim: claim) } + guard session.owns(claim) else { return } session.informationalMessage = session.crossEngineNotice } catch is CancellationError { + guard session.owns(claim) else { return } session.informationalMessage = String(localized: "Comparison cancelled.") } catch { + guard session.owns(claim) else { return } session.errorMessage = error.localizedDescription } } } internal func buildScript() { - guard session.canBuildScript else { return } + guard let task = makeScriptTask() else { return } + session.runTask = task + } + + /// Builds the script and waits, for the caller that needs one before it can show anything. + /// Apply takes this route, so a comparison is one press away from the sheet that reviews it + /// rather than two. + internal func buildScriptIfNeeded() async -> Bool { + guard session.statements.isEmpty else { return true } + guard let task = makeScriptTask() else { return false } + /// Read after `makeScriptTask`, because cancelling the previous run advances the revision. + let claim = session.currentClaim + session.runTask = task + await task.value + return session.owns(claim) && !session.statements.isEmpty + } + + private func makeScriptTask() -> Task? { + guard session.canBuildScript else { return nil } session.cancelRunningWork() session.errorMessage = nil - session.runTask = Task { [session] in + let claim = session.currentClaim + return Task { [session] in session.activity = .comparing defer { session.activity = .idle } do { @@ -77,6 +101,11 @@ internal struct CompareRunner { case .data: built = try await dataStatements(context) } + try Task.checkCancellation() + /// A script describes one setup and one set of choices. Committing it after either + /// moved would arm Apply with statements for a database the window no longer names, + /// or for an object the user has just excluded. + guard session.owns(claim) else { return } guard !built.isEmpty else { session.errorMessage = String(localized: "Nothing is selected to apply.") return @@ -84,15 +113,17 @@ internal struct CompareRunner { session.statements = built session.detailPane = .script } catch is CancellationError { + guard session.owns(claim) else { return } session.informationalMessage = String(localized: "Script generation cancelled.") } catch { + guard session.owns(claim) else { return } session.errorMessage = error.localizedDescription } } } internal func apply() { - guard session.canApply, let target = session.target else { return } + guard session.runRefusalReason == nil, let target = session.target else { return } session.cancelRunningWork() session.errorMessage = nil @@ -181,7 +212,7 @@ internal struct CompareRunner { ) } - private func capabilityRefusal(_ context: Context) async throws -> String? { + internal func capabilityRefusal(_ context: Context) async throws -> String? { if let refusal = try await metadataService.refusalReason( for: context.source, connection: context.sourceConnection, mode: session.mode ) { @@ -194,16 +225,12 @@ internal struct CompareRunner { // MARK: - Structure - private func runStructureCompare(_ context: Context) async throws { + private func runStructureCompare(_ context: Context, claim: CompareSyncSession.RunClaim) async throws { let wantsViews = session.includedKinds.contains(.view) || session.includedKinds.contains(.materializedView) - let sourceReads = try await metadataService.tableReads( - for: context.source, connection: context.sourceConnection, includeViews: wantsViews - ) - try Task.checkCancellation() - let targetReads = try await metadataService.tableReads( - for: context.target, connection: context.targetConnection, includeViews: wantsViews + let (sourceReads, targetReads) = try await metadataService.bothSideTableReads( + context: context, includeViews: wantsViews, profile: .structure ) try Task.checkCancellation() @@ -216,24 +243,32 @@ internal struct CompareRunner { let engine = StructureDiffEngine(options: session.structureOptions) let tableReport = engine.compare(source: sourceSnapshots, target: targetSnapshots) - session.sourceSnapshots = Dictionary( + /// Built locally and committed once. Writing the snapshots here and the report after the + /// next await let a reset in between leave one pair's snapshots under another pair's + /// report, which a later script build then turned into DDL for the wrong target. + let sourceByName = Dictionary( sourceSnapshots.map { ($0.qualifiedName, $0) }, uniquingKeysWith: { first, _ in first } ) - session.targetSnapshots = Dictionary( + let targetByName = Dictionary( targetSnapshots.map { ($0.qualifiedName, $0) }, uniquingKeysWith: { first, _ in first } ) var results = tableReport.results.map { result -> CompareObjectResult in CompareObjectResult.from( result, - sourceDefinition: session.sourceSnapshots[result.id].map(TableDefinitionRenderer.lines) ?? [], - targetDefinition: session.targetSnapshots[result.id].map(TableDefinitionRenderer.lines) ?? [] + sourceDefinition: sourceByName[result.id].map(TableDefinitionRenderer.lines) ?? [], + targetDefinition: targetByName[result.id].map(TableDefinitionRenderer.lines) ?? [] ) } results += unreadableResults(sourceTables, targetTables) results += try await sourceDefinedResults(context, sourceReads: sourceReads, targetReads: targetReads) + try Task.checkCancellation() + guard session.owns(claim) else { throw CancellationError() } + let report = CompareReport(results: results) + session.sourceSnapshots = sourceByName + session.targetSnapshots = targetByName session.report = report session.actions = [:] for result in report.comparable where session.pendingSelection.contains(result.id) { @@ -272,43 +307,45 @@ internal struct CompareRunner { ) async throws -> [CompareObjectResult] { var results: [CompareObjectResult] = [] + /// Each pair reads two independent endpoints, so the two sides run together rather than the + /// second waiting out the first. if session.includedKinds.contains(.view) || session.includedKinds.contains(.materializedView) { let sourceViews = sourceReads.map(\.table).filter { CompareTableKindClassifier.kind(of: $0) != .table } let targetViews = targetReads.map(\.table).filter { CompareTableKindClassifier.kind(of: $0) != .table } - let sourceDefinitions = try await metadataService.viewDefinitions( + async let sourceDefinitions = metadataService.viewDefinitions( for: context.source, connection: context.sourceConnection, views: sourceViews ) - let targetDefinitions = try await metadataService.viewDefinitions( + async let targetDefinitions = metadataService.viewDefinitions( for: context.target, connection: context.targetConnection, views: targetViews ) - results += SourceObjectDiffEngine(options: session.structureOptions) + results += try await SourceObjectDiffEngine(options: session.structureOptions) .compare(source: sourceDefinitions, target: targetDefinitions) } if session.includedKinds.contains(.procedure) || session.includedKinds.contains(.function) { - let sourceRoutines = try await metadataService.routineReads( + async let sourceRoutines = metadataService.routineReads( for: context.source, connection: context.sourceConnection ) - let targetRoutines = try await metadataService.routineReads( + async let targetRoutines = metadataService.routineReads( for: context.target, connection: context.targetConnection ) - results += SourceObjectDiffEngine(options: session.structureOptions) + results += try await SourceObjectDiffEngine(options: session.structureOptions) .compare(source: sourceRoutines, target: targetRoutines) .filter { session.includedKinds.contains($0.identity.kind) } } if session.includedKinds.contains(.trigger) { - let sourceTriggers = try await metadataService.triggerReads( + async let sourceTriggers = metadataService.triggerReads( for: context.source, connection: context.sourceConnection, tables: sourceReads.map(\.table.name) ) - let targetTriggers = try await metadataService.triggerReads( + async let targetTriggers = metadataService.triggerReads( for: context.target, connection: context.targetConnection, tables: targetReads.map(\.table.name) ) - results += SourceObjectDiffEngine(options: session.structureOptions) + results += try await SourceObjectDiffEngine(options: session.structureOptions) .compare(source: sourceTriggers, target: targetTriggers) } diff --git a/TablePro/Core/Compare/CompareSyncProfileStorage.swift b/TablePro/Core/Compare/CompareSyncProfileStorage.swift index bd8b72580..376d871ca 100644 --- a/TablePro/Core/Compare/CompareSyncProfileStorage.swift +++ b/TablePro/Core/Compare/CompareSyncProfileStorage.swift @@ -90,6 +90,7 @@ internal final class CompareSyncProfileStorage { private static let logger = Logger(subsystem: "com.TablePro", category: "CompareSyncProfileStorage") private static let defaultsKey = "compareSyncProfiles" + private static let lastSetupKey = "compareSyncLastSetup" private let defaults: UserDefaults @@ -97,6 +98,32 @@ internal final class CompareSyncProfileStorage { self.defaults = defaults } + // MARK: - Last setup + + /// The pair, the mode and the options the window last held, so reopening it lands on the same + /// comparison rather than on two empty pickers. It carries no included objects: what to change + /// is a decision about one comparison's results, and re-arming it against a report that has not + /// run yet would put a stale choice behind an Apply button. + internal func lastSetup() -> CompareSyncProfile? { + guard let data = defaults.data(forKey: Self.lastSetupKey) else { return nil } + do { + return try JSONDecoder().decode(CompareSyncProfile.self, from: data) + } catch { + Self.logger.error("Failed to decode last setup: \(error.localizedDescription, privacy: .public)") + return nil + } + } + + internal func rememberSetup(_ profile: CompareSyncProfile) { + do { + defaults.set(try JSONEncoder().encode(profile), forKey: Self.lastSetupKey) + } catch { + Self.logger.error("Failed to persist last setup: \(error.localizedDescription, privacy: .public)") + } + } + + // MARK: - Saved comparisons + internal func allProfiles() -> [CompareSyncProfile] { guard let data = defaults.data(forKey: Self.defaultsKey) else { return [] } do { diff --git a/TablePro/Core/Compare/CompareSyncSession+Editing.swift b/TablePro/Core/Compare/CompareSyncSession+Editing.swift index 22f92a7d1..fb07d2a02 100644 --- a/TablePro/Core/Compare/CompareSyncSession+Editing.swift +++ b/TablePro/Core/Compare/CompareSyncSession+Editing.swift @@ -102,37 +102,127 @@ internal extension CompareSyncSession { // MARK: - Saved comparisons + /// Every saved comparison, not only the ones matching the pair on screen. + /// + /// Filtering by the current source and target is what made the feature circular: a setup only + /// listed once its own two endpoints had already been picked by hand, which is the work loading + /// it exists to save. The scopes a profile stores are what it sets, so it is offered from a + /// window that has chosen nothing. var savedProfiles: [CompareSyncProfile] { - guard let source, let target else { return [] } - return CompareSyncProfileStorage.shared.profiles(source: source.scope, target: target.scope, mode: mode) + profileStorage.allProfiles() + .sorted { $0.name.localizedStandardCompare($1.name) == .orderedAscending } } func saveProfile(named name: String) { guard let source, let target, !name.trimmingCharacters(in: .whitespaces).isEmpty else { return } - let profile = CompareSyncProfile( - name: name, - source: source.scope, - target: target.scope, - mode: mode, - includedKinds: includedKinds, - structureOptions: structureOptions, - dataOptions: dataOptions, - selectedObjects: selectedObjectIdentifiers + profileStorage.save( + currentSetup(named: name, source: source, target: target, includingSelection: true) ) - CompareSyncProfileStorage.shared.save(profile) } - func apply(_ profile: CompareSyncProfile) { + /// Adopting a profile sets both endpoints, which is the whole point of having saved one. It + /// used to restore the mode and the options and leave the pickers untouched, so a load left the + /// window pointed at whatever pair happened to be open. + /// + /// A scope whose connection has since been deleted is reported rather than silently dropped: a + /// comparison that quietly loses its target would otherwise offer Compare against the endpoint + /// still in the other picker. + @discardableResult + func apply(_ profile: CompareSyncProfile) -> Bool { + /// A run in flight is writing to the target it captured, so swapping both endpoints under + /// it would leave the window reporting one pair while the executor finishes against + /// another. `canLoadProfile` is what the menu and the Load buttons validate against too. + guard canLoadProfile else { return false } + + let connections = connectionsProvider() + let resolvedSource = Self.endpoint(for: profile.source, in: connections) + let resolvedTarget = Self.endpoint(for: profile.target, in: connections) + mode = profile.mode includedKinds = profile.includedKinds.isEmpty ? [.table] : profile.includedKinds structureOptions = profile.structureOptions dataOptions = profile.dataOptions + source = resolvedSource + target = resolvedTarget resetComparison() pendingSelection = Set(profile.selectedObjects) + + /// Written to the setup message rather than to `errorMessage`, which the option changes + /// this load just made will clear through their own `onChange` reset. + setupErrorMessage = resolvedSource == nil || resolvedTarget == nil + ? String(format: String(localized: "%@ names a connection that no longer exists."), profile.name) + : nil + return true + } + + var canLoadProfile: Bool { + !isBusy } func deleteProfile(_ profile: CompareSyncProfile) { - CompareSyncProfileStorage.shared.delete(profile) + profileStorage.delete(profile) + } + + // MARK: - Last setup + + /// Reopening the window lands on the comparison it last held. Nothing is compared and nothing + /// is written by restoring it: it is the two pickers, the mode and the options, which is the + /// part of the work that was being repeated by hand every time. + func restore(_ setup: CompareSyncProfile, keepingSource pinnedSource: DatabaseEndpoint?) { + let connections = connectionsProvider() + mode = setup.mode + includedKinds = setup.includedKinds.isEmpty ? [.table] : setup.includedKinds + structureOptions = setup.structureOptions + dataOptions = setup.dataOptions + + guard let pinnedSource else { + source = Self.endpoint(for: setup.source, in: connections) + target = Self.endpoint(for: setup.target, in: connections) + return + } + /// The window was opened against one connection, so that connection is the source. The + /// remembered target only comes back when it was remembered against this same source, + /// because a target is the side that gets written to and inheriting one from an unrelated + /// comparison would arm the wrong database. + source = pinnedSource + /// The whole scope, not the connection alone. One connection reaches many databases and + /// many schemas, and `DatabaseEndpoint.id` already treats those as different endpoints, so + /// a connection match would hand database B the writable target remembered for database A. + guard setup.source == pinnedSource.scope else { return } + target = Self.endpoint(for: setup.target, in: connections) + } + + func rememberSetup() { + guard let source, let target else { return } + profileStorage.rememberSetup( + currentSetup(named: "", source: source, target: target, includingSelection: false) + ) + } + + private func currentSetup( + named name: String, + source: DatabaseEndpoint, + target: DatabaseEndpoint, + includingSelection: Bool + ) -> CompareSyncProfile { + CompareSyncProfile( + name: name, + source: source.scope, + target: target.scope, + mode: mode, + includedKinds: includedKinds, + structureOptions: structureOptions, + dataOptions: dataOptions, + selectedObjects: includingSelection ? selectedObjectIdentifiers : [] + ) + } + + private static func endpoint( + for scope: DatabaseScope, + in connections: [DatabaseConnection] + ) -> DatabaseEndpoint? { + guard let connection = connections.first(where: { $0.id == scope.connectionId }) else { return nil } + return DatabaseEndpoint.from(connection: connection, database: scope.database, schema: scope.schema) } private var selectedObjectIdentifiers: [String] { diff --git a/TablePro/Core/Compare/CompareSyncSession.swift b/TablePro/Core/Compare/CompareSyncSession.swift index 73322b267..45ece35ca 100644 --- a/TablePro/Core/Compare/CompareSyncSession.swift +++ b/TablePro/Core/Compare/CompareSyncSession.swift @@ -61,6 +61,16 @@ internal final class CompareSyncSession { internal var report: CompareReport? internal var dataPlans: [DataComparePlan] = [] + + /// True once the table list has been read for this pair, which an empty `dataPlans` cannot say + /// on its own. Without it "not read yet" and "these two share no table" are the same state, and + /// the pane claims the second whenever the first is true. + internal var hasLoadedDataPlans = false + + /// Tables that were listed on one side and whose metadata could not be read. An empty plan list + /// with unreadable tables behind it is not "these two share no table", and saying so sent the + /// reader looking for a naming difference that was not there. + internal var unreadableTableCount = 0 internal var actions: [String: TableSyncAction] = [:] internal var statements: [SyncStatement] = [] internal var runResult: CompareSyncRunResult? @@ -90,7 +100,45 @@ internal final class CompareSyncSession { internal var runTask: Task? internal var pendingSelection: Set = [] - internal init() {} + /// Which setup the answers on screen belong to. + /// + /// `Task.cancel()` is cooperative, so work that has already reached a driver finishes whatever + /// the window does next, and the setup it was started for can be gone by the time it has an + /// answer. Publishing that answer puts one pair's plans, snapshots or statements behind another + /// pair's Compare and Apply, which is the same trap `ConnectionAttemptRegistry` exists for on + /// the connection side. Every async publisher captures this and drops its result if it moved. + private(set) var setupGeneration = 0 + + /// Which set of choices the script on screen was built from. + /// + /// `setupGeneration` cannot answer this. Including an object, changing a key column, excluding + /// a row or flipping a write policy all invalidate the script without changing the setup, and + /// those controls stay live while a build is in flight. A build that finished after one of them + /// would republish statements for an object the user had just excluded, and Apply would then + /// open on them, because an ordinary INSERT or ALTER is not a hazard `runRefusalReason` catches. + private(set) var scriptRevision = 0 + + /// A setup problem, which outlives the comparison it interrupted. `errorMessage` is cleared by + /// the next reset, and a reset is exactly what changing the setup does, so a message about the + /// setup itself cannot live there: loading a profile whose connection is gone reported the + /// failure and had it wiped by the option change the same load caused. + internal var setupErrorMessage: String? + + /// The two things the setup needs from outside the session: where a saved comparison lives, and + /// which connections a stored scope can resolve against. Injected rather than reached for, so + /// the restore rules can be exercised without writing to the user's own defaults. + @ObservationIgnored internal let profileStorage: CompareSyncProfileStorage + @ObservationIgnored internal let connectionsProvider: @MainActor () -> [DatabaseConnection] + + internal init( + profileStorage: CompareSyncProfileStorage = .shared, + connectionsProvider: @escaping @MainActor () -> [DatabaseConnection] = { + ConnectionStorage.shared.loadConnections() + } + ) { + self.profileStorage = profileStorage + self.connectionsProvider = connectionsProvider + } // MARK: - Direction @@ -113,6 +161,12 @@ internal final class CompareSyncSession { resetComparison() } + /// The setup message says an endpoint could not be resolved, so choosing one clears it. + internal func clearSetupErrorIfResolved() { + guard source != nil, target != nil else { return } + setupErrorMessage = nil + } + internal var canCompare: Bool { compareDisabledReason == nil } @@ -255,6 +309,14 @@ internal final class CompareSyncSession { return nil } + /// Apply is available whenever there is a comparison to apply, and it builds its own script + /// when none has been built. Requiring Generate Script first made every sync a three-press + /// sequence for a script the Apply sheet shows in full anyway. + /// + /// An unallowed hazard is deliberately not a reason any more. The allowance for one lives + /// inside the Apply sheet, so withholding the sheet until every hazard was allowed put the + /// control behind the door it was locking. The sheet's own Apply button still refuses to run + /// while one is outstanding, which is where the refusal belongs. internal var applyDisabledReason: String? { if isBusy { return String(localized: "A run is already in progress.") } if isStaleAfterApply { @@ -263,7 +325,16 @@ internal final class CompareSyncSession { guard target?.canBeWrittenTo == true else { return target?.ineligibleAsTargetReason ?? String(localized: "Choose a target to write to.") } - guard !statements.isEmpty else { return String(localized: "Generate the script first.") } + guard statements.isEmpty else { return nil } + return scriptDisabledReason + } + + /// What the Apply sheet's own button answers to, and what the run itself refuses on. A + /// statement carrying an unallowed hazard stops the run rather than being dropped from it: the + /// count the user is about to run has to be the count they saw. + internal var runRefusalReason: String? { + if let reason = applyDisabledReason { return reason } + if statements.isEmpty { return String(localized: "There is no script to run.") } guard unacknowledgedHazardCount == 0 else { guard unacknowledgedHazardCount != 1 else { return String(localized: "1 statement would destroy data and is not allowed yet.") @@ -273,6 +344,7 @@ internal final class CompareSyncSession { unacknowledgedHazardCount ) } + guard runnableStatementCount > 0 else { return String(localized: "No statement is allowed to run.") } return nil } @@ -357,6 +429,8 @@ internal final class CompareSyncSession { sourceSnapshots = [:] targetSnapshots = [:] dataPlans = [] + hasLoadedDataPlans = false + unreadableTableCount = 0 actions = [:] selectedObjectId = nil selectedPlanId = nil @@ -366,6 +440,56 @@ internal final class CompareSyncSession { lastAction = .none isStaleAfterApply = false invalidateScript() + setupGeneration &+= 1 + /// Every path that resets a comparison is a path that changed the setup: an endpoint, the + /// mode, or an option. So this is also where the setup is written down, and reopening the + /// window lands on the same pair instead of on two empty pickers. + rememberSetup() + } + + /// True while the answer in hand still belongs to the setup on screen. + internal func isCurrent(_ generation: Int) -> Bool { + generation == setupGeneration + } + + /// What one run was started for, captured before it can suspend. Every helper takes the + /// caller's claim rather than reading the session again: re-reading inside a callee adopts + /// whatever the setup has become, which is exactly the ownership the fence is meant to check. + internal struct RunClaim: Sendable { + internal let setup: Int + internal let script: Int + internal let mode: CompareSyncMode + } + + internal var currentClaim: RunClaim { + RunClaim(setup: setupGeneration, script: scriptRevision, mode: mode) + } + + internal func owns(_ claim: RunClaim) -> Bool { + isCurrent(setup: claim.setup, script: claim.script) + } + + /// The one place a table list is published, so the tables a saved comparison asked for are + /// applied where the list arrives rather than at the next Compare. Left in `pendingSelection`, + /// they were reapplied later over whatever the user had ticked in the meantime. + internal func adoptDataPlans(_ plans: [DataComparePlan]) { + var adopted = plans + if !pendingSelection.isEmpty { + for index in adopted.indices { + adopted[index].isEnabled = pendingSelection.contains(adopted[index].id) + } + pendingSelection = [] + } + let previousSelection = selectedPlanId + dataPlans = adopted + hasLoadedDataPlans = true + /// A rebuild keeps whatever row the user was reading, when that table is still there. Only + /// a list that no longer holds it falls back to the first table taking part. + if let previousSelection, adopted.contains(where: { $0.id == previousSelection }) { + selectedPlanId = previousSelection + return + } + selectedPlanId = adopted.first { $0.isEnabled && $0.isComparable }?.id ?? adopted.first?.id } /// After a run the report describes a target that has since changed, so it is stale rather than @@ -383,11 +507,21 @@ internal final class CompareSyncSession { internal func invalidateScript() { statements = [] runResult = nil + scriptRevision &+= 1 + } + + /// True while both the setup and the choices a piece of work was started for still stand. + internal func isCurrent(setup: Int, script: Int) -> Bool { + setup == setupGeneration && script == scriptRevision } + /// Cancelling advances the script revision as well as asking the task to stop, because + /// `Task.cancel()` is cooperative: a build already inside a driver call finishes and would + /// otherwise publish over a comparison the user has stopped. internal func cancelRunningWork() { progress?.cancel() runTask?.cancel() + scriptRevision &+= 1 } internal var isBusy: Bool { diff --git a/TablePro/Core/Database/DatabaseDriver.swift b/TablePro/Core/Database/DatabaseDriver.swift index 567feffeb..48e9d87af 100644 --- a/TablePro/Core/Database/DatabaseDriver.swift +++ b/TablePro/Core/Database/DatabaseDriver.swift @@ -127,6 +127,28 @@ protocol DatabaseDriver: AnyObject, Sendable { /// per table, which is too expensive to run ahead of the user. var providesBulkForeignKeyFetch: Bool { get } + /// Whether `fetchAllColumns` is a single query that reports what per-table `fetchColumns` + /// reports. Both halves matter: a bulk query missing generated columns is not a substitute. + var providesBulkColumnFetch: Bool { get } + + /// Fetch indexes for every table in the current schema in bulk. + /// Default implementation falls back to per-table fetchIndexes. + func fetchAllIndexes(schema: String?) async throws -> [String: [IndexInfo]] + + /// Whether `fetchAllIndexes` is a single query. + var providesBulkIndexFetch: Bool { get } + + /// Fetch table metadata for every table in the current schema in bulk. + /// Default implementation falls back to per-table fetchTableMetadata. + func fetchAllTableMetadata(schema: String?) async throws -> [String: TableMetadata] + + /// Whether `fetchAllTableMetadata` is a single query. + var providesBulkTableMetadataFetch: Bool { get } + + /// Whether `fetchAllTriggers` lists a whole schema's triggers. Its default answers with nothing + /// rather than looping, so a caller has to know before it decides to ask per table. + var providesBulkTriggerFetch: Bool { get } + /// Fetch foreign keys for a specific set of tables. /// Default implementation calls fetchAllForeignKeys and filters, or falls back to per-table. func fetchForeignKeys(forTables tableNames: [String]) async throws -> [String: [ForeignKeyInfo]] @@ -414,6 +436,30 @@ extension DatabaseDriver { } var providesBulkForeignKeyFetch: Bool { false } + var providesBulkColumnFetch: Bool { false } + var providesBulkIndexFetch: Bool { false } + var providesBulkTableMetadataFetch: Bool { false } + var providesBulkTriggerFetch: Bool { false } + + func fetchAllIndexes(schema: String?) async throws -> [String: [IndexInfo]] { + let tables = try await fetchTables() + var result: [String: [IndexInfo]] = [:] + for table in tables { + let indexes = try await fetchIndexes(table: table.name) + if !indexes.isEmpty { result[table.name] = indexes } + } + return result + } + + func fetchAllTableMetadata(schema: String?) async throws -> [String: TableMetadata] { + let tables = try await fetchTables() + var result: [String: TableMetadata] = [:] + for table in tables { + guard let metadata = try? await fetchTableMetadata(tableName: table.name) else { continue } + result[table.name] = metadata + } + return result + } func fetchAllForeignKeys() async throws -> [String: [ForeignKeyInfo]] { let allTables = try await fetchTables() diff --git a/TablePro/Core/Menu/DatabaseMenuBuilder.swift b/TablePro/Core/Menu/DatabaseMenuBuilder.swift index c79fcdd92..cd58f9a35 100644 --- a/TablePro/Core/Menu/DatabaseMenuBuilder.swift +++ b/TablePro/Core/Menu/DatabaseMenuBuilder.swift @@ -118,6 +118,11 @@ enum DatabaseMenuBuilder { action: #selector(AppDelegate.compareAndSyncDatabases(_:)) ), MenuItemFactory.separator, + MenuItemFactory.item( + String(localized: "Save Comparison…"), + action: #selector(CompareSyncWindowController.saveComparison(_:)) + ), + MenuItemFactory.separator, MenuItemFactory.item( String(localized: "Compare Now"), action: #selector(CompareSyncWindowController.runComparison(_:)) diff --git a/TablePro/Core/Plugins/PluginDriverAdapter.swift b/TablePro/Core/Plugins/PluginDriverAdapter.swift index 2cdcebf2d..60ab0feb9 100644 --- a/TablePro/Core/Plugins/PluginDriverAdapter.swift +++ b/TablePro/Core/Plugins/PluginDriverAdapter.swift @@ -278,17 +278,35 @@ final class PluginDriverAdapter: DatabaseDriver, SchemaSwitchable, DatabaseRepor func fetchIndexes(table: String) async throws -> [IndexInfo] { let pluginIndexes = try await pluginDriver.fetchIndexes(table: table, schema: pluginDriver.currentSchema) - return pluginIndexes.map { idx in - IndexInfo( - name: idx.name, - columns: idx.columns, - isUnique: idx.isUnique, - isPrimary: idx.isPrimary, - type: idx.type, - columnPrefixes: idx.columnPrefixes, - whereClause: idx.whereClause - ) - } + return pluginIndexes.map(Self.mapPluginIndex) + } + + nonisolated private static func mapPluginIndex(_ index: PluginIndexInfo) -> IndexInfo { + IndexInfo( + name: index.name, + columns: index.columns, + isUnique: index.isUnique, + isPrimary: index.isPrimary, + type: index.type, + columnPrefixes: index.columnPrefixes, + whereClause: index.whereClause + ) + } + + nonisolated private static func mapPluginTableMetadata(_ metadata: PluginTableMetadata) -> TableMetadata { + TableMetadata( + tableName: metadata.tableName, + dataSize: metadata.dataSize, + indexSize: metadata.indexSize, + totalSize: metadata.totalSize, + avgRowLength: metadata.avgRowLength, + rowCount: metadata.rowCount, + comment: metadata.comment, + engine: metadata.engine, + collation: metadata.collation, + createTime: metadata.createTime, + updateTime: metadata.updateTime + ) } func fetchForeignKeys(table: String) async throws -> [ForeignKeyInfo] { @@ -400,19 +418,7 @@ final class PluginDriverAdapter: DatabaseDriver, SchemaSwitchable, DatabaseRepor table: tableName, schema: pluginDriver.currentSchema ) - return TableMetadata( - tableName: pluginMeta.tableName, - dataSize: pluginMeta.dataSize, - indexSize: pluginMeta.indexSize, - totalSize: pluginMeta.totalSize, - avgRowLength: pluginMeta.avgRowLength, - rowCount: pluginMeta.rowCount, - comment: pluginMeta.comment, - engine: pluginMeta.engine, - collation: pluginMeta.collation, - createTime: pluginMeta.createTime, - updateTime: pluginMeta.updateTime - ) + return Self.mapPluginTableMetadata(pluginMeta) } func fetchDatabases() async throws -> [String] { @@ -510,6 +516,22 @@ final class PluginDriverAdapter: DatabaseDriver, SchemaSwitchable, DatabaseRepor } var providesBulkForeignKeyFetch: Bool { pluginDriver.providesBulkForeignKeyFetch } + var providesBulkColumnFetch: Bool { pluginDriver.providesBulkColumnFetch } + var providesBulkIndexFetch: Bool { pluginDriver.providesBulkIndexFetch } + var providesBulkTableMetadataFetch: Bool { pluginDriver.providesBulkTableMetadataFetch } + var providesBulkTriggerFetch: Bool { pluginDriver.providesBulkTriggerFetch } + + func fetchAllIndexes(schema: String?) async throws -> [String: [IndexInfo]] { + let pluginResult = try await pluginDriver.fetchAllIndexes(schema: schema ?? pluginDriver.currentSchema) + return pluginResult.mapValues { $0.map(Self.mapPluginIndex) } + } + + func fetchAllTableMetadata(schema: String?) async throws -> [String: TableMetadata] { + let pluginResult = try await pluginDriver.fetchAllTableMetadata( + schema: schema ?? pluginDriver.currentSchema + ) + return pluginResult.mapValues(Self.mapPluginTableMetadata) + } func fetchAllForeignKeys() async throws -> [String: [ForeignKeyInfo]] { let pluginResult = try await pluginDriver.fetchAllForeignKeys(schema: pluginDriver.currentSchema) diff --git a/TablePro/Core/Plugins/PluginManager.swift b/TablePro/Core/Plugins/PluginManager.swift index af6fc58da..431223a05 100644 --- a/TablePro/Core/Plugins/PluginManager.swift +++ b/TablePro/Core/Plugins/PluginManager.swift @@ -14,7 +14,18 @@ import TableProPluginKit @MainActor @Observable final class PluginManager { static let shared = PluginManager() - nonisolated static let currentPluginKitVersion = 19 + /// Raised to 20 for the whole-schema index and table metadata requirements. + /// + /// They carry default implementations, so an already-built v19 plugin keeps loading here. The + /// break is the other way round, and it is not what Library Evolution covers: a plugin compiled + /// against these requirements emits undefined references to their method descriptors, their + /// default-implementation symbols and their async function pointers, none of which exist in a + /// 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 + + /// Still 19, so every plugin already published for the previous release keeps loading. nonisolated static let minimumCompatiblePluginKitVersion = 19 nonisolated static let currentInspectorKitVersion = 1 private static let disabledPluginsKey = "com.TablePro.disabledPlugins" diff --git a/TablePro/Views/Compare/CompareApplySheetView.swift b/TablePro/Views/Compare/CompareApplySheetView.swift index e278e5c18..7414cabe8 100644 --- a/TablePro/Views/Compare/CompareApplySheetView.swift +++ b/TablePro/Views/Compare/CompareApplySheetView.swift @@ -180,17 +180,17 @@ internal struct CompareApplySheetView: View { return order.map { PlannedObject(name: $0, count: counts[$0] ?? 0) } } + /// What a held-back statement means, said the way the button behaves. This used to read as + /// though the run went ahead without it, while Apply was in fact refusing to run at all until + /// every one of them was allowed. private var heldBackText: String { let count = session.unacknowledgedHazardCount guard count != 1 else { - return String( - format: String(localized: "1 statement stays out of this run and %@ keeps what it has for it."), - targetName - ) + return String(localized: "1 statement would destroy data. Allow it or exclude it before applying.") } return String( - format: String(localized: "%1$d statements stay out of this run and %2$@ keeps what it has for them."), - count, targetName + format: String(localized: "%d statements would destroy data. Allow them or exclude them before applying."), + count ) } @@ -421,7 +421,7 @@ internal struct CompareApplySheetView: View { } private var canApply: Bool { - session.canApply && session.unacknowledgedHazardCount == 0 && session.runnableStatementCount > 0 + session.runRefusalReason == nil } private func countBadge(_ label: String, _ count: Int, symbol: String, tint: Color) -> some View { diff --git a/TablePro/Views/Compare/CompareDataPlansView.swift b/TablePro/Views/Compare/CompareDataPlansView.swift index fdcc29bc8..98da90f06 100644 --- a/TablePro/Views/Compare/CompareDataPlansView.swift +++ b/TablePro/Views/Compare/CompareDataPlansView.swift @@ -293,24 +293,45 @@ internal struct CompareDataPlansView: View { // MARK: - Empty state + /// The table list is metadata, so it loads as soon as there is a pair to load it for rather + /// than costing a Compare of its own. This state is what is left: no pair yet, the list on its + /// way, or a pair with nothing in common. private var emptyState: some View { ContentUnavailableView { - Label("No Tables Yet", systemImage: "tablecells") + Label(emptyTitle, systemImage: "tablecells") } description: { Text(emptyDescription) } actions: { - Button("Compare", action: onCompare) - .disabled(!session.canCompare) - .accessibilityIdentifier("compare.plans.compare") + if session.compareDisabledReason == nil, !session.isBusy { + Button("Reload Tables", action: onCompare) + .accessibilityIdentifier("compare.plans.compare") + } } } + private var emptyTitle: String { + guard session.compareDisabledReason == nil else { return String(localized: "No Tables Yet") } + if session.isBusy { return String(localized: "Reading Tables") } + if session.unreadableTableCount > 0 { return String(localized: "Tables Could Not Be Read") } + return session.hasLoadedDataPlans + ? String(localized: "No Tables in Common") + : String(localized: "No Tables Yet") + } + /// Why Compare is unavailable beats a generic invitation to press it, which is what the HIG asks - /// for when a command cannot be carried out. + /// for when a command cannot be carried out. "Nothing in common" is claimed only once the list + /// has actually been read, because an empty list before that says nothing about either side. private var emptyDescription: String { - session.compareDisabledReason - ?? String( - localized: "Compare lists the tables both sides share. Choose the tables to compare, then press Compare." + if let reason = session.compareDisabledReason { return reason } + if session.isBusy { return String(localized: "Listing the tables both sides share.") } + if session.unreadableTableCount > 0 { + return String( + format: String(localized: "%d could not be read, so no pair could be made."), + session.unreadableTableCount ) + } + return session.hasLoadedDataPlans + ? String(localized: "The source and the target have no table of the same name.") + : String(localized: "Reload lists the tables both sides share.") } } diff --git a/TablePro/Views/Compare/CompareOptionsView.swift b/TablePro/Views/Compare/CompareOptionsView.swift index 6c105028a..1406effa7 100644 --- a/TablePro/Views/Compare/CompareOptionsView.swift +++ b/TablePro/Views/Compare/CompareOptionsView.swift @@ -57,6 +57,10 @@ internal struct CompareOptionsView: View { } else { session.invalidateScript() } + /// These edits deliberately do not reset the comparison, so they never reach the one place + /// the setup is written down. Without this, changing a tolerance or a write policy and + /// closing the window brought the previous values back. + session.rememberSetup() } // MARK: - Objects @@ -161,10 +165,13 @@ internal struct CompareOptionsView: View { // MARK: - Saved comparisons + /// Every saved comparison, not only the ones matching the pair on screen. The list used to be + /// filtered by the current source and target, so a setup appeared only after both of its + /// endpoints had been chosen by hand, which is the work loading it exists to save. private var savedComparisonsSection: some View { Section { if savedProfiles.isEmpty { - Text("No saved setups for this source and target.") + Text("Nothing saved yet.") .foregroundStyle(.secondary) } else { ForEach(savedProfiles) { profile in @@ -184,18 +191,28 @@ internal struct CompareOptionsView: View { } } header: { Text("Saved Comparisons") + } footer: { + Text("Loading one sets the source, the target, the mode and these options. The Comparisons button in the toolbar lists them too.") } } private func profileRow(_ profile: CompareSyncProfile) -> some View { HStack(spacing: 8) { - Text(profile.name) - .lineLimit(1) - .truncationMode(.middle) + VStack(alignment: .leading, spacing: 1) { + Text(profile.name) + .lineLimit(1) + .truncationMode(.middle) + Text(scope(of: profile)) + .font(.caption) + .foregroundStyle(.secondary) + .lineLimit(1) + .truncationMode(.middle) + } Spacer(minLength: 0) Button("Load") { session.apply(profile) } + .disabled(!session.canLoadProfile) .accessibilityIdentifier("compare.options.loadProfile.\(profile.id.uuidString)") Button("Delete", role: .destructive) { session.deleteProfile(profile) @@ -205,8 +222,17 @@ internal struct CompareOptionsView: View { } } + /// The pair a saved comparison names, so a list of several can be told apart without loading + /// one to find out which it was. + private func scope(of profile: CompareSyncProfile) -> String { + String( + format: String(localized: "%1$@ → %2$@, %3$@"), + profile.source.database, profile.target.database, profile.mode.displayName + ) + } + private var canSaveProfile: Bool { - guard session.source != nil, session.target != nil else { return false } + guard !session.isBusy, session.source != nil, session.target != nil else { return false } return !newProfileName.trimmingCharacters(in: .whitespaces).isEmpty } } diff --git a/TablePro/Views/Compare/CompareProgressView.swift b/TablePro/Views/Compare/CompareProgressView.swift index 3e2ea1882..7bca5a05c 100644 --- a/TablePro/Views/Compare/CompareProgressView.swift +++ b/TablePro/Views/Compare/CompareProgressView.swift @@ -93,8 +93,21 @@ internal struct CompareMessageBanner: View { @Environment(\.accessibilityDifferentiateWithoutColor) private var differentiateWithoutColor internal var body: some View { - if session.errorMessage != nil || session.informationalMessage != nil { + if session.errorMessage != nil || session.informationalMessage != nil + || session.setupErrorMessage != nil { VStack(alignment: .leading, spacing: 6) { + /// A problem with the setup rather than with a comparison, so it outlives the reset + /// that changing the setup performs and is dismissed on its own. + if let message = session.setupErrorMessage { + messageRow( + message, + systemImage: "exclamationmark.triangle.fill", + tint: CompareStatusStyle.error, + identifier: "compare.message.dismissSetupError" + ) { + session.setupErrorMessage = nil + } + } if let message = session.errorMessage { messageRow( message, diff --git a/TablePro/Views/Compare/CompareResultsView.swift b/TablePro/Views/Compare/CompareResultsView.swift index 75a22222d..0a633cce2 100644 --- a/TablePro/Views/Compare/CompareResultsView.swift +++ b/TablePro/Views/Compare/CompareResultsView.swift @@ -51,12 +51,34 @@ internal struct CompareResultsView: View { .toggleStyle(.checkbox) .accessibilityIdentifier("compare.results.showIdentical") Spacer(minLength: 0) + /// The same control data mode carries. Structure mode had per-row checkboxes and a + /// right-click menu that needs a selection first, so including a whole comparison was + /// one click per object. + Menu(String(localized: "Select")) { + Button("All") { + session.setIncluded(true, forIds: includableIds) + } + Button("None") { + session.setIncluded(false, forIds: includableIds) + } + } + .fixedSize() + .disabled(includableIds.isEmpty) + .accessibilityIdentifier("compare.results.select") } .padding(.horizontal, 12) .padding(.vertical, 6) .background(.bar) } + /// What the results pane is showing, so Select follows the search field and the identical + /// toggle rather than reaching past them into the whole report. + private var includableIds: [String] { + session.visibleResults + .filter { $0.isComparable && $0.suggestedAction != .skip } + .map(\.id) + } + // MARK: - Table private func resultsTable( diff --git a/TablePro/Views/Compare/CompareSyncWindowController.swift b/TablePro/Views/Compare/CompareSyncWindowController.swift index 6d3e41c6f..b3884990e 100644 --- a/TablePro/Views/Compare/CompareSyncWindowController.swift +++ b/TablePro/Views/Compare/CompareSyncWindowController.swift @@ -22,6 +22,7 @@ import AppKit import SwiftUI internal extension NSToolbarItem.Identifier { + static let compareSaved = NSToolbarItem.Identifier("com.TablePro.compare.saved") static let compareSource = NSToolbarItem.Identifier("com.TablePro.compare.source") static let compareSwap = NSToolbarItem.Identifier("com.TablePro.compare.swap") static let compareTarget = NSToolbarItem.Identifier("com.TablePro.compare.target") @@ -36,7 +37,7 @@ internal extension NSToolbarItem.Identifier { @MainActor internal final class CompareSyncWindowController: NSWindowController, - NSWindowDelegate, NSToolbarDelegate, NSUserInterfaceValidations { + NSWindowDelegate, NSToolbarDelegate, NSMenuDelegate, NSUserInterfaceValidations { private static var controllers: [UUID?: CompareSyncWindowController] = [:] private let session = CompareSyncSession() @@ -49,6 +50,7 @@ internal final class CompareSyncWindowController: NSWindowController, } private weak var modeControl: NSSegmentedControl? private weak var searchToolbarItem: NSSearchToolbarItem? + private var renderedEndpoints: String? internal static func present(prefillSource connectionId: UUID?) { let controller = controllers[connectionId] ?? CompareSyncWindowController(prefillSource: connectionId) @@ -78,6 +80,10 @@ internal final class CompareSyncWindowController: NSWindowController, window.applyAutosaveName(WindowIdentifier.compareSync) installToolbar(on: window) installStatusStrip(on: window) + /// A restored data comparison has a pair but no table list, and the list is what the mode + /// needs before anything can be ticked. Routed through the one chrome refresh so the + /// subtitle, the picker titles and the load all follow the same rule. + refreshEndpointChrome() } @available(*, unavailable) @@ -100,11 +106,21 @@ internal final class CompareSyncWindowController: NSWindowController, return hosting } + /// The window opens where it was left, which is what makes running the same comparison again + /// one press of Compare rather than a walk back through both pickers, the mode and the options. + /// A connection the window was opened *from* still wins the source, because that is what the + /// user just pointed at. private func applyPrefill(_ connectionId: UUID?) { - guard let connectionId else { return } - let connections = ConnectionStorage.shared.loadConnections() - guard let match = connections.first(where: { $0.id == connectionId }) else { return } - session.source = DatabaseEndpoint.from(connection: match) + let prefilled = connectionId.flatMap { id -> DatabaseEndpoint? in + let connections = ConnectionStorage.shared.loadConnections() + guard let match = connections.first(where: { $0.id == id }) else { return nil } + return DatabaseEndpoint.from(connection: match) + } + guard let setup = CompareSyncProfileStorage.shared.lastSetup() else { + session.source = prefilled + return + } + session.restore(setup, keepingSource: prefilled) } /// The strip belongs to the window frame, not to the content: it reports what the window is @@ -134,8 +150,22 @@ internal final class CompareSyncWindowController: NSWindowController, toolbar.autosavesConfiguration = true window.toolbar = toolbar window.toolbarStyle = .unified + insertSavedComparisonsItemOnce(into: toolbar) } + /// `autosavesConfiguration` restores the identifiers a configuration was saved with, so an item + /// added to the defaults afterwards never appears for anyone who already has one. Inserted once, + /// recorded once, and never again, so a user who then removes it keeps it removed. + private func insertSavedComparisonsItemOnce(into toolbar: NSToolbar) { + let defaults = AppStorageEnvironment.shared.defaults + guard !defaults.bool(forKey: Self.savedComparisonsInsertedKey) else { return } + defaults.set(true, forKey: Self.savedComparisonsInsertedKey) + guard !toolbar.items.contains(where: { $0.itemIdentifier == .compareSaved }) else { return } + toolbar.insertItem(withItemIdentifier: .compareSaved, at: 0) + } + + private static let savedComparisonsInsertedKey = "compareSyncToolbarHasSavedComparisonsItem" + /// The HIG's item grouping: what the window is about on the leading edge, view controls in the /// middle, and the actions on the trailing edge, where "items on the trailing edge remain /// visible at all window sizes" and where the one primary action belongs. Compare used to sit @@ -143,7 +173,7 @@ internal final class CompareSyncWindowController: NSWindowController, /// for identity and made it the first thing to be clipped. internal func toolbarDefaultItemIdentifiers(_ toolbar: NSToolbar) -> [NSToolbarItem.Identifier] { [ - .compareSource, .compareSwap, .compareTarget, + .compareSaved, .compareSource, .compareSwap, .compareTarget, .flexibleSpace, .compareMode, .compareGrouping, .compareOptions, .compareSearch, .space, @@ -161,6 +191,8 @@ internal final class CompareSyncWindowController: NSWindowController, willBeInsertedIntoToolbar flag: Bool ) -> NSToolbarItem? { switch itemIdentifier { + case .compareSaved: + return savedComparisonsItem(itemIdentifier) case .compareSource: return endpointMenus.item(for: .source, identifier: itemIdentifier) case .compareTarget: @@ -291,6 +323,92 @@ internal final class CompareSyncWindowController: NSWindowController, modeControl?.isEnabled = !session.isBusy } + /// Loading a saved comparison is the one action that sets both endpoints at once, so it sits on + /// the leading edge with them rather than among the view controls. Its menu is built by + /// `menuNeedsUpdate` rather than at construction, because a comparison saved a moment ago has + /// to be in the list without the window being reopened. + private func savedComparisonsItem(_ identifier: NSToolbarItem.Identifier) -> NSToolbarItem { + let item = NSMenuToolbarItem(itemIdentifier: identifier) + item.label = String(localized: "Comparisons") + item.paletteLabel = String(localized: "Saved Comparisons") + item.toolTip = String(localized: "Load a saved source, target and options") + item.image = NSImage( + systemSymbolName: "list.star", + accessibilityDescription: String(localized: "Saved Comparisons") + ) + item.showsIndicator = true + let menu = NSMenu() + /// Identified rather than remembered. Customize Toolbar asks the delegate for more copies + /// of an item with `willBeInsertedIntoToolbar` false, so a stored reference ends up naming + /// a palette copy that was thrown away, and the menu the user actually opens never matches + /// it. The same trap `refreshTitles` documents for the endpoint items. + menu.identifier = Self.savedComparisonsMenuIdentifier + menu.delegate = self + item.menu = menu + return item + } + + private static let savedComparisonsMenuIdentifier = + NSUserInterfaceItemIdentifier("com.TablePro.compare.savedMenu") + + internal func menuNeedsUpdate(_ menu: NSMenu) { + guard menu.identifier == Self.savedComparisonsMenuIdentifier else { return } + menu.removeAllItems() + for profile in session.savedProfiles { + let entry = NSMenuItem(title: profile.name, action: #selector(loadProfile(_:)), keyEquivalent: "") + entry.target = self + entry.representedObject = profile.id + entry.toolTip = describe(profile) + menu.addItem(entry) + } + if !menu.items.isEmpty { + menu.addItem(.separator()) + } + let save = NSMenuItem( + title: String(localized: "Save Comparison…"), + action: #selector(saveComparison(_:)), + keyEquivalent: "" + ) + save.target = self + menu.addItem(save) + } + + private func describe(_ profile: CompareSyncProfile) -> String { + String( + format: String(localized: "%1$@ → %2$@, %3$@"), + profile.source.database, profile.target.database, profile.mode.displayName + ) + } + + @objc private func loadProfile(_ sender: NSMenuItem) { + guard let id = sender.representedObject as? UUID, + let profile = session.savedProfiles.first(where: { $0.id == id }) else { return } + guard session.apply(profile) else { return } + refreshEndpointChrome() + } + + /// The name is asked for in an alert rather than in the Options popover, because a user who has + /// just set up a pair should not have to find a text field in a settings sheet to keep it. + @objc internal func saveComparison(_ sender: Any?) { + guard let window, session.source != nil, session.target != nil else { return } + let alert = NSAlert() + alert.messageText = String(localized: "Save this comparison") + alert.informativeText = String( + localized: "The source, the target, the mode and the options come back when you load it." + ) + let field = NSTextField(frame: NSRect(x: 0, y: 0, width: 260, height: 24)) + field.placeholderString = String(localized: "Name") + alert.accessoryView = field + alert.addButton(withTitle: String(localized: "Save")) + alert.addButton(withTitle: String(localized: "Cancel")) + alert.beginSheetModal(for: window) { response in + guard response == .alertFirstButtonReturn else { return } + MainActor.assumeIsolated { self.session.saveProfile(named: field.stringValue) } + } + /// The accessory is only in the window once the sheet is up, so focus is asked for after. + DispatchQueue.main.async { alert.window.makeFirstResponder(field) } + } + private func groupingItem(_ identifier: NSToolbarItem.Identifier) -> NSToolbarItem { let item = NSMenuToolbarItem(itemIdentifier: identifier) item.label = String(localized: "Group By") @@ -371,8 +489,31 @@ internal final class CompareSyncWindowController: NSWindowController, /// wrong database as the one about to be written to. private func endpointsChanged() { session.resetComparison() + session.clearSetupErrorIfResolved() + refreshEndpointChrome() + } + + /// The Source and Target titles and the window subtitle are rendered rather than observed, so + /// anything that sets an endpoint from outside this controller, loading a saved comparison from + /// the Options popover for one, used to leave the toolbar naming the old pair. Validation runs + /// on every event loop turn and this writes only when the pair actually changed, so the chrome + /// follows the session wherever the change came from. + private func refreshEndpointChrome() { + /// Keyed on the setup generation as well as the pair, because loading a saved comparison + /// for the pair already on screen, or changing a matching option, resets the session + /// without changing either endpoint id. Without the generation the promised reload never + /// started and the pane sat empty. + let rendered = "\(session.source?.id ?? "")\u{1F}\(session.target?.id ?? "")\u{1F}\(session.setupGeneration)" + guard renderedEndpoints != rendered else { return } + renderedEndpoints = rendered endpointMenus.refreshTitles() updateSubtitle() + syncModeControl() + /// A new pair in data mode needs its table list, whoever set the pair. Loading a saved + /// comparison from the Options popover reaches the session without passing through this + /// controller at all, so the list has to follow the pair rather than the call site, and one + /// call site rather than several is what keeps two reads of it from starting at once. + runner.loadDataPlans() } @objc internal func showOptions(_ sender: Any?) { @@ -408,8 +549,7 @@ internal final class CompareSyncWindowController: NSWindowController, guard session.mode != mode else { return } session.mode = mode session.resetComparison() - syncModeControl() - endpointMenus.refreshTitles() + refreshEndpointChrome() } /// Only the Group By menu, found by identifier. Walking every `NSMenuToolbarItem` also reached @@ -453,6 +593,7 @@ internal final class CompareSyncWindowController: NSWindowController, let reason = disabledReason(for: item.action) describe(item, reason: reason) syncModeControl() + refreshEndpointChrome() return reason == nil } @@ -471,6 +612,12 @@ internal final class CompareSyncWindowController: NSWindowController, return session.isBusy ? nil : String(localized: "Nothing is running.") case #selector(performFind(_:)): return searchToolbarItem == nil ? String(localized: "The filter field is not in the toolbar.") : nil + case #selector(saveComparison(_:)): + if session.isBusy { return String(localized: "A run is already in progress.") } + guard session.source != nil, session.target != nil else { + return String(localized: "Choose a source and a target first.") + } + return nil default: return nil } @@ -501,8 +648,24 @@ internal final class CompareSyncWindowController: NSWindowController, // MARK: - Sheets + /// Apply builds the script itself when there is not one yet, so the sheet that reviews it is + /// one press from a finished comparison. Generate Script stays for a user who wants to read the + /// SQL, or copy it, without going near the sheet. private func presentApplySheet() { - guard let window, session.canApply else { return } + guard session.canApply else { return } + guard session.statements.isEmpty else { return showApplySheet() } + /// The pickers stay live while the script builds, so the setup can move under the build. + /// Opening the sheet then would label it with the new target and offer the old script to + /// run against it. The generation the build started at is what says whether that happened. + let generation = session.setupGeneration + Task { [runner] in + guard await runner.buildScriptIfNeeded(), session.isCurrent(generation) else { return } + showApplySheet() + } + } + + private func showApplySheet() { + guard let window else { return } let sheet = EscapeDismissingHostingController( rootView: CompareApplySheetView(session: session) { [weak self] choice in guard let self else { return } diff --git a/TableProTests/Core/Compare/CompareMetadataReadPlanTests.swift b/TableProTests/Core/Compare/CompareMetadataReadPlanTests.swift new file mode 100644 index 000000000..f10521dfb --- /dev/null +++ b/TableProTests/Core/Compare/CompareMetadataReadPlanTests.swift @@ -0,0 +1,293 @@ +// +// CompareMetadataReadPlanTests.swift +// TableProTests +// +// What a comparison costs a server, counted. +// +// The read used to be four statements per table per side, so a 200-table pair +// was 1,600 round trips before one difference appeared on screen. These pin the +// count rather than the wall clock: a driver that answers a whole schema in one +// query is asked once no matter how many tables it holds, and a driver that +// cannot is still asked correctly. +// + +@testable import TablePro +import TableProPluginKit +import XCTest + +final class CompareMetadataReadPlanTests: XCTestCase { + private func tables(_ count: Int) -> [PluginTableInfo] { + (0 ..< count).map { PluginTableInfo(name: "t\($0)", schema: "public", comment: nil) } + } + + // MARK: - A driver with the whole-schema reads + + func testAWholeSchemaDriverIsAskedOncePerKindNoMatterHowManyTables() async throws { + let driver = CountingMetadataDriver(bulk: true) + + let reads = try await CompareMetadataService.read( + tables: tables(200), schema: "public", profile: .structure, + narrowed: false, databaseType: .postgresql, using: driver + ) + + XCTAssertEqual(reads.count, 200) + XCTAssertEqual(driver.count(of: "fetchAllColumns"), 1) + XCTAssertEqual(driver.count(of: "fetchAllIndexes"), 1) + XCTAssertEqual(driver.count(of: "fetchAllForeignKeys"), 1) + XCTAssertEqual(driver.count(of: "fetchAllTableMetadata"), 1) + XCTAssertEqual(driver.totalCalls, 4, "a 200-table schema costs four statements, not eight hundred") + } + + func testNoPerTableReadSurvivesTheWholeSchemaPath() async throws { + let driver = CountingMetadataDriver(bulk: true) + + _ = try await CompareMetadataService.read( + tables: tables(50), schema: "public", profile: .structure, + narrowed: false, databaseType: .postgresql, using: driver + ) + + XCTAssertEqual(driver.count(of: "fetchColumns"), 0) + XCTAssertEqual(driver.count(of: "fetchIndexes"), 0) + XCTAssertEqual(driver.count(of: "fetchForeignKeys"), 0) + XCTAssertEqual(driver.count(of: "fetchTableMetadata"), 0) + } + + func testTheWholeSchemaValuesReachTheRead() async throws { + let driver = CountingMetadataDriver(bulk: true) + + let reads = try await CompareMetadataService.read( + tables: tables(3), schema: "public", profile: .structure, + narrowed: false, databaseType: .postgresql, using: driver + ) + + let snapshot = try XCTUnwrap(reads.first?.snapshot) + XCTAssertEqual(snapshot.columns.map(\.name), ["id"]) + XCTAssertEqual(snapshot.indexes.map(\.name), ["t0_pkey"]) + XCTAssertEqual(snapshot.engine, "TestEngine") + } + + // MARK: - A driver without them + + func testADriverWithoutWholeSchemaReadsStillReadsEveryTable() async throws { + let driver = CountingMetadataDriver(bulk: false) + + let reads = try await CompareMetadataService.read( + tables: tables(6), schema: "public", profile: .structure, + narrowed: false, databaseType: .postgresql, using: driver + ) + + XCTAssertEqual(reads.count, 6) + XCTAssertEqual(driver.count(of: "fetchColumns"), 6) + XCTAssertEqual(driver.count(of: "fetchIndexes"), 6) + XCTAssertEqual(driver.count(of: "fetchForeignKeys"), 6) + XCTAssertEqual(driver.count(of: "fetchTableMetadata"), 6) + XCTAssertEqual(driver.count(of: "fetchAllColumns"), 0, "a driver that has not declared one is never asked") + } + + /// A caller that named the tables it wants would have the whole-schema queries read the rest of + /// the database to throw it away, so those callers keep the per-table reads. + func testANarrowedReadStaysPerTableEvenOnAWholeSchemaDriver() async throws { + let driver = CountingMetadataDriver(bulk: true) + + _ = try await CompareMetadataService.read( + tables: tables(2), schema: "public", profile: .structure, + narrowed: true, databaseType: .postgresql, using: driver + ) + + XCTAssertEqual(driver.count(of: "fetchAllColumns"), 0) + XCTAssertEqual(driver.count(of: "fetchColumns"), 2) + } + + // MARK: - Profiles + + func testADataComparisonNeverReadsIndexesOrTableMetadata() async throws { + let driver = CountingMetadataDriver(bulk: true) + + _ = try await CompareMetadataService.read( + tables: tables(10), schema: "public", profile: .data, + narrowed: false, databaseType: .postgresql, using: driver + ) + + XCTAssertEqual(driver.count(of: "fetchAllColumns"), 1) + XCTAssertEqual(driver.count(of: "fetchAllForeignKeys"), 1, "the statement ordering needs them") + XCTAssertEqual(driver.count(of: "fetchAllIndexes"), 0) + XCTAssertEqual(driver.count(of: "fetchAllTableMetadata"), 0) + } + + func testADataComparisonOnAPerTableDriverSkipsThemToo() async throws { + let driver = CountingMetadataDriver(bulk: false) + + _ = try await CompareMetadataService.read( + tables: tables(4), schema: "public", profile: .data, + narrowed: false, databaseType: .postgresql, using: driver + ) + + XCTAssertEqual(driver.count(of: "fetchColumns"), 4) + XCTAssertEqual(driver.count(of: "fetchIndexes"), 0) + XCTAssertEqual(driver.count(of: "fetchTableMetadata"), 0) + } + + // MARK: - Failure + + /// A whole-schema query that fails takes nothing with it. The per-table read reports against + /// the one table it belongs to, which is what keeps one unreadable table from losing the + /// comparison. + func testAFailedWholeSchemaReadFallsBackToThePerTableRead() async throws { + let driver = CountingMetadataDriver(bulk: true) + driver.failsBulkColumns = true + + let reads = try await CompareMetadataService.read( + tables: tables(3), schema: "public", profile: .structure, + narrowed: false, databaseType: .postgresql, using: driver + ) + + XCTAssertEqual(driver.count(of: "fetchColumns"), 3) + XCTAssertEqual(reads.compactMap(\.snapshot).count, 3) + } + + func testAnUnreadableTableIsReportedWithoutLosingTheOthers() async throws { + let driver = CountingMetadataDriver(bulk: false) + driver.failingTable = "t1" + + let reads = try await CompareMetadataService.read( + tables: tables(3), schema: "public", profile: .structure, + narrowed: false, databaseType: .postgresql, using: driver + ) + + XCTAssertEqual(reads.count, 3) + XCTAssertEqual(reads.filter { $0.failure != nil }.map(\.table.name), ["t1"]) + XCTAssertEqual(reads.compactMap(\.snapshot).count, 2) + } + + /// A table listed under one folding and stored under another still has to find its entry. + func testALookupFoldsCase() async throws { + let driver = CountingMetadataDriver(bulk: true) + driver.uppercasesBulkKeys = true + + let reads = try await CompareMetadataService.read( + tables: tables(2), schema: "public", profile: .structure, + narrowed: false, databaseType: .postgresql, using: driver + ) + + XCTAssertEqual(driver.count(of: "fetchColumns"), 0, "the folded name matched, so nothing was re-read") + XCTAssertEqual(reads.compactMap(\.snapshot).count, 2) + } +} + +private final class CountingMetadataDriver: PluginDatabaseDriver, @unchecked Sendable { + private let lock = NSLock() + private var calls: [String: Int] = [:] + private let bulk: Bool + + var failsBulkColumns = false + var failingTable: String? + var uppercasesBulkKeys = false + + init(bulk: Bool) { + self.bulk = bulk + } + + func count(of call: String) -> Int { + lock.withLock { calls[call] ?? 0 } + } + + var totalCalls: Int { + lock.withLock { calls.values.reduce(0, +) } + } + + private func record(_ call: String) { + lock.withLock { calls[call, default: 0] += 1 } + } + + private func key(_ table: String) -> String { + uppercasesBulkKeys ? table.uppercased() : table + } + + private func columns(for table: String) -> [PluginColumnInfo] { + [PluginColumnInfo(name: "id", dataType: "INTEGER", isNullable: false, isPrimaryKey: true)] + } + + private func indexes(for table: String) -> [PluginIndexInfo] { + [PluginIndexInfo(name: "\(table)_pkey", columns: ["id"], isUnique: true, isPrimary: true)] + } + + private var knownTables: [String] { (0 ..< 200).map { "t\($0)" } } + + // MARK: - Whole schema + + var providesBulkColumnFetch: Bool { bulk } + var providesBulkIndexFetch: Bool { bulk } + var providesBulkForeignKeyFetch: Bool { bulk } + var providesBulkTableMetadataFetch: Bool { bulk } + + func fetchAllColumns(schema: String?) async throws -> [String: [PluginColumnInfo]] { + record("fetchAllColumns") + if failsBulkColumns { throw CocoaError(.fileReadUnknown) } + return Dictionary(uniqueKeysWithValues: knownTables.map { (key($0), columns(for: $0)) }) + } + + func fetchAllIndexes(schema: String?) async throws -> [String: [PluginIndexInfo]] { + record("fetchAllIndexes") + return Dictionary(uniqueKeysWithValues: knownTables.map { (key($0), indexes(for: $0)) }) + } + + func fetchAllForeignKeys(schema: String?) async throws -> [String: [PluginForeignKeyInfo]] { + record("fetchAllForeignKeys") + return [:] + } + + func fetchAllTableMetadata(schema: String?) async throws -> [String: PluginTableMetadata] { + record("fetchAllTableMetadata") + return Dictionary( + uniqueKeysWithValues: knownTables.map { + (key($0), PluginTableMetadata(tableName: $0, engine: "TestEngine")) + } + ) + } + + // MARK: - Per table + + func fetchColumns(table: String, schema: String?) async throws -> [PluginColumnInfo] { + record("fetchColumns") + if table == failingTable { throw CocoaError(.fileReadNoSuchFile) } + return columns(for: table) + } + + func fetchIndexes(table: String, schema: String?) async throws -> [PluginIndexInfo] { + record("fetchIndexes") + return indexes(for: table) + } + + func fetchForeignKeys(table: String, schema: String?) async throws -> [PluginForeignKeyInfo] { + record("fetchForeignKeys") + return [] + } + + func fetchTableMetadata(table: String, schema: String?) async throws -> PluginTableMetadata { + record("fetchTableMetadata") + return PluginTableMetadata(tableName: table, engine: "TestEngine") + } + + // MARK: - Unused + + func connect() async throws {} + func disconnect() {} + var isConnected: Bool { true } + + func execute(query: String) async throws -> PluginQueryResult { + PluginQueryResult(columns: [], columnTypeNames: [], rows: [], rowsAffected: 0, executionTime: 0) + } + + func fetchTableDDL(table: String, schema: String?) async throws -> String { "" } + func fetchViewDefinition(view: String, schema: String?) async throws -> String { "" } + func fetchDatabases() async throws -> [String] { [] } + + func fetchDatabaseMetadata(_ database: String) async throws -> PluginDatabaseMetadata { + PluginDatabaseMetadata(name: database) + } + + func fetchTables(schema: String?) async throws -> [PluginTableInfo] { + record("fetchTables") + return knownTables.map { PluginTableInfo(name: $0, schema: schema, comment: nil) } + } +} diff --git a/TableProTests/Core/Compare/CompareSyncSetupRestoreTests.swift b/TableProTests/Core/Compare/CompareSyncSetupRestoreTests.swift new file mode 100644 index 000000000..f77428e5b --- /dev/null +++ b/TableProTests/Core/Compare/CompareSyncSetupRestoreTests.swift @@ -0,0 +1,294 @@ +// +// CompareSyncSetupRestoreTests.swift +// TableProTests +// +// What a saved comparison restores, and what the window comes back to. +// +// A saved comparison used to restore the mode and the options and leave both +// pickers alone, while the list of them was filtered by the pair already on +// screen. So the one feature meant to save the setup work could only be reached +// by doing the setup work first. +// + +@testable import TablePro +import XCTest + +@MainActor +final class CompareSyncSetupRestoreTests: XCTestCase { + private let sourceConnection = UUID() + private let targetConnection = UUID() + private var storage: CompareSyncProfileStorage! + private var defaults: UserDefaults! + private let suiteName = "CompareSyncSetupRestoreTests" + + override func setUp() { + super.setUp() + UserDefaults.standard.removePersistentDomain(forName: suiteName) + defaults = UserDefaults(suiteName: suiteName) + storage = CompareSyncProfileStorage(defaults: defaults) + } + + override func tearDown() { + UserDefaults.standard.removePersistentDomain(forName: suiteName) + storage = nil + defaults = nil + super.tearDown() + } + + private func profile( + name: String = "Nightly", + source: UUID? = nil, + target: UUID? = nil, + mode: CompareSyncMode = .structure + ) -> CompareSyncProfile { + var structureOptions = StructureCompareOptions.default + structureOptions.ignoreIdentifierCase = false + return CompareSyncProfile( + name: name, + source: DatabaseScope(connectionId: source ?? sourceConnection, database: "prod", schema: "public"), + target: DatabaseScope(connectionId: target ?? targetConnection, database: "staging", schema: "public"), + mode: mode, + includedKinds: [.table, .view], + structureOptions: structureOptions, + dataOptions: .default, + selectedObjects: ["public\u{1F}orders"] + ) + } + + // MARK: - Storage + + func testTheLastSetupComesBackAsItWentIn() { + storage.rememberSetup(profile(name: "")) + + let restored = storage.lastSetup() + + XCTAssertEqual(restored?.source.database, "prod") + XCTAssertEqual(restored?.target.database, "staging") + XCTAssertEqual(restored?.includedKinds, [.table, .view]) + XCTAssertEqual(restored?.structureOptions.ignoreIdentifierCase, false) + } + + func testThereIsNoLastSetupBeforeOneIsWritten() { + XCTAssertNil(storage.lastSetup()) + } + + /// The last setup and the saved comparisons are separate slots: remembering where the window + /// was must not add an unnamed entry to the list the user curates. + func testRememberingTheLastSetupDoesNotSaveAComparison() { + storage.rememberSetup(profile(name: "")) + + XCTAssertTrue(storage.allProfiles().isEmpty) + } + + // MARK: - Restoring into a session + + func testRestoringWithNoPinnedSourceAdoptsBothScopes() { + let session = makeSession() + + session.restore(profile(), keepingSource: nil) + + XCTAssertEqual(session.includedKinds, [.table, .view]) + XCTAssertFalse(session.structureOptions.ignoreIdentifierCase) + } + + /// The window was opened against one connection, so that connection is the source. Inheriting a + /// target remembered against a different source would arm a database the user never paired with + /// this one, and the target is the side that gets written to. + func testAPinnedSourceFromAnotherConnectionDoesNotInheritTheRememberedTarget() { + let session = makeSession() + let pinned = endpoint(connectionId: UUID(), database: "other") + + session.restore(profile(), keepingSource: pinned) + + XCTAssertEqual(session.source, pinned) + XCTAssertNil(session.target, "a target is never inherited across an unrelated source") + } + + func testAPinnedSourceMatchingTheRememberedOneKeepsItsTarget() { + let session = makeSession() + let pinned = endpoint(connectionId: sourceConnection, database: "prod") + + session.restore(profile(), keepingSource: pinned) + + XCTAssertEqual(session.source, pinned) + XCTAssertEqual(session.target?.connectionId, targetConnection) + } + + func testRestoringNeverCarriesAnIncludedObjectForward() { + let session = makeSession() + + session.restore(profile(), keepingSource: nil) + + XCTAssertTrue(session.pendingSelection.isEmpty, "what to change is a decision about one report") + } + + // MARK: - The saved list + + func testEverySavedComparisonIsOfferedWhateverPairIsOnScreen() { + let session = makeSession() + storage.save(profile(name: "Nightly")) + + XCTAssertNil(session.source) + XCTAssertNil(session.target) + XCTAssertEqual( + session.savedProfiles.map(\.name), ["Nightly"], + "a saved comparison is what picks the pair, so it cannot require the pair first" + ) + } + + func testLoadingASavedComparisonSetsBothEndpoints() { + let session = makeSession() + + session.apply(profile()) + + XCTAssertEqual(session.source?.connectionId, sourceConnection) + XCTAssertEqual(session.source?.database, "prod") + XCTAssertEqual(session.target?.connectionId, targetConnection) + XCTAssertEqual(session.target?.database, "staging") + XCTAssertNil(session.setupErrorMessage) + } + + func testLoadingASavedComparisonWhoseConnectionIsGoneSaysSo() { + let session = makeSession() + + session.apply(profile(name: "Nightly", target: UUID())) + + XCTAssertNil(session.target, "a target that cannot be resolved is not silently left behind") + XCTAssertEqual(session.setupErrorMessage, "Nightly names a connection that no longer exists.") + } + + func testChangingTheSetupWritesItDownForNextTime() { + let session = makeSession() + session.source = endpoint(connectionId: sourceConnection, database: "prod") + session.target = endpoint(connectionId: targetConnection, database: "staging") + + session.resetComparison() + + XCTAssertEqual(storage.lastSetup()?.source.database, "prod") + XCTAssertEqual(storage.lastSetup()?.target.database, "staging") + } + + func testAHalfChosenPairIsNotWrittenDown() { + let session = makeSession() + session.source = endpoint(connectionId: sourceConnection, database: "prod") + + session.resetComparison() + + XCTAssertNil(storage.lastSetup(), "one endpoint is not a comparison to come back to") + } + + /// One connection reaches many databases, so a connection match is not a pair. The remembered + /// target is the side that gets written to, and inheriting it across databases would arm a + /// database the user never paired with this one. + func testAPinnedSourceOnTheSameConnectionButAnotherDatabaseInheritsNoTarget() { + let session = makeSession() + let pinned = endpoint(connectionId: sourceConnection, database: "other") + + session.restore(profile(), keepingSource: pinned) + + XCTAssertEqual(session.source, pinned) + XCTAssertNil(session.target, "a connection is not a pair") + } + + // MARK: - Fencing + + func testEverySetupChangeMovesTheGeneration() { + let session = makeSession() + let first = session.setupGeneration + + session.resetComparison() + + XCTAssertNotEqual(session.setupGeneration, first) + XCTAssertFalse(session.isCurrent(first), "an answer built for the old setup is no longer current") + XCTAssertTrue(session.isCurrent(session.setupGeneration)) + } + + func testLoadingAProfileIsRefusedWhileWorkIsRunning() { + let session = makeSession() + session.activity = .applying + + XCTAssertFalse(session.canLoadProfile) + XCTAssertFalse(session.apply(profile()), "a run in flight owns the target it captured") + XCTAssertNil(session.source, "nothing is adopted from a refused load") + } + + /// The option changes a load causes fire their own reset, which clears `errorMessage`. A message + /// about the setup has to outlive that or the failed load reports nothing at all. + func testAMissingConnectionSurvivesTheResetTheLoadCauses() { + let session = makeSession() + + session.apply(profile(name: "Nightly", target: UUID())) + session.resetComparison() + + XCTAssertEqual(session.setupErrorMessage, "Nightly names a connection that no longer exists.") + } + + func testChoosingBothEndpointsClearsTheSetupError() { + let session = makeSession() + session.apply(profile(name: "Nightly", target: UUID())) + + session.target = endpoint(connectionId: targetConnection, database: "staging") + session.clearSetupErrorIfResolved() + + XCTAssertNil(session.setupErrorMessage) + } + + // MARK: - Data plans + + /// A saved comparison names its tables, and the list now arrives before Compare. Left pending, + /// the saved set was reapplied at the next Compare over whatever the user had ticked since. + func testAdoptingPlansConsumesTheSavedSelection() { + let session = makeSession() + session.pendingSelection = ["public.orders"] + + session.adoptDataPlans([plan(id: "public.orders"), plan(id: "public.customers")]) + + XCTAssertEqual(session.dataPlans.filter(\.isEnabled).map(\.id), ["public.orders"]) + XCTAssertTrue(session.pendingSelection.isEmpty) + XCTAssertTrue(session.hasLoadedDataPlans) + } + + func testAdoptingPlansWithNothingPendingLeavesThemUnticked() { + let session = makeSession() + + session.adoptDataPlans([plan(id: "public.orders")]) + + XCTAssertTrue(session.dataPlans.allSatisfy { !$0.isEnabled }) + } + + private func plan(id: String) -> DataComparePlan { + DataComparePlan( + table: String(id.split(separator: ".").last ?? ""), + schema: "public", + targetSchema: "public", + columns: ["id"], + columnDescriptors: [KeyColumnDescriptor(name: "id", dataType: "INTEGER", collation: nil)], + generatedColumns: [], + keyColumns: ["id"], + isEnabled: false, + excludedRowKeys: [] + ) + } + + private func makeSession() -> CompareSyncSession { + let connections = [ + connection(id: sourceConnection, name: "prod"), + connection(id: targetConnection, name: "staging") + ] + return CompareSyncSession(profileStorage: storage, connectionsProvider: { connections }) + } + + private func connection(id: UUID, name: String) -> DatabaseConnection { + DatabaseConnection(id: id, name: name, database: name, type: .postgresql) + } + + private func endpoint(connectionId: UUID, database: String) -> DatabaseEndpoint { + DatabaseEndpoint( + scope: DatabaseScope(connectionId: connectionId, database: database, schema: "public"), + connectionName: database, + databaseType: .postgresql, + safeModeLevel: .silent, + color: .blue + ) + } +} diff --git a/TableProUITests/CompareSyncUITests.swift b/TableProUITests/CompareSyncUITests.swift index 860bb5956..ba016e6d7 100644 --- a/TableProUITests/CompareSyncUITests.swift +++ b/TableProUITests/CompareSyncUITests.swift @@ -57,4 +57,21 @@ final class CompareSyncUITests: UITestCase { guard dialog.exists, dialog.buttons["Cancel"].exists else { return } dialog.buttons["Cancel"].click() } + + /// The HIG's rule that every toolbar item is also a menu-bar command, checked where it can be + /// checked without a license: the item has to be in the menu even though it validates to + /// disabled with no window behind it. + func testSavingAComparisonIsAMenuBarCommand() throws { + let app = try launchApp() + + let menuBar = app.menuBars.firstMatch + XCTAssertTrue(menuBar.waitToExist(timeout: 10)) + menuBar.menuBarItems["Database"].click() + menuBar.menuItems["Compare"].click() + + XCTAssertTrue( + menuBar.menuItems["Save Comparison…"].waitToExist(timeout: 10), + "Save Comparison must be reachable from Database > Compare" + ) + } } diff --git a/docs/development/plugin-development.mdx b/docs/development/plugin-development.mdx index 563002112..5755f33b8 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: 19 | +| `TableProPluginKitVersion` | integer | Yes | The PluginKit ABI the plugin was built against. Current value: 20 | | `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 68a12d209..bbc54b2a8 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": 19, + "pluginKitVersion": 20, "downloadURL": "https://github.com/TableProApp/TablePro/releases/download/plugin-oracle-v1.0.26/OracleDriver-arm64.zip", "sha256": "" }, { "architecture": "x86_64", - "pluginKitVersion": 19, + "pluginKitVersion": 20, "downloadURL": "https://github.com/TableProApp/TablePro/releases/download/plugin-oracle-v1.0.26/OracleDriver-x86_64.zip", "sha256": "" } diff --git a/docs/features/compare-sync.mdx b/docs/features/compare-sync.mdx index c4da9bc45..832e87d7a 100644 --- a/docs/features/compare-sync.mdx +++ b/docs/features/compare-sync.mdx @@ -15,12 +15,16 @@ Pick a source and a target, press **Compare**, and every object that differs lis - **Database > Compare > Compare & Sync Databases…** - Right-click a connection in the connection list and choose **Compare/Sync with…**. The connection clicked becomes the source. +The window comes back on the source, target, mode and options it last held, so running the same comparison again is one press of **Compare**. Opening it from a connection makes that connection the source, and the remembered target comes back only when it was the target of that same source. + Every toolbar control also sits under **Database > Compare**, so the whole flow is reachable from the keyboard. ## Choosing the two sides **Source** and **Target** are database pickers, not connection pickers: each one walks connection, then database, then schema. Two databases on one server are a valid pair, and so are two schemas in one database. +**Comparisons** in the toolbar lists every saved setup and sets both pickers from the one chosen. It does not need a pair to be chosen first. + The **source** never changes. The **target** is written to. A connection whose safe mode level is **Read-Only** is disabled in the target picker with the reason shown, so the refusal arrives at selection time rather than after comparing. **Swap** reverses the direction. Nothing is written until **Apply**. Until then the strip along the top reads **Comparing only. Nothing has been written.** @@ -55,7 +59,7 @@ These drift between environments by design, so they are ignored by default. Turn Each object lands in one of four states: **only in source**, **only in target**, **different**, or **identical**. **Group By** sections the table by difference or by object kind, and the search field filters by name. Identical objects stay hidden until **Show Identical Objects**. -Every row carries an **Include** checkbox, and a group header carries one for everything under it. Nothing is included until it is checked. +Every row carries an **Include** checkbox, and a group header carries one for everything under it. **Select > All** covers everything the pane is currently showing, so the search field and **Show Identical Objects** narrow what it reaches. Nothing is included until it is checked. An object whose metadata could not be read keeps its own **Could Not Compare** section with the driver's reason. One unreadable object never stops the rest of the comparison. @@ -63,9 +67,9 @@ The detail pane on the right has three tabs. **Definitions** shows the source an ## Comparing rows -Switch the mode control to **Data**. The left pane lists the tables present on both sides. +Switch the mode control to **Data**. The left pane lists the tables present on both sides as soon as there is a pair to list them for, without reading a row. -Tables start unchecked. Choose the ones to compare, then press **Compare**: a data comparison reads every row of every checked table on both sides, so comparing a whole database by accident is expensive. +Tables start unchecked. Tick the ones to compare, then press **Compare**: a data comparison reads every row of every ticked table on both sides, so comparing a whole database by accident is expensive. Rows are matched by key, read in key order from both sides and walked in lockstep, so neither side is ever held in memory in full. @@ -95,7 +99,7 @@ Script generation needs matching database types, with MySQL and MariaDB counting Anything that would destroy data is generated, listed, and held back. Dropping a table, dropping a column, narrowing a type, adding NOT NULL, changing a primary key and deleting a row each need an explicit allowance, and that allowance covers one run and is never saved. -**Apply…** opens a sheet with the script, a summary, and the warnings. **Cancel** is the default button and **Apply** is marked destructive. Apply stays disabled while any included statement still has an unacknowledged hazard. +**Apply…** opens a sheet with the script, a summary, and the warnings, building the script first when **Generate Script** has not run. **Cancel** is the default button and **Apply** is marked destructive. Allow a held-back statement in the sheet or in the **Script** pane; the sheet's **Apply** stays disabled while any included statement is still held back. Applying runs the script against the target. Statements that already ran stay applied unless the whole run is inside a transaction that rolls back. @@ -109,4 +113,8 @@ Closing the window mid-run asks first, and says that statements which already ra ## Saving a comparison -A named comparison remembers the source, the target, the mode, the object kinds, the options and the included objects. Load it from **Options** to run the same comparison again. +**Save Comparison…** names the current setup: the source, the target, the mode, the object kinds, the options and the included objects. It sits in the **Comparisons** menu and under **Database > Compare**. + +Loading one sets both pickers. **Comparisons** lists them all whatever pair is on screen; **Options** lists them too, with a **Delete** for each. + +A saved comparison whose connection has since been deleted says so instead of loading half a pair. diff --git a/scripts/check-sqlite-bulk-metadata.sh b/scripts/check-sqlite-bulk-metadata.sh new file mode 100755 index 000000000..c723ccdfe --- /dev/null +++ b/scripts/check-sqlite-bulk-metadata.sh @@ -0,0 +1,108 @@ +#!/usr/bin/env bash +# +# Checks that the SQLite driver's whole-schema metadata reads agree with its +# per-table ones. +# +# The bulk read stands in for the per-table read, so the two have to answer the +# same question. They did not: `fetchAllColumns` used `pragma_table_info`, which +# omits generated columns entirely, while `fetchColumns` uses `table_xinfo` for +# exactly that reason. A comparison built on the bulk read saw neither side's +# generated columns and reported them as matching. +# +# The queries here are the ones the driver runs, so a SQLite upgrade that changes +# what a pragma reports fails this instead of shipping a silent disagreement. +# +# Usage: scripts/check-sqlite-bulk-metadata.sh +set -euo pipefail + +if ! command -v sqlite3 >/dev/null 2>&1; then + echo "sqlite3 is not on PATH" >&2 + exit 2 +fi + +workdir="$(mktemp -d)" +trap 'rm -rf "$workdir"' EXIT +db="$workdir/probe.db" + +sqlite3 "$db" <<'SQL' +CREATE TABLE orders( + id INTEGER PRIMARY KEY, + a TEXT, + b TEXT, + virtual_len INT GENERATED ALWAYS AS (length(a)) VIRTUAL, + stored_len INT GENERATED ALWAYS AS (length(b)) STORED +); +CREATE UNIQUE INDEX ux_orders_ab ON orders(a, b); +CREATE INDEX ix_orders_b ON orders(b); +CREATE TABLE composite(a TEXT, b TEXT, v TEXT, PRIMARY KEY(a, b)); +CREATE TABLE plain(id INTEGER PRIMARY KEY, x TEXT); +SQL + +status=0 + +# fetchAllColumns, from SQLitePlugin.swift. table_xinfo, not table_info: the +# `hidden` column is what marks a generated column 2 (VIRTUAL) or 3 (STORED). +bulk_columns() { + sqlite3 "$db" 'SELECT m.name, p.cid, p.name, p.type, p."notnull", p.dflt_value, p.pk, p.hidden + FROM sqlite_master m, pragma_table_xinfo(m.name) p + WHERE m.type = '"'"'table'"'"' AND m.name NOT LIKE '"'"'sqlite_%'"'"' + ORDER BY m.name, p.cid;' +} + +# fetchColumns, from SQLitePlugin.swift, run for one table. +per_table_columns() { + sqlite3 "$db" "SELECT '$1', p.cid, p.name, p.type, p.\"notnull\", p.dflt_value, p.pk, p.hidden + FROM pragma_table_xinfo('$1') p ORDER BY p.cid;" +} + +# fetchAllIndexes, from SQLitePluginDriver+BulkMetadata.swift. +bulk_indexes() { + sqlite3 "$db" 'SELECT m.name, il.name, il."unique", il.origin, ii.name + FROM sqlite_master m + JOIN pragma_index_list(m.name) il + LEFT JOIN pragma_index_info(il.name) ii ON 1=1 + WHERE m.type = '"'"'table'"'"' AND m.name NOT LIKE '"'"'sqlite_%'"'"' + ORDER BY m.name, il.seq, ii.seqno;' +} + +# fetchIndexes, from SQLitePlugin.swift, run for one table. +per_table_indexes() { + sqlite3 "$db" "SELECT '$1', il.name, il.\"unique\", il.origin, ii.name + FROM pragma_index_list('$1') il + LEFT JOIN pragma_index_info(il.name) ii ON 1=1 + ORDER BY il.seq, ii.seqno;" +} + +tables="orders composite plain" + +for table in $tables; do + if ! diff -u \ + <(per_table_columns "$table") \ + <(bulk_columns | grep "^$table|" || true) >"$workdir/columns-$table.diff"; then + echo "columns disagree for $table:" >&2 + cat "$workdir/columns-$table.diff" >&2 + status=1 + fi + + if ! diff -u \ + <(per_table_indexes "$table") \ + <(bulk_indexes | grep "^$table|" || true) >"$workdir/indexes-$table.diff"; then + echo "indexes disagree for $table:" >&2 + cat "$workdir/indexes-$table.diff" >&2 + status=1 + fi +done + +# The reason the columns read moved to table_xinfo. A generated column must be +# in the list, and must carry the hidden flag that says which kind it is. +generated="$(bulk_columns | grep -c '|virtual_len|\||stored_len|' || true)" +if [ "$generated" -ne 2 ]; then + echo "the whole-schema column read lost a generated column (found $generated of 2)" >&2 + status=1 +fi + +if [ "$status" -eq 0 ]; then + echo "SQLite whole-schema reads agree with the per-table reads ($(sqlite3 "$db" 'SELECT sqlite_version();'))" +fi + +exit "$status"