diff --git a/CHANGELOG.md b/CHANGELOG.md index 31f5f990f5..d004679b3c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -335,6 +335,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - DuckDB macro dropped and not recreated by a Compare & Sync replace. - Copy To giving no reason for a view, routine or trigger whose definition could not be read. - Copy To skipping a view, routine or trigger with a comment above its `CREATE`. +- Materialized view indexes ignored by Compare & Sync, and lost when it or Copy To recreated the view. - SSH jump hosts dropped from a connection synced to iPhone and iPad, and that connection then skipped on the way back. - An SSH tunnel pinned to port 22, and its auth method read back as Password, after a round trip through iPhone and iPad. - Redis database list failing on servers that refuse `CONFIG` or `INFO`, such as AWS ElastiCache and Azure Cache for Redis. (#3036) diff --git a/TablePro/Core/Compare/CompareMetadataService+SourceDefinitions.swift b/TablePro/Core/Compare/CompareMetadataService+SourceDefinitions.swift index f64a8fca3c..c9e023714b 100644 --- a/TablePro/Core/Compare/CompareMetadataService+SourceDefinitions.swift +++ b/TablePro/Core/Compare/CompareMetadataService+SourceDefinitions.swift @@ -15,6 +15,7 @@ internal struct RoutineSourceRead: Sendable { internal let signature: String? internal let source: String internal let failure: String? + internal let indexes: ObjectIndexRead? internal init( name: String, @@ -22,7 +23,8 @@ internal struct RoutineSourceRead: Sendable { schema: String?, signature: String?, source: String, - failure: String? = nil + failure: String? = nil, + indexes: ObjectIndexRead? = nil ) { self.name = name self.kind = kind @@ -30,6 +32,7 @@ internal struct RoutineSourceRead: Sendable { self.signature = signature self.source = source self.failure = failure + self.indexes = indexes } } @@ -39,21 +42,22 @@ internal extension CompareMetadataService { ) nonisolated static func readViewDefinitions( - _ views: [PluginTableInfo], + _ views: [TableStructureRead], schema: String?, using plugin: any PluginDatabaseDriver ) async throws -> [RoutineSourceRead] { var reads: [RoutineSourceRead] = [] for view in views { try Task.checkCancellation() - let viewSchema = view.schema ?? schema + let viewSchema = view.table.schema ?? schema reads.append(try await definitionRead( - name: view.name, - kind: CompareTableKindClassifier.kind(of: view), + name: view.table.name, + kind: CompareTableKindClassifier.kind(of: view.table), schema: viewSchema, - signature: nil + signature: nil, + indexes: view.objectIndexes ) { - try await plugin.fetchViewDefinition(view: view.name, schema: viewSchema) + try await plugin.fetchViewDefinition(view: view.table.name, schema: viewSchema) }) } return reads @@ -175,11 +179,14 @@ internal extension CompareMetadataService { kind: CompareObjectKind, schema: String?, signature: String?, + indexes: ObjectIndexRead? = nil, reading: () async throws -> String ) async throws -> RoutineSourceRead { do { let source = try await reading() - return RoutineSourceRead(name: name, kind: kind, schema: schema, signature: signature, source: source) + return RoutineSourceRead( + name: name, kind: kind, schema: schema, signature: signature, source: source, indexes: indexes + ) } catch { guard !(error is CancellationError), !Task.isCancelled else { throw CancellationError() } definitionLogger.warning( @@ -187,7 +194,7 @@ internal extension CompareMetadataService { ) return RoutineSourceRead( name: name, kind: kind, schema: schema, signature: signature, - source: "", failure: error.localizedDescription + source: "", failure: error.localizedDescription, indexes: indexes ) } } diff --git a/TablePro/Core/Compare/CompareMetadataService.swift b/TablePro/Core/Compare/CompareMetadataService.swift index 01ffd129f8..5716bc9c45 100644 --- a/TablePro/Core/Compare/CompareMetadataService.swift +++ b/TablePro/Core/Compare/CompareMetadataService.swift @@ -30,6 +30,7 @@ internal struct TableStructureRead: Sendable { internal let foreignKeys: [PluginForeignKeyInfo] internal let metadata: PluginTableMetadata? internal let failure: String? + internal var objectIndexes: ObjectIndexRead? internal var snapshot: TableStructureSnapshot? { snapshot(indexes: indexes) @@ -118,6 +119,7 @@ internal struct CompareMetadataService { try await manager.ensureConnected(connection) let schema = endpoint.schema let databaseType = endpoint.databaseType + let indexedKinds = SourceObjectIndexes.carriedKinds(on: databaseType) let wanted = names.map { Set($0.map { $0.lowercased() }) } return try await manager.withMetadataDriver(scope: endpoint.scope) { driver in @@ -129,7 +131,8 @@ internal struct CompareMetadataService { } return try await Self.read( tables: tables, schema: schema, profile: profile, - narrowed: wanted != nil, databaseType: databaseType, using: plugin + narrowed: wanted != nil, databaseType: databaseType, + indexedKinds: indexedKinds, using: plugin ) } } @@ -191,7 +194,7 @@ internal struct CompareMetadataService { internal func viewDefinitions( for endpoint: DatabaseEndpoint, connection: DatabaseConnection, - views: [PluginTableInfo] + views: [TableStructureRead] ) async throws -> [RoutineSourceRead] { try await manager.ensureConnected(connection) let schema = endpoint.schema @@ -223,6 +226,7 @@ internal struct CompareMetadataService { profile: TableReadProfile, narrowed: Bool, databaseType: DatabaseType, + indexedKinds: Set, using plugin: any PluginDatabaseDriver ) async throws -> [TableStructureRead] { let bulk = narrowed @@ -240,7 +244,10 @@ internal struct CompareMetadataService { : 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) + await read( + table: table, schema: table.schema ?? schema, profile: profile, bulk: bulk, + indexedKinds: indexedKinds, using: plugin + ) } } @@ -380,18 +387,25 @@ internal struct CompareMetadataService { schema: String?, profile: TableReadProfile, bulk: BulkMetadata, + indexedKinds: Set, using plugin: any PluginDatabaseDriver ) async -> TableStructureRead { + let kind = CompareTableKindClassifier.kind(of: table) + let objectIndexes = kind != .table && indexedKinds.contains(kind) + ? await objectIndexes(of: table, schema: schema, profile: profile, bulk: bulk, using: plugin) + : nil do { let columns = try await columns(of: table, schema: schema, bulk: bulk, using: plugin) - let indexes = try await indexes(of: table, schema: schema, profile: profile, bulk: bulk, using: plugin) + let indexes = try await indexes( + of: table, schema: schema, profile: profile, bulk: bulk, objectIndexes: objectIndexes, using: plugin + ) let foreignKeys = try 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 + foreignKeys: foreignKeys, metadata: metadata, failure: nil, objectIndexes: objectIndexes ) } catch { Self.logger.warning( @@ -399,11 +413,30 @@ internal struct CompareMetadataService { ) return TableStructureRead( table: table, columns: [], indexes: [], foreignKeys: [], - metadata: nil, failure: error.localizedDescription + metadata: nil, failure: error.localizedDescription, objectIndexes: objectIndexes ) } } + nonisolated private static func objectIndexes( + of object: PluginTableInfo, + schema: String?, + profile: TableReadProfile, + bulk: BulkMetadata, + using plugin: any PluginDatabaseDriver + ) async -> ObjectIndexRead? { + guard profile.wantsIndexes else { return nil } + guard bulk.indexes == nil else { return .read(bulk.lookup(bulk.indexes, object.name) ?? []) } + do { + return .read(try await plugin.fetchIndexes(table: object.name, schema: schema)) + } catch { + Self.logger.warning( + "Index read failed for \(object.name, privacy: .private(mask: .hash)): \(error.publicLogShape, privacy: .public)" + ) + return .failed(error.localizedDescription) + } + } + /// 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. @@ -425,20 +458,25 @@ internal struct CompareMetadataService { /// offers to drop every index the table really has. So the failure fails the table, the way the /// column read already did, and the comparison reports it rather than acting on it. /// - /// A view is the one object where the failure means nothing: it has neither indexes nor foreign - /// keys, and engines disagree on whether asking answers empty or refuses. + /// A view or a materialized view is not asked here. Its indexes are part of its comparison only + /// where the engine's structure matrix says that kind takes them, and that read keeps its own + /// failure in `objectIndexes` rather than failing the object: engines whose views take no index + /// disagree on whether asking answers empty or refuses, and several answer with clustering or + /// sort keys that no `CREATE INDEX` can write back. nonisolated private static func indexes( of table: PluginTableInfo, schema: String?, profile: TableReadProfile, bulk: BulkMetadata, + objectIndexes: ObjectIndexRead?, using plugin: any PluginDatabaseDriver ) async throws -> [PluginIndexInfo] { guard profile.wantsIndexes else { return [] } - guard bulk.indexes == nil else { return bulk.lookup(bulk.indexes, table.name) ?? [] } guard CompareTableKindClassifier.kind(of: table) == .table else { - return (try? await plugin.fetchIndexes(table: table.name, schema: schema)) ?? [] + guard case .read(let indexes) = objectIndexes else { return [] } + return indexes } + guard bulk.indexes == nil else { return bulk.lookup(bulk.indexes, table.name) ?? [] } return try await plugin.fetchIndexes(table: table.name, schema: schema) } diff --git a/TablePro/Core/Compare/CompareObjectResult.swift b/TablePro/Core/Compare/CompareObjectResult.swift index 78b4f5c9a4..2a22cdcc0d 100644 --- a/TablePro/Core/Compare/CompareObjectResult.swift +++ b/TablePro/Core/Compare/CompareObjectResult.swift @@ -7,7 +7,9 @@ // A table is compared as parsed metadata and yields a `SchemaChange` list. A // view, procedure, function or trigger has no such structure: its definition is // a body of SQL, so it is compared as normalised text and the only honest -// answers are create, replace and drop. Both arrive here as one type, so the +// answers are create, replace and drop. A materialized view whose engine takes +// indexes on it also carries those indexes, which change in place when the text +// is the same on both sides. Both arrive here as one type, so the // results list, the selection model and the script builder do not each need to // know which engine produced a row. // @@ -53,6 +55,9 @@ internal struct CompareObjectResult: Identifiable, Hashable, Sendable { internal let targetDefinition: [String] internal let notes: [String] internal let comparisonError: String? + internal let sourceIndexes: [EditableIndexDefinition]? + internal let targetIndexes: [EditableIndexDefinition]? + internal let definitionMatches: Bool internal init( identity: CompareObjectIdentity, @@ -61,7 +66,10 @@ internal struct CompareObjectResult: Identifiable, Hashable, Sendable { sourceDefinition: [String] = [], targetDefinition: [String] = [], notes: [String] = [], - comparisonError: String? = nil + comparisonError: String? = nil, + sourceIndexes: [EditableIndexDefinition]? = nil, + targetIndexes: [EditableIndexDefinition]? = nil, + definitionMatches: Bool = false ) { self.identity = identity self.status = status @@ -70,10 +78,25 @@ internal struct CompareObjectResult: Identifiable, Hashable, Sendable { self.targetDefinition = targetDefinition self.notes = notes self.comparisonError = comparisonError + self.sourceIndexes = sourceIndexes + self.targetIndexes = targetIndexes + self.definitionMatches = definitionMatches } internal var id: String { identity.id } + internal var showsIndexes: Bool { + sourceIndexes != nil || (status == .onlyInTarget && targetIndexes != nil) + } + + internal var sourceIndexLines: [String] { + TableDefinitionRenderer.indexLines(for: sourceIndexes ?? []) + } + + internal var targetIndexLines: [String] { + TableDefinitionRenderer.indexLines(for: targetIndexes ?? []) + } + internal var isComparable: Bool { comparisonError == nil } internal var suggestedAction: TableSyncAction { diff --git a/TablePro/Core/Compare/CompareRunner.swift b/TablePro/Core/Compare/CompareRunner.swift index b028ca407a..46ca41776d 100644 --- a/TablePro/Core/Compare/CompareRunner.swift +++ b/TablePro/Core/Compare/CompareRunner.swift @@ -362,15 +362,16 @@ internal struct CompareRunner { let diffEngine = SourceObjectDiffEngine( options: session.structureOptions, sourceDatabaseType: context.source.databaseType, - targetDatabaseType: context.target.databaseType + targetDatabaseType: context.target.databaseType, + targetIndexedKinds: SourceObjectIndexes.carriedKinds(on: context.target.databaseType) ) /// Each pair reads two independent endpoints, so the two sides run together rather than the /// second waiting out the first. let viewKinds = includedKinds.intersection([.view, .materializedView]) if !viewKinds.isEmpty { - let sourceViews = sourceReads.map(\.table).filter { viewKinds.contains(CompareTableKindClassifier.kind(of: $0)) } - let targetViews = targetReads.map(\.table).filter { viewKinds.contains(CompareTableKindClassifier.kind(of: $0)) } + let sourceViews = sourceReads.filter { viewKinds.contains(CompareTableKindClassifier.kind(of: $0.table)) } + let targetViews = targetReads.filter { viewKinds.contains(CompareTableKindClassifier.kind(of: $0.table)) } async let sourceDefinitions = metadataService.viewDefinitions( for: context.source, connection: context.sourceConnection, views: sourceViews ) @@ -445,7 +446,9 @@ internal struct CompareRunner { targetDriver: plugin, targetDatabaseType: driver.connection.type ).build(operations: tableOperations, foreignKeysByTable: foreignKeys) let sourceBuilder = SourceObjectSyncBuilder( - targetDriver: plugin, targetDatabaseType: driver.connection.type + targetDriver: plugin, + targetDatabaseType: driver.connection.type, + indexSchema: plugin.currentSchema ) for entry in sourceDefined { statements += try sourceBuilder.build(for: entry.result, action: entry.action) diff --git a/TablePro/Core/Compare/SchemaSyncScriptBuilder.swift b/TablePro/Core/Compare/SchemaSyncScriptBuilder.swift index 7eb3a53469..c9dd60e236 100644 --- a/TablePro/Core/Compare/SchemaSyncScriptBuilder.swift +++ b/TablePro/Core/Compare/SchemaSyncScriptBuilder.swift @@ -84,8 +84,8 @@ internal struct SchemaSyncScriptBuilder { return try createStatements(for: snapshot) case .dropTable(let name, let schema): return dropStatements(name: name, schema: schema) - case .alterTable(let name, let schema, let changes): - return try alterStatements(name: name, schema: schema, changes: changes) + case .alterTable(let name, _, let changes): + return try changeStatements(on: name, objectName: name, changes: changes) } } @@ -135,19 +135,20 @@ internal struct SchemaSyncScriptBuilder { } } - private func alterStatements( - name: String, - schema: String?, - changes: [SchemaChange] + internal func changeStatements( + on relation: String, + objectName: String, + changes: [SchemaChange], + additionalHazards: (SchemaChange) -> [SyncHazard] = { _ in [] } ) throws -> [SyncStatement] { - let generator = SchemaStatementGenerator(tableName: name, pluginDriver: targetDriver) + let generator = SchemaStatementGenerator(tableName: relation, pluginDriver: targetDriver) var statements: [SyncStatement] = [] for change in SchemaChangeOrdering.sorted(changes) { - let hazards = classifier.hazards(for: change, typeFamily: targetTypeFamily) + let hazards = classifier.hazards(for: change, typeFamily: targetTypeFamily) + additionalHazards(change) let generated = try generator.generate(changes: [change]) for statement in generated { statements += scriptText.sendableStatements(statement.sql).map { sql in - SyncStatement(sql: sql, objectName: name, summary: statement.description, hazards: hazards) + SyncStatement(sql: sql, objectName: objectName, summary: statement.description, hazards: hazards) } } } diff --git a/TablePro/Core/Compare/SourceObjectDiffEngine.swift b/TablePro/Core/Compare/SourceObjectDiffEngine.swift index 94d87231a5..f9481ba8f0 100644 --- a/TablePro/Core/Compare/SourceObjectDiffEngine.swift +++ b/TablePro/Core/Compare/SourceObjectDiffEngine.swift @@ -23,15 +23,18 @@ internal struct SourceObjectDiffEngine { private let options: StructureCompareOptions private let sourceScriptText: SQLScriptText private let targetScriptText: SQLScriptText + private let targetIndexedKinds: Set internal init( options: StructureCompareOptions = .default, sourceDatabaseType: DatabaseType, - targetDatabaseType: DatabaseType + targetDatabaseType: DatabaseType, + targetIndexedKinds: Set ) { self.options = options self.sourceScriptText = SQLScriptText(databaseType: sourceDatabaseType) self.targetScriptText = SQLScriptText(databaseType: targetDatabaseType) + self.targetIndexedKinds = targetIndexedKinds } internal func compare( @@ -65,29 +68,94 @@ internal struct SourceObjectDiffEngine { ) -> CompareObjectResult { let sourceDefect = source.flatMap { SourceDefinitionDefect.of($0, sentAs: sourceScriptText) } let targetDefect = target.flatMap { SourceDefinitionDefect.of($0, sentAs: targetScriptText) } - let comparisonError = sourceDefect.map { $0.reason(on: .source) } ?? targetDefect.map { $0.reason(on: .target) } + let indexes = indexComparison(source: source, target: target) + let comparisonError = sourceDefect.map { $0.reason(on: .source) } + ?? targetDefect.map { $0.reason(on: .target) } + ?? indexes.failure + let definitionMatches = comparisonError == nil && definitionsMatch(source: source, target: target) return CompareObjectResult( identity: CompareObjectIdentity( kind: read.kind, schema: read.schema, name: read.name, signature: read.signature ), - status: status(source: source, target: target, comparable: comparisonError == nil), + status: status( + source: source, + target: target, + comparable: comparisonError == nil, + definitionMatches: definitionMatches, + indexes: indexes + ), + changes: comparisonError == nil ? indexes.changes : [], sourceDefinition: source.map(displayedLines) ?? [], targetDefinition: target.map(displayedLines) ?? [], - comparisonError: comparisonError + notes: comparisonError == nil ? indexes.notes : [], + comparisonError: comparisonError, + sourceIndexes: indexes.source, + targetIndexes: indexes.target, + definitionMatches: definitionMatches ) } private func status( source: RoutineSourceRead?, target: RoutineSourceRead?, - comparable: Bool + comparable: Bool, + definitionMatches: Bool, + indexes: IndexComparison ) -> TableDiffStatus { - guard let source else { return .onlyInTarget } - guard let target else { return .onlyInSource } - guard comparable else { return .differs } - let equal = normalize(source.source, scriptText: sourceScriptText) + guard source != nil else { return .onlyInTarget } + guard target != nil else { return .onlyInSource } + guard comparable, definitionMatches else { return .differs } + return indexes.changes.isEmpty && indexes.notes.isEmpty ? .identical : .differs + } + + private func definitionsMatch(source: RoutineSourceRead?, target: RoutineSourceRead?) -> Bool { + guard let source, let target else { return false } + return normalize(source.source, scriptText: sourceScriptText) == normalize(target.source, scriptText: targetScriptText) - return equal ? .identical : .differs + } + + private struct IndexComparison { + var source: [EditableIndexDefinition]? + var target: [EditableIndexDefinition]? + var changes: [SchemaChange] = [] + var notes: [String] = [] + var failure: String? + } + + private func indexComparison(source: RoutineSourceRead?, target: RoutineSourceRead?) -> IndexComparison { + var comparison = IndexComparison() + if case .read(let found)? = target?.indexes { + comparison.target = SourceObjectIndexes.definitions(found) + } + guard let source, let sourceRead = source.indexes else { return comparison } + guard targetIndexedKinds.contains(source.kind) else { + if target == nil, case .read(let found) = sourceRead, !SourceObjectIndexes.definitions(found).isEmpty { + comparison.notes = [SourceObjectIndexes.notCarriedByTargetNote] + } + return comparison + } + switch sourceRead { + case .failed(let reason): + comparison.failure = String(format: String(localized: "The source's indexes could not be read: %@"), reason) + return comparison + case .read(let found): + comparison.source = SourceObjectIndexes.definitions(found) + } + guard let target else { return comparison } + guard let targetRead = target.indexes, let sourceIndexes = comparison.source else { + comparison.source = nil + return comparison + } + if case .failed(let reason) = targetRead { + comparison.failure = String(format: String(localized: "The target's indexes could not be read: %@"), reason) + return comparison + } + let outcome = StructureDiffEngine(options: options).indexChanges( + source: sourceIndexes, target: comparison.target ?? [] + ) + comparison.changes = outcome.changes + comparison.notes = outcome.notes + return comparison } private func displayedLines(_ read: RoutineSourceRead) -> [String] { diff --git a/TablePro/Core/Compare/SourceObjectIndexes.swift b/TablePro/Core/Compare/SourceObjectIndexes.swift new file mode 100644 index 0000000000..45f693090b --- /dev/null +++ b/TablePro/Core/Compare/SourceObjectIndexes.swift @@ -0,0 +1,93 @@ +// +// SourceObjectIndexes.swift +// TablePro +// + +import Foundation +import TableProPluginKit + +internal enum ObjectIndexRead: Sendable { + case read([PluginIndexInfo]) + case failed(String) +} + +internal enum SourceObjectIndexes { + internal static func tableType(of kind: CompareObjectKind) -> TableInfo.TableType? { + switch kind { + case .view: return .view + case .materializedView: return .materializedView + case .table, .procedure, .function, .trigger, .sequence: return nil + } + } + + internal static func areCarried(for kind: CompareObjectKind, by matrix: StructureObjectEditMatrix) -> Bool { + guard let tableType = tableType(of: kind) else { return false } + return StructureEditEligibility.allows(.addIndex, on: tableType, matrix: matrix) + && StructureEditEligibility.allows(.dropIndex, on: tableType, matrix: matrix) + } + + internal static func carriedKinds(by matrix: StructureObjectEditMatrix) -> Set { + Set(CompareObjectKind.allCases.filter { areCarried(for: $0, by: matrix) }) + } + + @MainActor + internal static func carriedKinds(on databaseType: DatabaseType) -> Set { + carriedKinds(by: PluginManager.shared.structureEditMatrix(for: databaseType)) + } + + internal static func definitions(_ indexes: [PluginIndexInfo]) -> [EditableIndexDefinition] { + indexes.map { EditableIndexDefinition.from(IndexInfo($0)) }.filter { !$0.isPrimary } + } + + internal static var notCarriedByTargetNote: String { + String(localized: "Its indexes are left out, because the target is not known to take indexes on this kind of object.") + } + + internal static var otherSchemaNote: String { + String( + localized: "Its indexes are left out, because the target's index statements would name a different schema than its definition." + ) + } +} + +internal enum SourceObjectIndexCopy: Equatable { + case none + case write([EditableIndexDefinition]) + case leaveOut(String) + case refuse(String) + + internal static func decide( + for read: RoutineSourceRead, + targetCarries: Bool, + indexSchema: String? + ) -> SourceObjectIndexCopy { + guard let indexes = read.indexes else { return .none } + switch indexes { + case .failed(let reason): + guard targetCarries else { return .leaveOut(SourceObjectIndexes.notCarriedByTargetNote) } + return .refuse(String(format: String(localized: "Its indexes could not be read: %@"), reason)) + case .read(let found): + let definitions = SourceObjectIndexes.definitions(found) + guard !definitions.isEmpty else { return .none } + guard targetCarries else { return .leaveOut(SourceObjectIndexes.notCarriedByTargetNote) } + guard let indexSchema, read.schema == indexSchema else { + return .leaveOut(SourceObjectIndexes.otherSchemaNote) + } + return .write(definitions) + } + } +} + +internal enum ConcurrentRefreshIndexRule { + internal static func isUsable(_ index: EditableIndexDefinition) -> Bool { + let predicate = index.whereClause?.trimmingCharacters(in: .whitespacesAndNewlines) ?? "" + return index.isUnique + && index.expressions.isEmpty + && predicate.isEmpty + && index.type == .btree + } + + internal static func allowsConcurrentRefresh(_ indexes: [EditableIndexDefinition]) -> Bool { + indexes.contains(where: isUsable) + } +} diff --git a/TablePro/Core/Compare/SourceObjectSyncBuilder.swift b/TablePro/Core/Compare/SourceObjectSyncBuilder.swift index 9a913db6fb..e839430c90 100644 --- a/TablePro/Core/Compare/SourceObjectSyncBuilder.swift +++ b/TablePro/Core/Compare/SourceObjectSyncBuilder.swift @@ -4,12 +4,15 @@ // // Statements for the objects whose definition is SQL text. // -// There is nothing to synthesise here: the source's own definition is the -// statement. What the builder decides is how to get from the target's current -// definition to that one, which for a view or a routine means dropping what is -// there and running the source's text. `CREATE OR REPLACE` is not written here, -// because the engines spell it differently and several do not accept it for a -// signature change. The one exception is a definition the source already wrote +// The source's own definition is the statement. What the builder decides is how +// to get from the target's current definition to that one, which for a view or +// a routine means dropping what is there and running the source's text. The one +// part it writes itself is a materialized view's indexes, where the engine's +// structure matrix says that kind takes them: the text does not carry them, so +// a view dropped and created again from it comes back without any. +// +// `CREATE OR REPLACE` is not written here, because the engines spell it +// differently and several do not accept it for a signature change. The one exception is a definition the source already wrote // as `CREATE OR REPLACE`, on a target whose driver says that replaces the // object in place: there the DROP is what would lose the object when the new // definition fails, so it is left out. @@ -24,13 +27,21 @@ internal struct SourceObjectSyncBuilder { private let targetDriver: any PluginDatabaseDriver private let targetDatabaseType: DatabaseType + internal let indexSchema: String? private let scriptText: SQLScriptText + private let changeBuilder: SchemaSyncScriptBuilder private let classifier = SyncSafetyClassifier() - internal init(targetDriver: any PluginDatabaseDriver, targetDatabaseType: DatabaseType) { + internal init( + targetDriver: any PluginDatabaseDriver, + targetDatabaseType: DatabaseType, + indexSchema: String? = nil + ) { self.targetDriver = targetDriver self.targetDatabaseType = targetDatabaseType + self.indexSchema = indexSchema self.scriptText = SQLScriptText(databaseType: targetDatabaseType) + self.changeBuilder = SchemaSyncScriptBuilder(targetDriver: targetDriver, targetDatabaseType: targetDatabaseType) } internal func build(for result: CompareObjectResult, action: TableSyncAction) throws -> [SyncStatement] { @@ -39,12 +50,18 @@ internal struct SourceObjectSyncBuilder { return [] case .create: try refuseWithoutACreateStatement(result) - return createStatements(for: result, isReplacement: false) + let indexes = try indexCreationStatements(for: result) + return createStatements(for: result, isReplacement: false) + indexes case .alter: + if result.definitionMatches, result.sourceIndexes != nil { + return try indexChangeStatements(for: result) + } try refuseWithoutACreateStatement(result) guard replacesInPlace(result.identity, with: result) else { + let indexes = try indexCreationStatements(for: result) return dropStatements(for: result, isReplacement: true) + createStatements(for: result, isReplacement: false) + + indexes } return createStatements(for: result, isReplacement: true) case .drop: @@ -52,6 +69,51 @@ internal struct SourceObjectSyncBuilder { } } + private func indexCreationStatements(for result: CompareObjectResult) throws -> [SyncStatement] { + let indexes = (result.sourceIndexes ?? []).filter { !$0.isPrimary } + guard !indexes.isEmpty else { return [] } + try refuseIndexesInAnotherSchema(result) + return try changeBuilder.changeStatements( + on: result.identity.name, + objectName: result.identity.displayName, + changes: indexes.map(SchemaChange.addIndex) + ) + } + + private func indexChangeStatements(for result: CompareObjectResult) throws -> [SyncStatement] { + guard !result.changes.isEmpty else { return [] } + try refuseIndexesInAnotherSchema(result) + let refreshHazard = classifier.concurrentRefreshHazard( + on: result.identity, from: result.targetIndexes ?? [], to: result.sourceIndexes ?? [] + ) + return try changeBuilder.changeStatements( + on: result.identity.name, + objectName: result.identity.displayName, + changes: result.changes + ) { change in + guard let refreshHazard, case .deleteIndex(let index) = change, + ConcurrentRefreshIndexRule.isUsable(index) + else { return [] } + return [refreshHazard] + } + } + + private func refuseIndexesInAnotherSchema(_ result: CompareObjectResult) throws { + guard let indexSchema, result.identity.schema == indexSchema else { + Self.logger.error( + "Refused index statements for \(result.identity.name, privacy: .private(mask: .hash)) outside the schema its definition creates it in" + ) + throw CompareSyncError.unsupportedOperation( + String( + format: String( + localized: "%@ cannot be scripted with its indexes, because the target's index statements would name a different schema than its definition." + ), + result.identity.displayName + ) + ) + } + } + private func refuseWithoutACreateStatement(_ result: CompareObjectResult) throws { let definition = result.sourceDefinition.joined(separator: "\n") guard SourceDefinitionDefect.of(definition: definition, sentAs: scriptText) != nil else { return } @@ -73,6 +135,7 @@ internal struct SourceObjectSyncBuilder { guard targetDriver.replacesDefinitionsInPlace, existing.kind == replacement.identity.kind, existing.kind != .materializedView, + replacement.sourceIndexes == nil, let first = scriptText.sendableStatements(replacement.sourceDefinition.joined(separator: "\n")).first else { return false } let leadingWords = first.split(whereSeparator: \.isWhitespace).prefix(3).map { $0.uppercased() } @@ -105,7 +168,7 @@ internal struct SourceObjectSyncBuilder { format: String(localized: "Drop %1$@ %2$@"), result.identity.kind.displayName.lowercased(), result.identity.displayName ) - let hazards = classifier.hazards(forDropping: result.identity, isReplacement: isReplacement) + let hazards = dropHazards(for: result, isReplacement: isReplacement) return scriptText.sendableStatements(sql).map { statement in SyncStatement( sql: statement, @@ -116,6 +179,21 @@ internal struct SourceObjectSyncBuilder { } } + private func dropHazards(for result: CompareObjectResult, isReplacement: Bool) -> [SyncHazard] { + let recreatedIndexes = isReplacement ? (result.sourceIndexes ?? []) : [] + let hazards = classifier.hazards( + forDropping: result.identity, + isReplacement: isReplacement, + recreatesIndexes: !recreatedIndexes.isEmpty + ) + guard isReplacement, let current = result.targetIndexes, + let refreshHazard = classifier.concurrentRefreshHazard( + on: result.identity, from: current, to: recreatedIndexes + ) + else { return hazards } + return hazards + [refreshHazard] + } + /// A routine and a trigger are not addressed by name alone on every engine. PostgreSQL needs an /// overloaded routine's argument list and spells a trigger drop `DROP TRIGGER name ON table`, /// while MySQL rejects the argument list and takes no `ON`. Only the driver knows which, so the diff --git a/TablePro/Core/Compare/StructureChangeGuard.swift b/TablePro/Core/Compare/StructureChangeGuard.swift index 59c548f67a..5003c40126 100644 --- a/TablePro/Core/Compare/StructureChangeGuard.swift +++ b/TablePro/Core/Compare/StructureChangeGuard.swift @@ -34,6 +34,8 @@ internal struct StructureGenerationInput: Hashable, Sendable { internal let changes: [SchemaChange] internal let sourceSnapshot: TableStructureSnapshot? internal let sourceDefinition: [String] + internal let sourceIndexes: [EditableIndexDefinition]? + internal let definitionMatches: Bool } internal enum StructureChangeGuard { @@ -50,11 +52,13 @@ internal enum StructureChangeGuard { qualifiedName: result.identity.qualifiedName, action: action, status: result.status, - changes: result.identity.kind == .table ? result.changes.map { $0.withoutIdentity() } : [], + changes: result.changes.map { $0.withoutIdentity() }, sourceSnapshot: result.identity.kind == .table ? sourceSnapshots[result.identity.qualifiedName]?.withoutIdentity() : nil, - sourceDefinition: result.identity.kind == .table ? [] : result.sourceDefinition + sourceDefinition: result.identity.kind == .table ? [] : result.sourceDefinition, + sourceIndexes: result.sourceIndexes?.map { $0.withoutIdentity() }, + definitionMatches: result.definitionMatches ) } return inputs diff --git a/TablePro/Core/Compare/StructureDiffEngine+Members.swift b/TablePro/Core/Compare/StructureDiffEngine+Members.swift index 3b34d0b3bb..845558e0fa 100644 --- a/TablePro/Core/Compare/StructureDiffEngine+Members.swift +++ b/TablePro/Core/Compare/StructureDiffEngine+Members.swift @@ -55,8 +55,15 @@ internal extension StructureDiffEngine { source: TableStructureSnapshot, target: TableStructureSnapshot ) -> MemberOutcome { - let sourceIndexes = source.indexes.filter { !$0.isPrimary } - let targetIndexes = target.indexes.filter { !$0.isPrimary } + indexChanges(source: source.indexes, target: target.indexes) + } + + func indexChanges( + source: [EditableIndexDefinition], + target: [EditableIndexDefinition] + ) -> MemberOutcome { + let sourceIndexes = source.filter { !$0.isPrimary } + let targetIndexes = target.filter { !$0.isPrimary } var remaining = targetIndexes var changes: [SchemaChange] = [] diff --git a/TablePro/Core/Compare/SyncHazard.swift b/TablePro/Core/Compare/SyncHazard.swift index b05a2dd5e2..675c9054be 100644 --- a/TablePro/Core/Compare/SyncHazard.swift +++ b/TablePro/Core/Compare/SyncHazard.swift @@ -17,6 +17,7 @@ internal enum SyncHazardKind: String, Codable, Hashable, Sendable { case primaryKeyChange case engineOrStorageChange case notSupportedByTarget + case concurrentRefresh } internal enum SyncHazardSeverity: Int, Codable, Hashable, Sendable, Comparable { @@ -60,6 +61,8 @@ internal extension SyncHazardKind { return String(localized: "Storage engine change") case .notSupportedByTarget: return String(localized: "Not supported by the target") + case .concurrentRefresh: + return String(localized: "Concurrent refresh") } } } diff --git a/TablePro/Core/Compare/SyncSafetyClassifier.swift b/TablePro/Core/Compare/SyncSafetyClassifier.swift index 52a054796d..58033b80e7 100644 --- a/TablePro/Core/Compare/SyncSafetyClassifier.swift +++ b/TablePro/Core/Compare/SyncSafetyClassifier.swift @@ -90,19 +90,18 @@ internal struct SyncSafetyClassifier { /// A view or a routine holds no rows, so dropping one is recoverable from the source and only /// warns. A materialized view does hold rows, so it is refused like a table. Either way a drop /// can break something that depends on it, which is what the second hazard says. - internal func hazards(forDropping identity: CompareObjectIdentity, isReplacement: Bool) -> [SyncHazard] { + internal func hazards( + forDropping identity: CompareObjectIdentity, + isReplacement: Bool, + recreatesIndexes: Bool = false + ) -> [SyncHazard] { var hazards: [SyncHazard] = [] if identity.kind == .materializedView { hazards.append(SyncHazard( kind: .dataLoss, severity: .refusedByDefault, - explanation: String( - format: String( - localized: "Dropping materialized view %@ discards its stored rows, which have to be rebuilt." - ), - identity.displayName - ) + explanation: materializedViewDropExplanation(identity, recreatesIndexes: recreatesIndexes) )) } @@ -131,6 +130,47 @@ internal struct SyncSafetyClassifier { return hazards } + internal func concurrentRefreshHazard( + on identity: CompareObjectIdentity, + from current: [EditableIndexDefinition], + to resulting: [EditableIndexDefinition] + ) -> SyncHazard? { + guard identity.kind == .materializedView, + ConcurrentRefreshIndexRule.allowsConcurrentRefresh(current), + !ConcurrentRefreshIndexRule.allowsConcurrentRefresh(resulting) + else { return nil } + return SyncHazard( + kind: .concurrentRefresh, + severity: .warning, + explanation: String( + format: String( + localized: "%@ is left with no unique index a concurrent refresh can use, so REFRESH MATERIALIZED VIEW CONCURRENTLY fails on it." + ), + identity.displayName + ) + ) + } + + private func materializedViewDropExplanation(_ identity: CompareObjectIdentity, recreatesIndexes: Bool) -> String { + guard recreatesIndexes else { + return String( + format: String( + localized: "Dropping materialized view %@ discards its stored rows along with its indexes, comments and privileges." + ), + identity.displayName + ) + } + return String( + format: String( + localized: """ + Dropping materialized view %@ discards its stored rows, comments and privileges. \ + The rows are computed again and the source's indexes are created on it. + """ + ), + identity.displayName + ) + } + private func modifyColumnHazards( old: EditableColumnDefinition, new: EditableColumnDefinition, diff --git a/TablePro/Core/Compare/TableDefinitionRenderer.swift b/TablePro/Core/Compare/TableDefinitionRenderer.swift index e063121c0a..fbbd3206ef 100644 --- a/TablePro/Core/Compare/TableDefinitionRenderer.swift +++ b/TablePro/Core/Compare/TableDefinitionRenderer.swift @@ -22,18 +22,7 @@ internal enum TableDefinitionRenderer { result.append(" PRIMARY KEY (\(primaryKey.joined(separator: ", ")))") } - for index in snapshot.indexes.filter({ !$0.isPrimary }) - .sorted(by: { $0.name.localizedStandardCompare($1.name) == .orderedAscending }) { - let unique = index.isUnique ? "UNIQUE " : "" - var line = " \(unique)INDEX \(index.name) (\(index.columns.joined(separator: ", "))) USING \(index.type.rawValue)" - if !index.includedColumns.isEmpty { - line += " INCLUDE (\(index.includedColumns.joined(separator: ", ")))" - } - if let whereClause = index.whereClause, !whereClause.isEmpty { - line += " WHERE \(whereClause)" - } - result.append(line) - } + result += indexLines(for: snapshot.indexes).map { " " + $0 } for foreignKey in snapshot.foreignKeys .sorted(by: { $0.name.localizedStandardCompare($1.name) == .orderedAscending }) { @@ -54,6 +43,24 @@ internal enum TableDefinitionRenderer { return result } + internal static func indexLines(for indexes: [EditableIndexDefinition]) -> [String] { + indexes.filter { !$0.isPrimary } + .sorted { $0.name.localizedStandardCompare($1.name) == .orderedAscending } + .map(indexLine) + } + + private static func indexLine(_ index: EditableIndexDefinition) -> String { + let unique = index.isUnique ? "UNIQUE " : "" + var line = "\(unique)INDEX \(index.name) (\(index.columns.joined(separator: ", "))) USING \(index.type.rawValue)" + if !index.includedColumns.isEmpty { + line += " INCLUDE (\(index.includedColumns.joined(separator: ", ")))" + } + if let whereClause = index.whereClause, !whereClause.isEmpty { + line += " WHERE \(whereClause)" + } + return line + } + private static func columnAttributes(_ column: EditableColumnDefinition) -> String { var parts: [String] = [] if column.unsigned { parts.append("UNSIGNED") } diff --git a/TablePro/Core/ObjectCopy/ObjectCopyPlan.swift b/TablePro/Core/ObjectCopy/ObjectCopyPlan.swift index 43d8460d91..aaa5b42c99 100644 --- a/TablePro/Core/ObjectCopy/ObjectCopyPlan.swift +++ b/TablePro/Core/ObjectCopy/ObjectCopyPlan.swift @@ -120,6 +120,19 @@ internal struct ObjectCopyDefinitionStep: Identifiable, Sendable { internal let selection: ObjectCopySelection internal let dropStatements: [SyncStatement] internal let createStatements: [SyncStatement] + internal let note: String? + + internal init( + selection: ObjectCopySelection, + dropStatements: [SyncStatement], + createStatements: [SyncStatement], + note: String? = nil + ) { + self.selection = selection + self.dropStatements = dropStatements + self.createStatements = createStatements + self.note = note + } internal var id: String { selection.id } @@ -152,6 +165,13 @@ internal struct ObjectCopySkip: Identifiable, Sendable { internal var id: String { selection.id } } +internal struct ObjectCopyPartialNote: Identifiable, Sendable { + internal let selection: ObjectCopySelection + internal let text: String + + internal var id: String { selection.id } +} + internal struct ObjectCopyPlan: Sendable { internal let request: ObjectCopyRequest internal let createsDatabase: Bool @@ -213,6 +233,13 @@ internal struct ObjectCopyPlan: Sendable { return warnings } + internal var partialNotes: [ObjectCopyPartialNote] { + tableSteps.compactMap { step in step.note.map { ObjectCopyPartialNote(selection: step.selection, text: $0) } } + + definitionSteps.compactMap { step in + step.note.map { ObjectCopyPartialNote(selection: step.selection, text: $0) } + } + } + /// Every type, default and index the crossing changed, worst first. internal var conversionNotes: [CrossEngineConversionNote] { tableSteps.flatMap(\.conversionNotes).orderedForReview diff --git a/TablePro/Core/ObjectCopy/ObjectCopyPlanner+SourceDefinitions.swift b/TablePro/Core/ObjectCopy/ObjectCopyPlanner+SourceDefinitions.swift index 5196fcf391..565d42065d 100644 --- a/TablePro/Core/ObjectCopy/ObjectCopyPlanner+SourceDefinitions.swift +++ b/TablePro/Core/ObjectCopy/ObjectCopyPlanner+SourceDefinitions.swift @@ -11,7 +11,52 @@ internal enum ObjectCopyDefinitionOutcome: Equatable, Sendable { case skipped(String) } +internal struct ObjectCopyDefinitionInput: Sendable { + internal let id: String + internal let identity: CompareObjectIdentity + internal let definition: String + internal let read: RoutineSourceRead + internal let targetCarriesIndexes: Bool + internal let drop: CompareObjectResult? +} + +internal enum ObjectCopyDefinitionBuild: Sendable { + case built(drop: [SyncStatement], create: [SyncStatement], note: String?) + case refused(String) +} + internal extension ObjectCopyPlanner { + nonisolated static func definitionBuild( + for input: ObjectCopyDefinitionInput, + using builder: SourceObjectSyncBuilder + ) throws -> ObjectCopyDefinitionBuild { + var sourceIndexes: [EditableIndexDefinition]? + var note: String? + switch SourceObjectIndexCopy.decide( + for: input.read, targetCarries: input.targetCarriesIndexes, indexSchema: builder.indexSchema + ) { + case .none: + break + case .write(let indexes): + sourceIndexes = indexes + case .leaveOut(let text): + note = text + case .refuse(let reason): + return .refused(reason) + } + let create = CompareObjectResult( + identity: input.identity, + status: .onlyInSource, + sourceDefinition: [input.definition], + sourceIndexes: sourceIndexes + ) + let drop = try input.drop.map { existing -> [SyncStatement] in + guard !builder.replacesInPlace(existing.identity, with: create) else { return [] } + return try builder.build(for: existing, action: .drop) + } ?? [] + return .built(drop: drop, create: try builder.build(for: create, action: .create), note: note) + } + nonisolated static func definitionOutcome( _ read: RoutineSourceRead?, sentAs scriptText: SQLScriptText @@ -30,7 +75,7 @@ internal extension ObjectCopyPlanner { nonisolated static func sourceDefinitionReads( for selections: [ObjectCopySelection], - views: [PluginTableInfo], + views: [TableStructureRead], triggerTables: [String], schema: String?, endpointName: String, diff --git a/TablePro/Core/ObjectCopy/ObjectCopyPlanner.swift b/TablePro/Core/ObjectCopy/ObjectCopyPlanner.swift index 5d6afd98db..8013cad522 100644 --- a/TablePro/Core/ObjectCopy/ObjectCopyPlanner.swift +++ b/TablePro/Core/ObjectCopy/ObjectCopyPlanner.swift @@ -760,10 +760,16 @@ internal struct ObjectCopyPlanner { connection: connection ) let targetScriptText = SQLScriptText(databaseType: request.target.databaseType) - var pending: [(selection: ObjectCopySelection, definition: String, target: ObjectCopySelection?)] = [] + var pending: [( + selection: ObjectCopySelection, definition: String, read: RoutineSourceRead, target: ObjectCopySelection? + )] = [] for selection in Self.orderedByKind(selections) { + guard let read = definitionReads[selection.id] else { + skipped.append(ObjectCopySkip(selection: selection, reason: Self.noDefinition)) + continue + } let definition: String - switch Self.definitionOutcome(definitionReads[selection.id], sentAs: targetScriptText) { + switch Self.definitionOutcome(read, sentAs: targetScriptText) { case .skipped(let reason): skipped.append(ObjectCopySkip(selection: selection, reason: reason)) continue @@ -777,23 +783,23 @@ internal struct ObjectCopyPlanner { skipped.append(ObjectCopySkip(selection: selection, reason: Self.alreadyThere)) continue } - pending.append((selection, definition, existing)) + pending.append((selection, definition, read, existing)) } guard !pending.isEmpty else { return [] } + let targetIndexedKinds = SourceObjectIndexes.carriedKinds(on: request.target.databaseType) let inputs = pending.map { item in - ( + ObjectCopyDefinitionInput( id: item.selection.id, - create: CompareObjectResult( - identity: CompareObjectIdentity( - kind: item.selection.kind, - schema: targetSchema ?? item.selection.schema, - name: item.selection.name, - signature: item.selection.signature - ), - status: .onlyInSource, - sourceDefinition: [item.definition] + identity: CompareObjectIdentity( + kind: item.selection.kind, + schema: targetSchema ?? item.selection.schema, + name: item.selection.name, + signature: item.selection.signature ), + definition: item.definition, + read: item.read, + targetCarriesIndexes: targetIndexedKinds.contains(item.selection.kind), /// Dropped as the kind the target actually holds. A source view over a target /// materialized view emitted `DROP VIEW`, which those engines refuse. drop: item.target.map { target in @@ -818,30 +824,33 @@ internal struct ObjectCopyPlanner { guard let plugin = CompareMetadataService.pluginDriver(from: driver) else { throw ObjectCopyError.refused(Self.noTargetDriver) } - let builder = SourceObjectSyncBuilder(targetDriver: plugin, targetDatabaseType: driver.connection.type) - var statements: [String: (drop: [SyncStatement], create: [SyncStatement])] = [:] + let builder = SourceObjectSyncBuilder( + targetDriver: plugin, + targetDatabaseType: driver.connection.type, + indexSchema: plugin.currentSchema + ) + var builds: [String: ObjectCopyDefinitionBuild] = [:] for input in inputs { - let drop = try input.drop.map { existing -> [SyncStatement] in - guard !builder.replacesInPlace(existing.identity, with: input.create) else { return [] } - return try builder.build(for: existing, action: .drop) - } ?? [] - let create = try builder.build(for: input.create, action: .create) - statements[input.id] = (drop, create) + builds[input.id] = try Self.definitionBuild(for: input, using: builder) } - return statements + return builds } var steps: [ObjectCopyDefinitionStep] = [] for item in pending { - guard let statements = built[item.selection.id], !statements.create.isEmpty else { + switch built[item.selection.id] { + case .refused(let reason)?: + skipped.append(ObjectCopySkip(selection: item.selection, reason: reason)) + case .built(let drop, let create, let note)? where !create.isEmpty: + steps.append(ObjectCopyDefinitionStep( + selection: item.selection, + dropStatements: drop, + createStatements: create, + note: note + )) + case .built?, nil: skipped.append(ObjectCopySkip(selection: item.selection, reason: Self.noDefinition)) - continue } - steps.append(ObjectCopyDefinitionStep( - selection: item.selection, - dropStatements: statements.drop, - createStatements: statements.create - )) } return steps } @@ -852,10 +861,10 @@ internal struct ObjectCopyPlanner { sourceReads: [TableStructureRead], connection: DatabaseConnection ) async throws -> [String: RoutineSourceRead] { - let views = sourceReads.map(\.table).filter { info in + let views = sourceReads.filter { read in selections.contains { selection in (selection.kind == .view || selection.kind == .materializedView) - && selection.name.lowercased() == info.name.lowercased() + && selection.name.lowercased() == read.table.name.lowercased() } } let triggerTables = Set(selections.filter { $0.kind == .trigger }.compactMap(\.owner)) diff --git a/TablePro/Resources/Localizable.xcstrings b/TablePro/Resources/Localizable.xcstrings index ffce3b31ec..a203816316 100644 --- a/TablePro/Resources/Localizable.xcstrings +++ b/TablePro/Resources/Localizable.xcstrings @@ -6359,6 +6359,9 @@ }, "%@ are invalid, so queries skip them and exports leave them out. Drop them, or rebuild them with REINDEX." : { + }, + "%@ cannot be scripted with its indexes, because the target's index statements would name a different schema than its definition." : { + }, "%@ cannot be scripted, because its definition is not a statement that recreates it." : { @@ -6368,6 +6371,9 @@ }, "%@ is invalid, so queries skip it and exports leave it out. Drop it, or rebuild it with REINDEX." : { + }, + "%@ is left with no unique index a concurrent refresh can use, so REFRESH MATERIALIZED VIEW CONCURRENTLY fails on it." : { + }, "%d added" : { "localizations" : { @@ -37200,6 +37206,9 @@ } } } + }, + "Concurrent refresh" : { + }, "Concurrent refresh needs a valid unique index on the view's columns, with no WHERE clause and no expressions." : { @@ -56261,39 +56270,11 @@ } } }, - "Dropping materialized view %@ discards its stored rows, which have to be rebuilt." : { - "localizations" : { - "ko" : { - "stringUnit" : { - "state" : "translated", - "value" : "구체화 뷰 %@을(를) 삭제하면 저장된 행이 버려지며, 다시 만들어야 합니다." - } - }, - "tr" : { - "stringUnit" : { - "state" : "translated", - "value" : "%@ materyalize görünümünü silmek saklanan satırlarını atar; bunların yeniden oluşturulması gerekir." - } - }, - "vi" : { - "stringUnit" : { - "state" : "translated", - "value" : "Xóa materialized view %@ sẽ bỏ các dòng đã lưu của nó, và chúng phải được dựng lại." - } - }, - "zh-Hans" : { - "stringUnit" : { - "state" : "translated", - "value" : "删除物化视图 %@ 会丢弃其存储的行,这些行必须重建。" - } - }, - "zh-Hant" : { - "stringUnit" : { - "state" : "translated", - "value" : "刪除具體化檢視表 %@ 會捨棄其儲存的列,這些列必須重建。" - } - } - } + "Dropping materialized view %@ discards its stored rows along with its indexes, comments and privileges." : { + + }, + "Dropping materialized view %@ discards its stored rows, comments and privileges. The rows are computed again and the source's indexes are created on it." : { + }, "Dropping table %@ permanently removes the table and all of its rows." : { "localizations" : { @@ -74876,6 +74857,9 @@ } } } + }, + "Its indexes are left out, because the target is not known to take indexes on this kind of object." : { + }, "history entries" : { "extractionState" : "stale", @@ -75641,6 +75625,9 @@ } } } + }, + "Its indexes are left out, because the target's index statements would name a different schema than its definition." : { + }, "hyphen" : { "extractionState" : "manual", @@ -81536,6 +81523,9 @@ } } } + }, + "Its indexes could not be read: %@" : { + }, "is empty" : { "localizations" : { @@ -161565,6 +161555,9 @@ }, "The batch is committing and cannot be stopped." : { + }, + "The source's indexes could not be read: %@" : { + }, "The statement before it stays applied." : { @@ -162097,6 +162090,9 @@ }, "The target returned an empty definition." : { + }, + "The target's indexes could not be read: %@" : { + }, "The target's object is dropped and built again from the source." : { diff --git a/TablePro/Views/Compare/CompareDetailView.swift b/TablePro/Views/Compare/CompareDetailView.swift index e3accc257d..2357e9fb06 100644 --- a/TablePro/Views/Compare/CompareDetailView.swift +++ b/TablePro/Views/Compare/CompareDetailView.swift @@ -95,6 +95,14 @@ internal struct CompareDefinitionsPane: View { targetLines: result.targetDefinition ) + if result.showsIndexes { + StructureDefinitionDiffView( + title: String(localized: "Indexes"), + sourceLines: result.sourceIndexLines, + targetLines: result.targetIndexLines + ) + } + if !result.changes.isEmpty { changesSection(result.changes) } diff --git a/TablePro/Views/Compare/CompareResultGrouping.swift b/TablePro/Views/Compare/CompareResultGrouping.swift index 9ae59cce02..05302d0f7b 100644 --- a/TablePro/Views/Compare/CompareResultGrouping.swift +++ b/TablePro/Views/Compare/CompareResultGrouping.swift @@ -164,11 +164,12 @@ internal enum CompareResultGrouping { ) } - /// A source-defined object has no parsed change list, only a body of SQL, so it deliberately - /// summarises to nothing rather than to a count of zero. + /// Most source-defined objects have no parsed change list, only a body of SQL, so they summarise + /// to nothing rather than to a count of zero. A materialized view's index changes are counted + /// like a table's. private static func changeSummary(for result: CompareObjectResult) -> String { if let error = result.comparisonError { return error } - guard result.identity.kind == .table, !result.changes.isEmpty else { return "" } + guard !result.changes.isEmpty else { return "" } return String(format: String(localized: "%d changes"), result.changes.count) } } diff --git a/TablePro/Views/ObjectCopy/CopyObjectsReviewView.swift b/TablePro/Views/ObjectCopy/CopyObjectsReviewView.swift index 26a8b13dbf..46612f0fa1 100644 --- a/TablePro/Views/ObjectCopy/CopyObjectsReviewView.swift +++ b/TablePro/Views/ObjectCopy/CopyObjectsReviewView.swift @@ -155,13 +155,13 @@ internal struct CopyObjectsReviewView: View { @ViewBuilder private func notes(_ plan: ObjectCopyPlan) -> some View { - let noted = plan.tableSteps.compactMap { step in step.note.map { (step.id, step.selection, $0) } } + let noted = plan.partialNotes if !noted.isEmpty { VStack(alignment: .leading, spacing: 6) { Text("Partly copied") .font(.subheadline.weight(.medium)) - ForEach(noted, id: \.0) { _, selection, note in - Text(verbatim: "\(selection.displayName): \(note)") + ForEach(noted) { note in + Text(verbatim: "\(note.selection.displayName): \(note.text)") .font(.callout) .foregroundStyle(.secondary) .fixedSize(horizontal: false, vertical: true) diff --git a/TableProTests/Core/Compare/CompareMetadataReadPlanTests.swift b/TableProTests/Core/Compare/CompareMetadataReadPlanTests.swift index 53962716e1..49cea46052 100644 --- a/TableProTests/Core/Compare/CompareMetadataReadPlanTests.swift +++ b/TableProTests/Core/Compare/CompareMetadataReadPlanTests.swift @@ -27,7 +27,7 @@ final class CompareMetadataReadPlanTests: XCTestCase { let reads = try await CompareMetadataService.read( tables: tables(200), schema: "public", profile: .structure, - narrowed: false, databaseType: .postgresql, using: driver + narrowed: false, databaseType: .postgresql, indexedKinds: [], using: driver ) XCTAssertEqual(reads.count, 200) @@ -43,7 +43,7 @@ final class CompareMetadataReadPlanTests: XCTestCase { _ = try await CompareMetadataService.read( tables: tables(50), schema: "public", profile: .structure, - narrowed: false, databaseType: .postgresql, using: driver + narrowed: false, databaseType: .postgresql, indexedKinds: [], using: driver ) XCTAssertEqual(driver.count(of: "fetchColumns"), 0) @@ -57,7 +57,7 @@ final class CompareMetadataReadPlanTests: XCTestCase { let reads = try await CompareMetadataService.read( tables: tables(3), schema: "public", profile: .structure, - narrowed: false, databaseType: .postgresql, using: driver + narrowed: false, databaseType: .postgresql, indexedKinds: [], using: driver ) let snapshot = try XCTUnwrap(reads.first?.snapshot) @@ -73,7 +73,7 @@ final class CompareMetadataReadPlanTests: XCTestCase { let reads = try await CompareMetadataService.read( tables: tables(6), schema: "public", profile: .structure, - narrowed: false, databaseType: .postgresql, using: driver + narrowed: false, databaseType: .postgresql, indexedKinds: [], using: driver ) XCTAssertEqual(reads.count, 6) @@ -91,7 +91,7 @@ final class CompareMetadataReadPlanTests: XCTestCase { _ = try await CompareMetadataService.read( tables: tables(2), schema: "public", profile: .structure, - narrowed: true, databaseType: .postgresql, using: driver + narrowed: true, databaseType: .postgresql, indexedKinds: [], using: driver ) XCTAssertEqual(driver.count(of: "fetchAllColumns"), 0) @@ -105,7 +105,7 @@ final class CompareMetadataReadPlanTests: XCTestCase { _ = try await CompareMetadataService.read( tables: tables(10), schema: "public", profile: .data, - narrowed: false, databaseType: .postgresql, using: driver + narrowed: false, databaseType: .postgresql, indexedKinds: [], using: driver ) XCTAssertEqual(driver.count(of: "fetchAllColumns"), 1) @@ -119,7 +119,7 @@ final class CompareMetadataReadPlanTests: XCTestCase { _ = try await CompareMetadataService.read( tables: tables(4), schema: "public", profile: .data, - narrowed: false, databaseType: .postgresql, using: driver + narrowed: false, databaseType: .postgresql, indexedKinds: [], using: driver ) XCTAssertEqual(driver.count(of: "fetchColumns"), 4) @@ -138,7 +138,7 @@ final class CompareMetadataReadPlanTests: XCTestCase { let reads = try await CompareMetadataService.read( tables: tables(3), schema: "public", profile: .structure, - narrowed: false, databaseType: .postgresql, using: driver + narrowed: false, databaseType: .postgresql, indexedKinds: [], using: driver ) XCTAssertEqual(driver.count(of: "fetchColumns"), 3) @@ -154,7 +154,7 @@ final class CompareMetadataReadPlanTests: XCTestCase { let reads = try await CompareMetadataService.read( tables: tables(3), schema: "public", profile: .structure, - narrowed: false, databaseType: .postgresql, using: driver + narrowed: false, databaseType: .postgresql, indexedKinds: [], using: driver ) XCTAssertEqual(reads.filter { $0.failure != nil }.map(\.table.name), ["t1"]) @@ -167,7 +167,7 @@ final class CompareMetadataReadPlanTests: XCTestCase { let reads = try await CompareMetadataService.read( tables: tables(3), schema: "public", profile: .structure, - narrowed: false, databaseType: .postgresql, using: driver + narrowed: false, databaseType: .postgresql, indexedKinds: [], using: driver ) XCTAssertEqual(reads.filter { $0.failure != nil }.map(\.table.name), ["t2"]) @@ -182,7 +182,7 @@ final class CompareMetadataReadPlanTests: XCTestCase { let reads = try await CompareMetadataService.read( tables: [PluginTableInfo(name: "v0", type: "VIEW", schema: "public", comment: nil)], schema: "public", profile: .structure, - narrowed: false, databaseType: .postgresql, using: driver + narrowed: false, databaseType: .postgresql, indexedKinds: [], using: driver ) XCTAssertNil(reads.first?.failure) @@ -195,7 +195,7 @@ final class CompareMetadataReadPlanTests: XCTestCase { let reads = try await CompareMetadataService.read( tables: tables(3), schema: "public", profile: .structure, - narrowed: false, databaseType: .postgresql, using: driver + narrowed: false, databaseType: .postgresql, indexedKinds: [], using: driver ) XCTAssertEqual(reads.count, 3) @@ -210,12 +210,115 @@ final class CompareMetadataReadPlanTests: XCTestCase { let reads = try await CompareMetadataService.read( tables: tables(2), schema: "public", profile: .structure, - narrowed: false, databaseType: .postgresql, using: driver + narrowed: false, databaseType: .postgresql, indexedKinds: [], using: driver ) XCTAssertEqual(driver.count(of: "fetchColumns"), 0, "the folded name matched, so nothing was re-read") XCTAssertEqual(reads.compactMap(\.snapshot).count, 2) } + + // MARK: - Materialized view indexes + + private let materializedView = PluginTableInfo( + name: "mv", type: "MATERIALIZED VIEW", schema: "public", comment: nil + ) + private let view = PluginTableInfo(name: "v0", type: "VIEW", schema: "public", comment: nil) + private let uniqueIndex = PluginIndexInfo(name: "mv_id_idx", columns: ["id"], isUnique: true) + + private func indexNames(_ read: TableStructureRead?) -> [String]? { + guard case .read(let indexes)? = read?.objectIndexes else { return nil } + return indexes.map(\.name) + } + + func testACarriedMaterializedViewReadsItsIndexes() async throws { + let driver = CountingMetadataDriver(bulk: false) + driver.objectIndexes = ["mv": [uniqueIndex]] + + let reads = try await CompareMetadataService.read( + tables: [materializedView], schema: "public", profile: .structure, + narrowed: false, databaseType: .postgresql, indexedKinds: [.materializedView], using: driver + ) + + XCTAssertEqual(indexNames(reads.first), ["mv_id_idx"]) + XCTAssertEqual(driver.count(of: "fetchIndexes"), 1) + } + + /// A failed read is not an answer of "no indexes": a script written from one would drop the + /// view and create it again without the unique index its concurrent refresh needs. + func testACarriedMaterializedViewWhoseIndexReadFailsKeepsTheFailure() async throws { + let driver = CountingMetadataDriver(bulk: false) + driver.failingIndexTable = "mv" + + let reads = try await CompareMetadataService.read( + tables: [materializedView], schema: "public", profile: .structure, + narrowed: false, databaseType: .postgresql, indexedKinds: [.materializedView], using: driver + ) + + let read = try XCTUnwrap(reads.first) + guard case .failed(let reason)? = read.objectIndexes else { + return XCTFail("a refused index read must be reported, got \(String(describing: read.objectIndexes))") + } + XCTAssertFalse(reason.isEmpty) + XCTAssertNil(read.failure, "the failure belongs to the indexes, so the definition is still read") + } + + func testAViewIsNotAskedForIndexesWhereItsKindTakesNone() async throws { + let driver = CountingMetadataDriver(bulk: false) + driver.failingIndexTable = "v0" + + let reads = try await CompareMetadataService.read( + tables: [view, materializedView], schema: "public", profile: .structure, + narrowed: false, databaseType: .postgresql, indexedKinds: [.materializedView], using: driver + ) + + XCTAssertEqual(driver.count(of: "fetchIndexes"), 1, "only the materialized view is asked") + XCTAssertNil(reads.first { $0.table.name == "v0" }?.objectIndexes) + XCTAssertNil(reads.first { $0.table.name == "v0" }?.failure) + } + + /// Redshift and Snowflake answer with sort, distribution or clustering keys, which no + /// `CREATE INDEX` can write back, so an engine whose matrix takes no index on the kind is not + /// asked at all. + func testAnEngineWhoseMaterializedViewsTakeNoIndexIsNeverAsked() async throws { + let driver = CountingMetadataDriver(bulk: false) + driver.objectIndexes = ["mv": [uniqueIndex]] + + let reads = try await CompareMetadataService.read( + tables: [materializedView], schema: "public", profile: .structure, + narrowed: false, databaseType: .postgresql, indexedKinds: [], using: driver + ) + + XCTAssertEqual(driver.count(of: "fetchIndexes"), 0) + XCTAssertNil(reads.first?.objectIndexes) + XCTAssertEqual(reads.first?.indexes.count, 0) + } + + func testTheWholeSchemaIndexReadReachesACarriedMaterializedView() async throws { + let driver = CountingMetadataDriver(bulk: true) + driver.objectIndexes = ["mv": [uniqueIndex]] + + let reads = try await CompareMetadataService.read( + tables: [materializedView], schema: "public", profile: .structure, + narrowed: false, databaseType: .postgresql, indexedKinds: [.materializedView], using: driver + ) + + XCTAssertEqual(indexNames(reads.first), ["mv_id_idx"]) + XCTAssertEqual(driver.count(of: "fetchIndexes"), 0) + XCTAssertEqual(driver.count(of: "fetchAllIndexes"), 1) + } + + func testADataComparisonReadsNoMaterializedViewIndexes() async throws { + let driver = CountingMetadataDriver(bulk: false) + driver.objectIndexes = ["mv": [uniqueIndex]] + + let reads = try await CompareMetadataService.read( + tables: [materializedView], schema: "public", profile: .data, + narrowed: false, databaseType: .postgresql, indexedKinds: [.materializedView], using: driver + ) + + XCTAssertEqual(driver.count(of: "fetchIndexes"), 0) + XCTAssertNil(reads.first?.objectIndexes) + } } private final class CountingMetadataDriver: PluginDatabaseDriver, @unchecked Sendable { @@ -228,6 +331,7 @@ private final class CountingMetadataDriver: PluginDatabaseDriver, @unchecked Sen var failingIndexTable: String? var failingForeignKeyTable: String? var uppercasesBulkKeys = false + var objectIndexes: [String: [PluginIndexInfo]] = [:] init(bulk: Bool) { self.bulk = bulk @@ -275,6 +379,7 @@ private final class CountingMetadataDriver: PluginDatabaseDriver, @unchecked Sen func fetchAllIndexes(schema: String?) async throws -> [String: [PluginIndexInfo]] { record("fetchAllIndexes") return Dictionary(uniqueKeysWithValues: knownTables.map { (key($0), indexes(for: $0)) }) + .merging(objectIndexes) { table, _ in table } } func fetchAllForeignKeys(schema: String?) async throws -> [String: [PluginForeignKeyInfo]] { @@ -302,7 +407,7 @@ private final class CountingMetadataDriver: PluginDatabaseDriver, @unchecked Sen func fetchIndexes(table: String, schema: String?) async throws -> [PluginIndexInfo] { record("fetchIndexes") if table == failingIndexTable { throw CocoaError(.fileReadNoPermission) } - return indexes(for: table) + return objectIndexes[table] ?? indexes(for: table) } func fetchForeignKeys(table: String, schema: String?) async throws -> [PluginForeignKeyInfo] { diff --git a/TableProTests/Core/Compare/CompareSourceDefinitionReadTests.swift b/TableProTests/Core/Compare/CompareSourceDefinitionReadTests.swift index f11a3fd738..cfcbf278fd 100644 --- a/TableProTests/Core/Compare/CompareSourceDefinitionReadTests.swift +++ b/TableProTests/Core/Compare/CompareSourceDefinitionReadTests.swift @@ -24,16 +24,24 @@ final class CompareSourceDefinitionReadTests: XCTestCase { // MARK: - Views + private func object( + _ name: String, + type: String = "VIEW", + indexes: ObjectIndexRead? = nil + ) -> TableStructureRead { + TableStructureRead( + table: PluginTableInfo(name: name, type: type, schema: "shop", comment: nil), + columns: [], indexes: [], foreignKeys: [], metadata: nil, failure: nil, objectIndexes: indexes + ) + } + func testAViewTheDriverRefusesCarriesTheDriversReason() async throws { let driver = SourceDefinitionStubDriver() let refusal = PluginObjectSourceError.insufficientPrivilege("recent") driver.viewDefinitions = ["recent": .failure(refusal), "totals": .failure(refusal)] let reads = try await CompareMetadataService.readViewDefinitions( - [ - PluginTableInfo(name: "recent", type: "VIEW", schema: "shop", comment: nil), - PluginTableInfo(name: "totals", type: "MATERIALIZED VIEW", schema: "shop", comment: nil) - ], + [object("recent"), object("totals", type: "MATERIALIZED VIEW")], schema: "shop", using: driver ) @@ -48,7 +56,7 @@ final class CompareSourceDefinitionReadTests: XCTestCase { driver.viewDefinitions = ["recent": .success("CREATE VIEW recent AS SELECT 1")] let reads = try await CompareMetadataService.readViewDefinitions( - [PluginTableInfo(name: "recent", type: "VIEW", schema: "shop", comment: nil)], + [object("recent")], schema: "shop", using: driver ) @@ -57,13 +65,43 @@ final class CompareSourceDefinitionReadTests: XCTestCase { XCTAssertEqual(reads.first?.source, "CREATE VIEW recent AS SELECT 1") } + func testAMaterializedViewsIndexesTravelWithItsDefinition() async throws { + let driver = SourceDefinitionStubDriver() + driver.viewDefinitions = [ + "totals": .success("CREATE MATERIALIZED VIEW shop.totals AS SELECT 1 AS id"), + "denied": .success("CREATE MATERIALIZED VIEW shop.denied AS SELECT 1 AS id") + ] + + let reads = try await CompareMetadataService.readViewDefinitions( + [ + object("totals", type: "MATERIALIZED VIEW", indexes: .read([ + PluginIndexInfo(name: "totals_id_idx", columns: ["id"], isUnique: true) + ])), + object("denied", type: "MATERIALIZED VIEW", indexes: .failed("permission denied")), + object("recent") + ], + schema: "shop", + using: driver + ) + + guard case .read(let indexes)? = reads.first?.indexes else { + return XCTFail("the indexes read with the view must reach its definition read") + } + XCTAssertEqual(indexes.map(\.name), ["totals_id_idx"]) + guard case .failed(let reason)? = reads.dropFirst().first?.indexes else { + return XCTFail("a failed index read must reach the definition read as a failure") + } + XCTAssertEqual(reason, "permission denied") + XCTAssertNil(reads.last?.indexes) + } + func testACancelledViewReadIsNotAFailure() async { let driver = SourceDefinitionStubDriver() driver.viewDefinitions = ["recent": .failure(CancellationError())] do { _ = try await CompareMetadataService.readViewDefinitions( - [PluginTableInfo(name: "recent", type: "VIEW", schema: "shop", comment: nil)], + [object("recent")], schema: "shop", using: driver ) diff --git a/TableProTests/Core/Compare/ConcurrentRefreshIndexRuleTests.swift b/TableProTests/Core/Compare/ConcurrentRefreshIndexRuleTests.swift new file mode 100644 index 0000000000..4a764ba7fd --- /dev/null +++ b/TableProTests/Core/Compare/ConcurrentRefreshIndexRuleTests.swift @@ -0,0 +1,44 @@ +// +// ConcurrentRefreshIndexRuleTests.swift +// TableProTests +// + +@testable import TablePro +import XCTest + +/// The rule PostgreSQL applies to `REFRESH MATERIALIZED VIEW CONCURRENTLY`, as the index read +/// reports it. Each shape was measured on PostgreSQL 17.11. +final class ConcurrentRefreshIndexRuleTests: XCTestCase { + private func index( + unique: Bool = true, + type: EditableIndexDefinition.IndexType = .btree, + columns: [String] = ["id"], + expressions: [String] = [], + includedColumns: [String] = [], + whereClause: String? = nil + ) -> EditableIndexDefinition { + EditableIndexDefinition( + id: UUID(), name: "i", columns: columns, type: type, isUnique: unique, isPrimary: false, + comment: nil, whereClause: whereClause, expressions: expressions, includedColumns: includedColumns + ) + } + + func testAPlainUniqueIndexAllowsIt() { + XCTAssertTrue(ConcurrentRefreshIndexRule.isUsable(index())) + XCTAssertTrue(ConcurrentRefreshIndexRule.isUsable(index(columns: ["customer", "id"]))) + XCTAssertTrue(ConcurrentRefreshIndexRule.isUsable(index(includedColumns: ["customer"]))) + } + + func testAPredicateAnExpressionOrANonUniqueIndexDoesNot() { + XCTAssertFalse(ConcurrentRefreshIndexRule.isUsable(index(whereClause: "id > 0"))) + XCTAssertFalse(ConcurrentRefreshIndexRule.isUsable(index(columns: ["(id + 0)"], expressions: ["(id + 0)"]))) + XCTAssertFalse(ConcurrentRefreshIndexRule.isUsable(index(unique: false))) + XCTAssertFalse(ConcurrentRefreshIndexRule.isUsable(index(type: .gist))) + } + + func testOneUsableIndexIsEnough() { + XCTAssertTrue(ConcurrentRefreshIndexRule.allowsConcurrentRefresh([index(unique: false), index()])) + XCTAssertFalse(ConcurrentRefreshIndexRule.allowsConcurrentRefresh([index(unique: false)])) + XCTAssertFalse(ConcurrentRefreshIndexRule.allowsConcurrentRefresh([])) + } +} diff --git a/TableProTests/Core/Compare/MaterializedViewIndexCompareTests.swift b/TableProTests/Core/Compare/MaterializedViewIndexCompareTests.swift new file mode 100644 index 0000000000..c8096d1025 --- /dev/null +++ b/TableProTests/Core/Compare/MaterializedViewIndexCompareTests.swift @@ -0,0 +1,187 @@ +// +// MaterializedViewIndexCompareTests.swift +// TableProTests +// +// A materialized view's definition text does not carry its indexes, so a comparison that read +// the text alone called two views identical when only their indexes differed, and a sync that +// dropped and created one again left it without the unique index its concurrent refresh needs. +// + +@testable import TablePro +import TableProPluginKit +import XCTest + +final class MaterializedViewIndexCompareTests: XCTestCase { + private let definition = "CREATE MATERIALIZED VIEW public.mv AS SELECT id, customer FROM orders WITH DATA;" + + private let uniqueId = PluginIndexInfo(name: "mv_id_idx", columns: ["id"], isUnique: true) + private let customer = PluginIndexInfo(name: "mv_customer_idx", columns: ["customer"]) + + private func read( + _ source: String? = nil, + schema: String? = "public", + indexes: ObjectIndexRead? + ) -> RoutineSourceRead { + RoutineSourceRead( + name: "mv", kind: .materializedView, schema: schema, signature: nil, + source: source ?? definition, indexes: indexes + ) + } + + private func compare( + source: RoutineSourceRead?, + target: RoutineSourceRead?, + targetCarries: Bool = true + ) -> CompareObjectResult? { + SourceObjectDiffEngine( + sourceDatabaseType: .postgresql, + targetDatabaseType: .postgresql, + targetIndexedKinds: targetCarries ? [.materializedView] : [] + ) + .compare(source: source.map { [$0] } ?? [], target: target.map { [$0] } ?? []) + .first + } + + // MARK: - Differences + + func testAnIndexOnlyDifferenceIsADifference() throws { + let result = try XCTUnwrap(compare( + source: read(indexes: .read([uniqueId])), + target: read(indexes: .read([])) + )) + + XCTAssertEqual(result.status, .differs) + XCTAssertTrue(result.definitionMatches) + XCTAssertEqual(result.changes.map(\.description), ["Add index 'mv_id_idx'"]) + XCTAssertEqual(result.suggestedAction, .alter) + } + + func testEqualIndexSetsAreIdentical() { + let result = compare( + source: read(indexes: .read([uniqueId, customer])), + target: read(indexes: .read([customer, uniqueId])) + ) + + XCTAssertEqual(result?.status, .identical) + XCTAssertEqual(result?.changes.count, 0) + } + + func testTheSameIndexUnderAnotherNameIsANoteRatherThanAChange() { + let renamed = PluginIndexInfo(name: "mv_id_key", columns: ["id"], isUnique: true) + + let result = compare(source: read(indexes: .read([uniqueId])), target: read(indexes: .read([renamed]))) + + XCTAssertEqual(result?.changes.count, 0) + XCTAssertEqual(result?.notes.count, 1) + } + + func testADefinitionDifferenceStillListsTheIndexChanges() throws { + let result = try XCTUnwrap(compare( + source: read("CREATE MATERIALIZED VIEW public.mv AS SELECT id FROM orders;", indexes: .read([uniqueId])), + target: read(indexes: .read([customer])) + )) + + XCTAssertEqual(result.status, .differs) + XCTAssertFalse(result.definitionMatches) + XCTAssertEqual(result.sourceIndexes?.map(\.name), ["mv_id_idx"]) + XCTAssertEqual(result.targetIndexes?.map(\.name), ["mv_customer_idx"]) + } + + // MARK: - Failed reads + + func testAFailedIndexReadOnEitherSideIsNotCompared() { + let sourceFailed = compare( + source: read(indexes: .failed("permission denied for pg_index")), + target: read(indexes: .read([uniqueId])) + ) + let targetFailed = compare( + source: read(indexes: .read([uniqueId])), + target: read(indexes: .failed("permission denied for pg_index")) + ) + + for result in [sourceFailed, targetFailed] { + XCTAssertEqual(result?.comparisonError?.contains("permission denied for pg_index"), true) + XCTAssertEqual(result?.suggestedAction, .skip) + XCTAssertEqual(result?.availableActions, [.skip]) + XCTAssertEqual(result?.changes.count, 0, "no DROP INDEX may come from a read that failed") + } + XCTAssertEqual(sourceFailed?.comparisonError?.contains("source"), true) + XCTAssertEqual(targetFailed?.comparisonError?.contains("target"), true) + } + + func testAViewWhoseIndexesCouldNotBeReadIsNeverCreated() { + let result = compare(source: read(indexes: .failed("denied")), target: nil) + + XCTAssertEqual(result?.status, .onlyInSource) + XCTAssertNotNil(result?.comparisonError) + XCTAssertEqual(result?.suggestedAction, .skip) + } + + func testATargetWhoseIndexesCouldNotBeReadCanStillBeDropped() { + let result = compare(source: nil, target: read(indexes: .failed("denied"))) + + XCTAssertNil(result?.comparisonError) + XCTAssertEqual(result?.suggestedAction, .drop) + } + + // MARK: - Engines that do not carry them + + func testATargetThatTakesNoIndexesComparesTheDefinitionAlone() { + let result = compare( + source: read(indexes: .read([uniqueId])), + target: read(indexes: nil), + targetCarries: false + ) + + XCTAssertEqual(result?.status, .identical) + XCTAssertNil(result?.sourceIndexes) + XCTAssertEqual(result?.showsIndexes, false) + } + + func testASourceThatReportsNoIndexesComparesTheDefinitionAlone() { + let result = compare(source: read(indexes: nil), target: read(indexes: .read([uniqueId]))) + + XCTAssertEqual(result?.status, .identical) + XCTAssertNil(result?.sourceIndexes) + } + + func testACreateOnATargetThatTakesNoIndexesSaysTheyAreLeftOut() { + let result = compare(source: read(indexes: .read([uniqueId])), target: nil, targetCarries: false) + + XCTAssertEqual(result?.notes, [SourceObjectIndexes.notCarriedByTargetNote]) + XCTAssertNil(result?.sourceIndexes) + } + + func testACreateCarriesTheSourcesIndexes() { + let result = compare(source: read(indexes: .read([uniqueId, customer])), target: nil) + + XCTAssertEqual(result?.sourceIndexes?.map(\.name), ["mv_id_idx", "mv_customer_idx"]) + XCTAssertEqual(result?.showsIndexes, true) + } + + // MARK: - What the definitions pane shows + + func testTheIndexLinesAreDisplayedApartFromTheDefinition() throws { + let result = try XCTUnwrap(compare( + source: read(indexes: .read([uniqueId])), + target: read(indexes: .read([])) + )) + + XCTAssertEqual(result.sourceIndexLines, ["UNIQUE INDEX mv_id_idx (id) USING BTREE"]) + XCTAssertEqual(result.targetIndexLines, []) + XCTAssertFalse(result.sourceDefinition.joined().contains("INDEX")) + } + + func testAnIndexOnlyDifferenceCountsItsChangesInTheResultsList() throws { + let result = try XCTUnwrap(compare( + source: read(indexes: .read([uniqueId, customer])), + target: read(indexes: .read([])) + )) + + let row = CompareResultGrouping.rows( + from: [result], sortedUsing: [KeyPathComparator(\CompareResultRow.objectName)] + ).first + + XCTAssertEqual(row?.changeSummary, String(format: String(localized: "%d changes"), 2)) + } +} diff --git a/TableProTests/Core/Compare/SourceObjectDiffEngineTests.swift b/TableProTests/Core/Compare/SourceObjectDiffEngineTests.swift index ec9f85eb74..a9624e683b 100644 --- a/TableProTests/Core/Compare/SourceObjectDiffEngineTests.swift +++ b/TableProTests/Core/Compare/SourceObjectDiffEngineTests.swift @@ -29,7 +29,9 @@ final class SourceObjectDiffEngineTests: XCTestCase { _ options: StructureCompareOptions = .default, databaseType: DatabaseType = .postgresql ) -> SourceObjectDiffEngine { - SourceObjectDiffEngine(options: options, sourceDatabaseType: databaseType, targetDatabaseType: databaseType) + SourceObjectDiffEngine( + options: options, sourceDatabaseType: databaseType, targetDatabaseType: databaseType, targetIndexedKinds: [] + ) } private func status( @@ -286,11 +288,15 @@ final class SourceObjectDiffEngineTests: XCTestCase { let mysqlDefinition = "# reporting view\nCREATE VIEW v AS SELECT 1" let postgresDefinition = "CREATE VIEW v AS SELECT 1" - let fromMySQL = SourceObjectDiffEngine(sourceDatabaseType: .mysql, targetDatabaseType: .postgresql).compare( + let fromMySQL = SourceObjectDiffEngine( + sourceDatabaseType: .mysql, targetDatabaseType: .postgresql, targetIndexedKinds: [] + ).compare( source: [read("v", kind: .view, schema: nil, source: mysqlDefinition)], target: [read("v", kind: .view, schema: nil, source: postgresDefinition)] ) - let intoMySQL = SourceObjectDiffEngine(sourceDatabaseType: .postgresql, targetDatabaseType: .mysql).compare( + let intoMySQL = SourceObjectDiffEngine( + sourceDatabaseType: .postgresql, targetDatabaseType: .mysql, targetIndexedKinds: [] + ).compare( source: [read("v", kind: .view, schema: nil, source: postgresDefinition)], target: [read("v", kind: .view, schema: nil, source: mysqlDefinition)] ) diff --git a/TableProTests/Core/Compare/SourceObjectIndexCarriageTests.swift b/TableProTests/Core/Compare/SourceObjectIndexCarriageTests.swift new file mode 100644 index 0000000000..13ad3475d5 --- /dev/null +++ b/TableProTests/Core/Compare/SourceObjectIndexCarriageTests.swift @@ -0,0 +1,29 @@ +// +// SourceObjectIndexCarriageTests.swift +// TableProTests +// + +@testable import TablePro +import XCTest + +@MainActor +final class SourceObjectIndexCarriageTests: XCTestCase { + func testTheMatrixDecidesWhichKindsCarryIndexes() { + XCTAssertTrue(SourceObjectIndexes.areCarried(for: .materializedView, by: .postgreSQL)) + XCTAssertFalse(SourceObjectIndexes.areCarried(for: .view, by: .postgreSQL)) + XCTAssertFalse(SourceObjectIndexes.areCarried(for: .materializedView, by: .tablesOnly)) + XCTAssertFalse(SourceObjectIndexes.areCarried(for: .table, by: .postgreSQL)) + XCTAssertFalse(SourceObjectIndexes.areCarried(for: .function, by: .postgreSQL)) + } + + func testPostgreSQLAndPGliteCarryAMaterializedViewsIndexes() { + XCTAssertEqual(SourceObjectIndexes.carriedKinds(on: .postgresql), [.materializedView]) + XCTAssertEqual(SourceObjectIndexes.carriedKinds(on: .pglite), [.materializedView]) + } + + func testEnginesNobodyHasCuratedCarryNone() { + XCTAssertEqual(SourceObjectIndexes.carriedKinds(on: .cockroachdb), []) + XCTAssertEqual(SourceObjectIndexes.carriedKinds(on: .redshift), []) + XCTAssertEqual(SourceObjectIndexes.carriedKinds(on: DatabaseType(rawValue: "NotARealEngine")), []) + } +} diff --git a/TableProTests/Core/Compare/SourceObjectIndexCopyTests.swift b/TableProTests/Core/Compare/SourceObjectIndexCopyTests.swift new file mode 100644 index 0000000000..8feba6acb2 --- /dev/null +++ b/TableProTests/Core/Compare/SourceObjectIndexCopyTests.swift @@ -0,0 +1,71 @@ +// +// SourceObjectIndexCopyTests.swift +// TableProTests +// + +@testable import TablePro +import TableProPluginKit +import XCTest + +final class SourceObjectIndexCopyTests: XCTestCase { + private let uniqueId = PluginIndexInfo(name: "mv_id_idx", columns: ["id"], isUnique: true) + + private func read(schema: String? = "public", indexes: ObjectIndexRead?) -> RoutineSourceRead { + RoutineSourceRead( + name: "mv", kind: .materializedView, schema: schema, signature: nil, + source: "CREATE MATERIALIZED VIEW public.mv AS SELECT 1 AS id;", indexes: indexes + ) + } + + func testIndexesAreWrittenWhereTheTargetTakesThemInTheSameSchema() { + let decision = SourceObjectIndexCopy.decide( + for: read(indexes: .read([uniqueId])), targetCarries: true, indexSchema: "public" + ) + + guard case .write(let indexes) = decision else { return XCTFail("expected the indexes, got \(decision)") } + XCTAssertEqual(indexes.map(\.name), ["mv_id_idx"]) + } + + func testATargetThatTakesNoIndexesGetsTheViewAndANote() { + XCTAssertEqual( + SourceObjectIndexCopy.decide( + for: read(indexes: .read([uniqueId])), targetCarries: false, indexSchema: "public" + ), + .leaveOut(SourceObjectIndexes.notCarriedByTargetNote) + ) + } + + /// A duplicated database is planned against the server's default database, whose schema is + /// `public`, while the definition names the schema it came from. + func testIndexStatementsThatWouldNameAnotherSchemaAreLeftOut() { + let other = SourceObjectIndexCopy.decide( + for: read(schema: "sales", indexes: .read([uniqueId])), targetCarries: true, indexSchema: "public" + ) + let unknown = SourceObjectIndexCopy.decide( + for: read(indexes: .read([uniqueId])), targetCarries: true, indexSchema: nil + ) + + XCTAssertEqual(other, .leaveOut(SourceObjectIndexes.otherSchemaNote)) + XCTAssertEqual(unknown, .leaveOut(SourceObjectIndexes.otherSchemaNote)) + } + + func testAFailedIndexReadRefusesTheViewWithItsReason() { + let decision = SourceObjectIndexCopy.decide( + for: read(indexes: .failed("permission denied for pg_index")), targetCarries: true, indexSchema: "public" + ) + + guard case .refuse(let reason) = decision else { return XCTFail("expected a refusal, got \(decision)") } + XCTAssertTrue(reason.contains("permission denied for pg_index"), reason) + } + + func testNothingIsSaidWhenThereIsNothingToCarry() { + XCTAssertEqual( + SourceObjectIndexCopy.decide(for: read(indexes: .read([])), targetCarries: true, indexSchema: "public"), + .none + ) + XCTAssertEqual( + SourceObjectIndexCopy.decide(for: read(indexes: nil), targetCarries: true, indexSchema: "public"), + .none + ) + } +} diff --git a/TableProTests/Core/Compare/SourceObjectSyncBuilderTests.swift b/TableProTests/Core/Compare/SourceObjectSyncBuilderTests.swift index b5473ff4cd..edc58f8bc0 100644 --- a/TableProTests/Core/Compare/SourceObjectSyncBuilderTests.swift +++ b/TableProTests/Core/Compare/SourceObjectSyncBuilderTests.swift @@ -295,4 +295,181 @@ final class SourceObjectSyncBuilderTests: XCTestCase { XCTAssertEqual(statements.map(\.sql), ["DROP VIEW \"shop\".\"recent\""]) } + + // MARK: - A materialized view's indexes + + private let matviewDefinition = "CREATE MATERIALIZED VIEW \"public\".\"mv\" AS SELECT id, customer FROM orders" + + private func index(_ name: String, _ columns: [String], unique: Bool = false) -> EditableIndexDefinition { + EditableIndexDefinition( + id: UUID(), name: name, columns: columns, type: .btree, isUnique: unique, isPrimary: false, comment: nil + ) + } + + private func matview( + schema: String = "public", + status: TableDiffStatus, + changes: [SchemaChange] = [], + sourceIndexes: [EditableIndexDefinition]?, + targetIndexes: [EditableIndexDefinition]? = nil, + definitionMatches: Bool = false + ) -> CompareObjectResult { + CompareObjectResult( + identity: CompareObjectIdentity(kind: .materializedView, schema: schema, name: "mv"), + status: status, + changes: changes, + sourceDefinition: [matviewDefinition], + sourceIndexes: sourceIndexes, + targetIndexes: targetIndexes, + definitionMatches: definitionMatches + ) + } + + private func indexBuilder( + _ driver: IndexStatementStubDriver = IndexStatementStubDriver(), + indexSchema: String? = "public" + ) -> SourceObjectSyncBuilder { + SourceObjectSyncBuilder(targetDriver: driver, targetDatabaseType: .postgresql, indexSchema: indexSchema) + } + + func testACreatedMaterializedViewGetsTheSourcesIndexesAfterIt() throws { + let result = matview( + status: .onlyInSource, + sourceIndexes: [index("mv_id_idx", ["id"], unique: true), index("mv_customer_idx", ["customer"])] + ) + + let statements = try indexBuilder().build(for: result, action: .create) + + XCTAssertEqual(statements.map(\.sql), [ + matviewDefinition, + "CREATE UNIQUE INDEX \"mv_id_idx\" ON \"public\".\"mv\" USING btree (\"id\")", + "CREATE INDEX \"mv_customer_idx\" ON \"public\".\"mv\" USING btree (\"customer\")" + ]) + XCTAssertEqual(Set(statements.map(\.objectName)), ["public.mv"], "one view is one object in the Apply sheet") + } + + func testAReplacedMaterializedViewGetsTheSourcesIndexesBack() throws { + let result = matview( + status: .differs, + sourceIndexes: [index("mv_id_idx", ["id"], unique: true)], + targetIndexes: [index("mv_id_idx", ["id"], unique: true)] + ) + + let statements = try indexBuilder().build(for: result, action: .alter) + + XCTAssertEqual(statements.map(\.sql), [ + "DROP MATERIALIZED VIEW \"public\".\"mv\"", + matviewDefinition, + "CREATE UNIQUE INDEX \"mv_id_idx\" ON \"public\".\"mv\" USING btree (\"id\")" + ]) + let dropHazards = statements[0].hazards + XCTAssertTrue(dropHazards.contains { $0.severity == .refusedByDefault }) + XCTAssertTrue(dropHazards.contains { $0.explanation.contains("the source's indexes are created on it") }) + XCTAssertFalse(dropHazards.contains { $0.kind == .concurrentRefresh }) + } + + /// Measured on PostgreSQL 17.11: an index changed in place kept the 100 rows the view stored + /// while its base table had 101, where a DROP and CREATE would have computed them again. + func testAnIndexOnlyDifferenceChangesTheIndexesInPlace() throws { + let old = index("mv_customer_idx", ["customer"]) + let new = index("mv_customer_amount_idx", ["customer", "amount"]) + let result = matview( + status: .differs, + changes: [.addIndex(new), .deleteIndex(old)], + sourceIndexes: [new], + targetIndexes: [old], + definitionMatches: true + ) + + let statements = try indexBuilder().build(for: result, action: .alter) + + XCTAssertEqual(statements.map(\.sql), [ + "DROP INDEX \"public\".\"mv_customer_idx\"", + "CREATE INDEX \"mv_customer_amount_idx\" ON \"public\".\"mv\" USING btree (\"customer\", \"amount\")" + ]) + XCTAssertFalse(statements.contains { $0.sql.contains("MATERIALIZED VIEW") }) + XCTAssertEqual(Set(statements.map(\.objectName)), ["public.mv"]) + } + + func testDroppingTheLastUniqueIndexWarnsThatAConcurrentRefreshStopsWorking() throws { + let unique = index("mv_id_idx", ["id"], unique: true) + let plain = index("mv_id_plain_idx", ["id"]) + let result = matview( + status: .differs, + changes: [.addIndex(plain), .deleteIndex(unique)], + sourceIndexes: [plain], + targetIndexes: [unique], + definitionMatches: true + ) + + let statements = try indexBuilder().build(for: result, action: .alter) + + let drop = try XCTUnwrap(statements.first { $0.sql.hasPrefix("DROP INDEX") }) + let create = try XCTUnwrap(statements.first { $0.sql.hasPrefix("CREATE INDEX") }) + XCTAssertTrue(drop.hazards.contains { $0.kind == .concurrentRefresh && $0.severity == .warning }) + XCTAssertFalse(create.hazards.contains { $0.kind == .concurrentRefresh }) + } + + func testAReplacementThatLosesTheUniqueIndexWarnsOnTheDrop() throws { + let result = matview( + status: .differs, + sourceIndexes: [index("mv_id_idx", ["id"])], + targetIndexes: [index("mv_id_idx", ["id"], unique: true)] + ) + + let statements = try indexBuilder().build(for: result, action: .alter) + + XCTAssertTrue(statements[0].hazards.contains { $0.kind == .concurrentRefresh }) + } + + func testAnIndexChangeThatKeepsAUsableUniqueIndexDoesNotWarn() throws { + let unique = index("mv_id_idx", ["id"], unique: true) + let key = index("mv_id_key", ["id", "customer"], unique: true) + let result = matview( + status: .differs, + changes: [.addIndex(key), .deleteIndex(unique)], + sourceIndexes: [key], + targetIndexes: [unique], + definitionMatches: true + ) + + let statements = try indexBuilder().build(for: result, action: .alter) + + XCTAssertFalse(statements.contains { $0.hazards.contains { $0.kind == .concurrentRefresh } }) + } + + /// Measured on PostgreSQL 17.11: `CREATE MATERIALIZED VIEW "a"."mv"` followed by an index on + /// `"b"."mv"` left `a.mv` with no index and indexed the target's own `b.mv` instead. + func testIndexesThatWouldNameAnotherSchemaThanTheDefinitionAreRefused() { + let result = matview(schema: "a", status: .onlyInSource, sourceIndexes: [index("mv_id_idx", ["id"])]) + let unknown = matview(status: .onlyInSource, sourceIndexes: [index("mv_id_idx", ["id"])]) + + XCTAssertThrowsError(try indexBuilder(indexSchema: "b").build(for: result, action: .create)) { error in + XCTAssertTrue(error.localizedDescription.contains("a.mv"), error.localizedDescription) + } + XCTAssertThrowsError(try indexBuilder(indexSchema: nil).build(for: unknown, action: .create)) + } + + func testAViewWithNoIndexesToWriteNeedsNoSchemaToWriteThemIn() throws { + let result = matview(schema: "a", status: .onlyInSource, sourceIndexes: []) + + XCTAssertEqual(try indexBuilder(indexSchema: "b").build(for: result, action: .create).count, 1) + } + + func testAnIndexTheTargetCannotWriteRefusesTheScript() { + let result = matview(status: .onlyInSource, sourceIndexes: [index("mv_id_idx", ["id"])]) + + XCTAssertThrowsError( + try indexBuilder(IndexStatementStubDriver(writesIndexes: false)).build(for: result, action: .create) + ) + } + + func testAMaterializedViewWhoseIndexesAreNotComparedKeepsTheDefinitionOnlyReplacement() throws { + let result = matview(status: .differs, sourceIndexes: nil) + + let statements = try indexBuilder().build(for: result, action: .alter) + + XCTAssertEqual(statements.map(\.sql), ["DROP MATERIALIZED VIEW \"public\".\"mv\"", matviewDefinition]) + XCTAssertTrue(statements[0].hazards.contains { $0.explanation.contains("along with its indexes") }) + } } diff --git a/TableProTests/Core/Compare/StructureChangeGuardTests.swift b/TableProTests/Core/Compare/StructureChangeGuardTests.swift index 6c04e6afe3..8052987e91 100644 --- a/TableProTests/Core/Compare/StructureChangeGuardTests.swift +++ b/TableProTests/Core/Compare/StructureChangeGuardTests.swift @@ -152,6 +152,72 @@ final class StructureChangeGuardTests: XCTestCase { ) } + // MARK: - A materialized view's indexes + + private func matviewIndex(_ name: String, unique: Bool = false) -> EditableIndexDefinition { + EditableIndexDefinition( + id: UUID(), name: name, columns: ["id"], type: .btree, isUnique: unique, isPrimary: false, comment: nil + ) + } + + private func matview( + changes: [SchemaChange] = [], + sourceIndexes: [EditableIndexDefinition]?, + definitionMatches: Bool = true + ) -> CompareObjectResult { + CompareObjectResult( + identity: CompareObjectIdentity(kind: .materializedView, schema: "shop", name: "totals"), + status: .differs, + changes: changes, + sourceDefinition: ["CREATE MATERIALIZED VIEW shop.totals AS SELECT 1 AS id"], + sourceIndexes: sourceIndexes, + definitionMatches: definitionMatches + ) + } + + func testTwoReadsOfAnUnchangedMaterializedViewAreAllowed() { + let read = { + self.matview( + changes: [.addIndex(self.matviewIndex("totals_id_idx"))], + sourceIndexes: [self.matviewIndex("totals_id_idx")] + ) + } + + XCTAssertNil(StructureChangeGuard.refusal( + expected: inputs([read()], action: .alter), actual: inputs([read()], action: .alter) + )) + } + + func testASourceIndexAddedAfterComparingRefusesTheScript() { + let expected = inputs([matview(sourceIndexes: [matviewIndex("totals_id_idx")])], action: .create) + let actual = inputs( + [matview(sourceIndexes: [matviewIndex("totals_id_idx"), matviewIndex("totals_key", unique: true)])], + action: .create + ) + + XCTAssertNotNil(StructureChangeGuard.refusal(expected: expected, actual: actual)) + } + + func testATargetIndexChangedAfterComparingRefusesAnIndexOnlyAlter() { + let expected = inputs( + [matview(changes: [.deleteIndex(matviewIndex("totals_old_idx"))], sourceIndexes: [])], action: .alter + ) + let actual = inputs( + [matview(changes: [.deleteIndex(matviewIndex("totals_other_idx"))], sourceIndexes: [])], action: .alter + ) + + XCTAssertNotNil(StructureChangeGuard.refusal(expected: expected, actual: actual)) + } + + /// An index-only alter keeps the view and its rows, and a definition that moved since would + /// turn it into a DROP and CREATE the user never reviewed. + func testADefinitionThatStoppedMatchingRefusesAnIndexOnlyAlter() { + let expected = inputs([matview(sourceIndexes: [], definitionMatches: true)], action: .alter) + let actual = inputs([matview(sourceIndexes: [], definitionMatches: false)], action: .alter) + + XCTAssertNotNil(StructureChangeGuard.refusal(expected: expected, actual: actual)) + } + // MARK: - Two reads of the same tables private func ordersRead( diff --git a/TableProTests/Core/ObjectCopy/ObjectCopyPlanTests.swift b/TableProTests/Core/ObjectCopy/ObjectCopyPlanTests.swift index e9c9f69bf8..33c37e4656 100644 --- a/TableProTests/Core/ObjectCopy/ObjectCopyPlanTests.swift +++ b/TableProTests/Core/ObjectCopy/ObjectCopyPlanTests.swift @@ -207,4 +207,44 @@ final class ObjectCopyPlanTests: XCTestCase { /// The summary still counts the object once, which is the reason the ids had to collide. XCTAssertEqual(result.failedCount, 1) } + + func testPartlyCopiedListsViewsAsWellAsTables() { + let table = ObjectCopyTableStep( + selection: ObjectCopySelection(kind: .table, name: "orders", schema: "public"), + dropStatements: [], + sequenceStatements: [], + createStatements: [statement("CREATE TABLE orders (id int)", "orders")], + truncateStatements: [], + columns: [], + primaryKeyColumns: [], + sourceQuery: "", + targetTable: "orders", + targetSchema: "public", + estimatedRows: nil, + copiesData: false, + copiesIdentityColumn: false, + note: "The source and the target share no writable column." + ) + let view = ObjectCopyDefinitionStep( + selection: ObjectCopySelection(kind: .materializedView, name: "totals", schema: "public"), + dropStatements: [], + createStatements: [statement("CREATE MATERIALIZED VIEW totals AS SELECT 1", "totals")], + note: SourceObjectIndexes.notCarriedByTargetNote + ) + let quiet = ObjectCopyDefinitionStep( + selection: ObjectCopySelection(kind: .view, name: "recent", schema: "public"), + dropStatements: [], + createStatements: [statement("CREATE VIEW recent AS SELECT 1", "recent")] + ) + + let copy = ObjectCopyPlan( + request: request(content: .structure), + createsDatabase: false, + tableSteps: [table], + definitionSteps: [view, quiet] + ) + + XCTAssertEqual(copy.partialNotes.map(\.selection.name), ["orders", "totals"]) + XCTAssertEqual(copy.partialNotes.last?.text, SourceObjectIndexes.notCarriedByTargetNote) + } } diff --git a/TableProTests/Core/ObjectCopy/ObjectCopyPlannerSourceDefinitionTests.swift b/TableProTests/Core/ObjectCopy/ObjectCopyPlannerSourceDefinitionTests.swift index 3c640e8e52..3d6b0d200c 100644 --- a/TableProTests/Core/ObjectCopy/ObjectCopyPlannerSourceDefinitionTests.swift +++ b/TableProTests/Core/ObjectCopy/ObjectCopyPlannerSourceDefinitionTests.swift @@ -68,7 +68,12 @@ final class ObjectCopyPlannerSourceDefinitionTests: XCTestCase { let reads = try await ObjectCopyPlanner.sourceDefinitionReads( for: [view, function], - views: [PluginTableInfo(name: "recent", type: "VIEW", schema: "public", comment: nil)], + views: [ + TableStructureRead( + table: PluginTableInfo(name: "recent", type: "VIEW", schema: "public", comment: nil), + columns: [], indexes: [], foreignKeys: [], metadata: nil, failure: nil + ) + ], triggerTables: [], schema: "public", endpointName: "Local / app / public", @@ -145,4 +150,81 @@ final class ObjectCopyPlannerSourceDefinitionTests: XCTestCase { XCTAssertTrue(error is CancellationError, "\(error)") } } + + // MARK: - A materialized view's indexes + + private let definition = "CREATE MATERIALIZED VIEW \"sales\".\"totals\" AS SELECT 1 AS id" + + private func input( + schema: String = "sales", + indexes: ObjectIndexRead?, + targetCarries: Bool = true + ) -> ObjectCopyDefinitionInput { + ObjectCopyDefinitionInput( + id: "totals", + identity: CompareObjectIdentity(kind: .materializedView, schema: schema, name: "totals"), + definition: definition, + read: RoutineSourceRead( + name: "totals", kind: .materializedView, schema: schema, signature: nil, + source: definition, indexes: indexes + ), + targetCarriesIndexes: targetCarries, + drop: nil + ) + } + + private func builder(indexSchema: String) -> SourceObjectSyncBuilder { + SourceObjectSyncBuilder( + targetDriver: IndexStatementStubDriver(indexSchema: indexSchema), + targetDatabaseType: .postgresql, + indexSchema: indexSchema + ) + } + + private let uniqueId = PluginIndexInfo(name: "totals_id_idx", columns: ["id"], isUnique: true) + + func testACopiedMaterializedViewGetsItsIndexesAfterItIsCreated() throws { + let build = try ObjectCopyPlanner.definitionBuild( + for: input(indexes: .read([uniqueId])), using: builder(indexSchema: "sales") + ) + + guard case .built(_, let create, let note) = build else { return XCTFail("expected statements, got \(build)") } + XCTAssertEqual(create.map(\.sql), [ + definition, + "CREATE UNIQUE INDEX \"totals_id_idx\" ON \"sales\".\"totals\" USING btree (\"id\")" + ]) + XCTAssertNil(note) + } + + /// A duplicated database is planned on the server's default database, so the index statements + /// name its schema while the definition names the schema the view came from. Written anyway, + /// they would index some other view of the same name or fail. + func testADuplicatedDatabaseWhoseIndexesWouldLandInAnotherSchemaCopiesTheViewAndSaysSo() throws { + let build = try ObjectCopyPlanner.definitionBuild( + for: input(indexes: .read([uniqueId])), using: builder(indexSchema: "public") + ) + + guard case .built(_, let create, let note) = build else { return XCTFail("expected statements, got \(build)") } + XCTAssertEqual(create.map(\.sql), [definition]) + XCTAssertEqual(note, SourceObjectIndexes.otherSchemaNote) + } + + func testATargetThatTakesNoIndexesGetsTheViewAndANote() throws { + let build = try ObjectCopyPlanner.definitionBuild( + for: input(indexes: .read([uniqueId]), targetCarries: false), using: builder(indexSchema: "sales") + ) + + guard case .built(_, let create, let note) = build else { return XCTFail("expected statements, got \(build)") } + XCTAssertEqual(create.map(\.sql), [definition]) + XCTAssertEqual(note, SourceObjectIndexes.notCarriedByTargetNote) + } + + func testAViewWhoseIndexesCouldNotBeReadIsLeftOutWithTheReason() throws { + let build = try ObjectCopyPlanner.definitionBuild( + for: input(indexes: .failed("permission denied for pg_index")), using: builder(indexSchema: "sales") + ) + + guard case .refused(let reason) = build else { return XCTFail("expected a refusal, got \(build)") } + XCTAssertTrue(reason.contains("permission denied for pg_index"), reason) + } } diff --git a/TableProTests/Helpers/IndexStatementStubDriver.swift b/TableProTests/Helpers/IndexStatementStubDriver.swift new file mode 100644 index 0000000000..9d10709775 --- /dev/null +++ b/TableProTests/Helpers/IndexStatementStubDriver.swift @@ -0,0 +1,59 @@ +// +// IndexStatementStubDriver.swift +// TableProTests +// + +import Foundation +@testable import TablePro +import TableProPluginKit + +internal final class IndexStatementStubDriver: PluginDatabaseDriver, @unchecked Sendable { + internal let indexSchema: String + internal let writesIndexes: Bool + + internal init(indexSchema: String = "public", writesIndexes: Bool = true) { + self.indexSchema = indexSchema + self.writesIndexes = writesIndexes + } + + internal var currentSchema: String? { indexSchema } + + internal func quoteIdentifier(_ name: String) -> String { "\"\(name)\"" } + + internal func generateAddIndexSQL(table: String, index: PluginIndexDefinition) -> String? { + guard writesIndexes else { return nil } + return PostgreSQLIndexClauses.createStatement( + for: index, qualifiedTable: "\(quoteIdentifier(indexSchema)).\(quoteIdentifier(table))" + ) + } + + internal func generateDropIndexSQL(table: String, indexName: String) -> String? { + guard writesIndexes else { return nil } + return "DROP INDEX \(quoteIdentifier(indexSchema)).\(quoteIdentifier(indexName))" + } + + internal func connect() async throws {} + internal func disconnect() {} + internal var isConnected: Bool { true } + + internal func execute(query: String) async throws -> PluginQueryResult { + PluginQueryResult(columns: [], columnTypeNames: [], rows: [], rowsAffected: 0, executionTime: 0) + } + + internal func fetchTables(schema: String?) async throws -> [PluginTableInfo] { [] } + internal func fetchColumns(table: String, schema: String?) async throws -> [PluginColumnInfo] { [] } + internal func fetchIndexes(table: String, schema: String?) async throws -> [PluginIndexInfo] { [] } + internal func fetchForeignKeys(table: String, schema: String?) async throws -> [PluginForeignKeyInfo] { [] } + internal func fetchTableDDL(table: String, schema: String?) async throws -> String { "" } + internal func fetchViewDefinition(view: String, schema: String?) async throws -> String { "" } + + internal func fetchTableMetadata(table: String, schema: String?) async throws -> PluginTableMetadata { + PluginTableMetadata(tableName: table) + } + + internal func fetchDatabases() async throws -> [String] { [] } + + internal func fetchDatabaseMetadata(_ database: String) async throws -> PluginDatabaseMetadata { + PluginDatabaseMetadata(name: database) + } +} diff --git a/docs/features/compare-sync.mdx b/docs/features/compare-sync.mdx index 838d241107..b9ca0f7d17 100644 --- a/docs/features/compare-sync.mdx +++ b/docs/features/compare-sync.mdx @@ -36,7 +36,8 @@ Nothing is written until **Apply**. Until then the strip along the top reads **C | Kind | Compared as | |---|---| | Tables | Parsed columns, indexes, foreign keys, storage engine and collation | -| Views, materialized views | Normalized definition text | +| Views | Normalized definition text | +| Materialized views | Normalized definition text, plus parsed indexes on PostgreSQL and PGlite | | Procedures, functions | Normalized definition text, matched on name and argument list | | Triggers | Normalized definition text, per table | @@ -71,7 +72,7 @@ A procedure, function or trigger list that cannot be read stops the comparison a On MySQL a list that is short is not an error. An account without `EXECUTE` lists no procedures or functions, and one without `TRIGGER` lists no triggers, so the other side's copies show as only on that side. Compare from accounts that hold both privileges. -The detail pane on the right has three tabs. **Definitions** shows the source and target side by side, split or unified, rendered from the same function so a formatting difference cannot appear as a real one. **Rows** is the data comparison. **Script** is the generated SQL. +The detail pane on the right has three tabs. **Definitions** shows the source and target side by side, split or unified, rendered from the same function so a formatting difference cannot appear as a real one. A PostgreSQL or PGlite materialized view shows its indexes under its definition, and one whose text matches but whose indexes differ is listed as **different**. **Rows** is the data comparison. **Script** is the generated SQL. ## Comparing rows @@ -125,6 +126,8 @@ Statements are ordered by foreign key dependency, not alphabetically. Tables are A view, routine or trigger that differs is dropped and created again. On Oracle it is replaced by its own `CREATE OR REPLACE` instead, with no `DROP` first, so a definition the server refuses leaves the target's object in place. +A PostgreSQL or PGlite materialized view whose definition matches and whose indexes differ keeps its rows: only its indexes are dropped and created. One whose definition differs is dropped, created again and computed again, then given the source's indexes. Its comments and privileges are not recreated. A change that leaves it with no unique index `REFRESH MATERIALIZED VIEW CONCURRENTLY` can use is listed among the warnings. The script stops at a materialized view whose indexes would be written into a schema other than the one its definition names: exclude it, or compare two schemas with the same name. + A saved script is written for the target engine's own client, so each statement ends the way that client reads it: | Target | A plain statement ends with | A routine, trigger or block ends with | diff --git a/docs/features/copy-objects.mdx b/docs/features/copy-objects.mdx index dc12cab21d..76902dbb1f 100644 --- a/docs/features/copy-objects.mdx +++ b/docs/features/copy-objects.mdx @@ -40,13 +40,19 @@ written until you have read the script. | Kind | Structure | Data | |---|---|---| | Tables | Columns, primary key, indexes, foreign keys, storage engine and collation | Every row, streamed in batches | -| Views, materialized views | The source's definition | None to carry | +| Views | The source's definition | None to carry | +| Materialized views | The source's definition, plus its indexes into PostgreSQL and PGlite | None to carry | | Procedures, functions | The source's definition | None to carry | | Triggers | The source's definition | None to carry | A data-only copy leaves views, routines and triggers out and says so in the review step: they hold no rows. +A materialized view's indexes are created right after the view. A target other than PostgreSQL or +PGlite gets the view without them, and so does a duplicate whose index statements would name a +schema other than the view's own; both are listed under **Partly copied**. A view whose indexes the +source refuses to list is left out with the server's reason when the target would take them. + Within one engine, generated and computed columns are dropped from the write. The server recomputes them, and every engine that has them rejects an `INSERT` that names one. Crossing to another engine they arrive as ordinary columns carrying the values they held on the source. @@ -202,7 +208,8 @@ Everything is torn down children first, so a foreign key is gone before the tabl built parents first, both in one pass so a stop between them cannot leave objects dropped with nothing put back. Rows are copied after that. Triggers and materialized views go in last: a trigger installed before the rows fires on the copy itself, and a materialized view is filled at the moment -it is created. +it is created. Its indexes follow it, and a unique index its rows violate fails with the view named +in the result. A copied foreign key is repointed at the copy. A key that referenced the source's own schema references the target's afterwards, so the duplicate stands on its own; one that referenced a third