Skip to content

Commit 0bfa0dc

Browse files
authored
fix(compare): stop scripting a view, routine or trigger whose definition could not be read (#3069)
* feat(datagrid): show whether a materialized view can be refreshed concurrently, and gate its structure edits by kind * fix(datagrid): let only the latest concurrent refresh check settle * fix(compare): stop scripting a view, routine or trigger whose definition could not be read * fix(compare): read a source definition in the source engine's grammar
1 parent d79eec5 commit 0bfa0dc

23 files changed

Lines changed: 1232 additions & 312 deletions

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
323323
- Saved Compare & Sync scripts that SQL*Plus, DISQL, the mysql client or SQL Server tools could not run.
324324
- Oracle, Dameng and MySQL SQL dumps whose routines and triggers the engine's own client could not restore.
325325
- Compare & Sync showing an Oracle unit missing the `;` after its `END` as identical.
326+
- Compare & Sync scripting a `DROP` with no `CREATE` for a view, routine or trigger whose definition it could not read.
327+
- Compare & Sync offering to drop every target procedure, function or trigger when the source's list could not be read.
328+
- DuckDB macro dropped and not recreated by a Compare & Sync replace.
329+
- Copy To giving no reason for a view, routine or trigger whose definition could not be read.
330+
- Copy To skipping a view, routine or trigger with a comment above its `CREATE`.
326331
- SSH jump hosts dropped from a connection synced to iPhone and iPad, and that connection then skipped on the way back.
327332
- An SSH tunnel pinned to port 22, and its auth method read back as Password, after a round trip through iPhone and iPad.
328333
- Redis database list failing on servers that refuse `CONFIG` or `INFO`, such as AWS ElastiCache and Azure Cache for Redis. (#3036)
Lines changed: 194 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,194 @@
1+
//
2+
// CompareMetadataService+SourceDefinitions.swift
3+
// TablePro
4+
//
5+
6+
import Foundation
7+
import os
8+
import TableProPluginKit
9+
import TableProSQLGrammar
10+
11+
internal struct RoutineSourceRead: Sendable {
12+
internal let name: String
13+
internal let kind: CompareObjectKind
14+
internal let schema: String?
15+
internal let signature: String?
16+
internal let source: String
17+
internal let failure: String?
18+
19+
internal init(
20+
name: String,
21+
kind: CompareObjectKind,
22+
schema: String?,
23+
signature: String?,
24+
source: String,
25+
failure: String? = nil
26+
) {
27+
self.name = name
28+
self.kind = kind
29+
self.schema = schema
30+
self.signature = signature
31+
self.source = source
32+
self.failure = failure
33+
}
34+
}
35+
36+
internal extension CompareMetadataService {
37+
nonisolated private static let definitionLogger = Logger(
38+
subsystem: "com.TablePro", category: "CompareMetadataService"
39+
)
40+
41+
nonisolated static func readViewDefinitions(
42+
_ views: [PluginTableInfo],
43+
schema: String?,
44+
using plugin: any PluginDatabaseDriver
45+
) async throws -> [RoutineSourceRead] {
46+
var reads: [RoutineSourceRead] = []
47+
for view in views {
48+
try Task.checkCancellation()
49+
let viewSchema = view.schema ?? schema
50+
reads.append(try await definitionRead(
51+
name: view.name,
52+
kind: CompareTableKindClassifier.kind(of: view),
53+
schema: viewSchema,
54+
signature: nil
55+
) {
56+
try await plugin.fetchViewDefinition(view: view.name, schema: viewSchema)
57+
})
58+
}
59+
return reads
60+
}
61+
62+
nonisolated static func readRoutineDefinitions(
63+
schema: String?,
64+
endpointName: String,
65+
using plugin: any PluginDatabaseDriver
66+
) async throws -> [RoutineSourceRead] {
67+
let routines: [PluginRoutineInfo]
68+
do {
69+
routines = try await plugin.fetchRoutines(schema: schema)
70+
} catch {
71+
throw listingFailure(error, message: String(
72+
format: String(localized: "The procedures and functions in %1$@ could not be listed: %2$@"),
73+
endpointName, error.localizedDescription
74+
))
75+
}
76+
var reads: [RoutineSourceRead] = []
77+
for routine in routines {
78+
try Task.checkCancellation()
79+
reads.append(try await definitionRead(
80+
name: routine.name,
81+
kind: routine.kind == .procedure ? .procedure : .function,
82+
schema: routine.schema ?? schema,
83+
signature: routine.argumentSignature
84+
) {
85+
try await plugin.fetchRoutineDDL(routine)
86+
})
87+
}
88+
return reads
89+
}
90+
91+
nonisolated static func readTriggerDefinitions(
92+
tables: [String],
93+
schema: String?,
94+
endpointName: String,
95+
using plugin: any PluginDatabaseDriver
96+
) async throws -> [RoutineSourceRead] {
97+
let listed = try await listTriggers(tables: tables, schema: schema, endpointName: endpointName, using: plugin)
98+
var reads: [RoutineSourceRead] = []
99+
for (trigger, owningTable) in listed {
100+
try Task.checkCancellation()
101+
reads.append(try await definitionRead(
102+
name: trigger.name,
103+
kind: .trigger,
104+
schema: trigger.schema ?? schema,
105+
signature: trigger.table ?? owningTable
106+
) {
107+
if let definition = trigger.definition, StatementBlank.hasContent(definition) {
108+
return definition
109+
}
110+
return try await plugin.fetchTriggerDDL(trigger)
111+
})
112+
}
113+
return reads
114+
}
115+
116+
nonisolated private static func listTriggers(
117+
tables: [String],
118+
schema: String?,
119+
endpointName: String,
120+
using plugin: any PluginDatabaseDriver
121+
) async throws -> [(trigger: PluginTriggerInfo, owningTable: String?)] {
122+
guard plugin.providesBulkTriggerFetch else {
123+
return try await listTriggersPerTable(tables, schema: schema, endpointName: endpointName, using: plugin)
124+
}
125+
let triggers: [PluginTriggerInfo]
126+
do {
127+
triggers = try await plugin.fetchAllTriggers(schema: schema)
128+
} catch is CancellationError {
129+
throw CancellationError()
130+
} catch {
131+
definitionLogger.warning(
132+
"Whole-schema trigger read failed, falling back per table: \(error.publicLogShape, privacy: .public)"
133+
)
134+
return try await listTriggersPerTable(tables, schema: schema, endpointName: endpointName, using: plugin)
135+
}
136+
let inScope = Set(tables.map { $0.lowercased() })
137+
return triggers
138+
.filter { trigger in
139+
guard let table = trigger.table?.lowercased() else { return true }
140+
return inScope.contains(table)
141+
}
142+
.map { (trigger: $0, owningTable: nil) }
143+
}
144+
145+
nonisolated private static func listTriggersPerTable(
146+
_ tables: [String],
147+
schema: String?,
148+
endpointName: String,
149+
using plugin: any PluginDatabaseDriver
150+
) async throws -> [(trigger: PluginTriggerInfo, owningTable: String?)] {
151+
var listed: [(trigger: PluginTriggerInfo, owningTable: String?)] = []
152+
for table in tables {
153+
try Task.checkCancellation()
154+
do {
155+
listed += try await plugin.fetchTriggers(table: table, schema: schema)
156+
.map { (trigger: $0, owningTable: table) }
157+
} catch {
158+
throw listingFailure(error, message: String(
159+
format: String(localized: "The triggers on %1$@ in %2$@ could not be listed: %3$@"),
160+
table, endpointName, error.localizedDescription
161+
))
162+
}
163+
}
164+
return listed
165+
}
166+
167+
nonisolated private static func listingFailure(_ error: Error, message: String) -> Error {
168+
guard !(error is CancellationError), !Task.isCancelled else { return CancellationError() }
169+
definitionLogger.warning("Definition listing failed: \(error.publicLogShape, privacy: .public)")
170+
return CompareSyncError.readFailed(message)
171+
}
172+
173+
nonisolated private static func definitionRead(
174+
name: String,
175+
kind: CompareObjectKind,
176+
schema: String?,
177+
signature: String?,
178+
reading: () async throws -> String
179+
) async throws -> RoutineSourceRead {
180+
do {
181+
let source = try await reading()
182+
return RoutineSourceRead(name: name, kind: kind, schema: schema, signature: signature, source: source)
183+
} catch {
184+
guard !(error is CancellationError), !Task.isCancelled else { throw CancellationError() }
185+
definitionLogger.warning(
186+
"Definition read failed for \(kind.rawValue, privacy: .public) \(name, privacy: .private(mask: .hash)): \(error.publicLogShape, privacy: .public)"
187+
)
188+
return RoutineSourceRead(
189+
name: name, kind: kind, schema: schema, signature: signature,
190+
source: "", failure: error.localizedDescription
191+
)
192+
}
193+
}
194+
}

TablePro/Core/Compare/CompareMetadataService.swift

Lines changed: 7 additions & 103 deletions
Original file line numberDiff line numberDiff line change
@@ -39,14 +39,6 @@ internal struct TableStructureRead: Sendable {
3939
}
4040
}
4141

42-
internal struct RoutineSourceRead: Sendable {
43-
internal let name: String
44-
internal let kind: CompareObjectKind
45-
internal let schema: String?
46-
internal let signature: String?
47-
internal let source: String
48-
}
49-
5042
@MainActor
5143
internal struct CompareMetadataService {
5244
nonisolated private static let logger = Logger(subsystem: "com.TablePro", category: "CompareMetadataService")
@@ -159,106 +151,33 @@ internal struct CompareMetadataService {
159151
return try await (source, target)
160152
}
161153

162-
/// `fetchRoutines` supersedes the old per-kind pair and carries `identity`, which is what
163-
/// `fetchRoutineDDL` needs to address an overloaded routine again. A routine whose DDL cannot
164-
/// be read is still listed, with an empty definition, so it shows as present rather than
165-
/// vanishing from the comparison.
166154
internal func routineReads(
167155
for endpoint: DatabaseEndpoint,
168156
connection: DatabaseConnection
169157
) async throws -> [RoutineSourceRead] {
170158
try await manager.ensureConnected(connection)
171159
let schema = endpoint.schema
160+
let endpointName = endpoint.qualifiedDescription
172161
return try await manager.withMetadataDriver(scope: endpoint.scope) { driver in
173162
guard let plugin = Self.pluginDriver(from: driver) else { return [] }
174-
let routines = (try? await plugin.fetchRoutines(schema: schema)) ?? []
175-
var reads: [RoutineSourceRead] = []
176-
for routine in routines {
177-
try Task.checkCancellation()
178-
var source = routine.definition ?? ""
179-
if source.isEmpty {
180-
source = (try? await plugin.fetchRoutineDDL(routine)) ?? ""
181-
}
182-
reads.append(RoutineSourceRead(
183-
name: routine.name,
184-
kind: routine.kind == .procedure ? .procedure : .function,
185-
schema: routine.schema ?? schema,
186-
signature: routine.argumentSignature,
187-
source: source
188-
))
189-
}
190-
return reads
163+
return try await Self.readRoutineDefinitions(schema: schema, endpointName: endpointName, using: plugin)
191164
}
192165
}
193166

194-
/// A trigger on a table that is not in scope is not in scope either, so the tables the
195-
/// structure read already listed are the ones kept.
196-
///
197-
/// The whole-schema read is one query where the driver has one. Where it does not, the
198-
/// protocol's default answers with nothing rather than looping, so the per-table read is the
199-
/// only correct fallback and `providesBulkTriggerFetch` is what tells the two apart.
200167
internal func triggerReads(
201168
for endpoint: DatabaseEndpoint,
202169
connection: DatabaseConnection,
203170
tables: [String]
204171
) async throws -> [RoutineSourceRead] {
205172
try await manager.ensureConnected(connection)
206173
let schema = endpoint.schema
207-
let inScope = Set(tables.map { $0.lowercased() })
174+
let endpointName = endpoint.qualifiedDescription
208175
return try await manager.withMetadataDriver(scope: endpoint.scope) { driver in
209176
guard let plugin = Self.pluginDriver(from: driver) else { return [] }
210-
guard plugin.providesBulkTriggerFetch else {
211-
return try await Self.perTableTriggerReads(tables: tables, schema: schema, using: plugin)
212-
}
213-
/// A failed whole-schema query is not an answer of "no triggers". Swallowing it made an
214-
/// empty set authoritative on one side, so every trigger on the other side read as a
215-
/// real difference and the script offered to drop or create all of them.
216-
let triggers: [PluginTriggerInfo]
217-
do {
218-
triggers = try await plugin.fetchAllTriggers(schema: schema)
219-
} catch is CancellationError {
220-
throw CancellationError()
221-
} catch {
222-
Self.logger.warning(
223-
"Whole-schema trigger read failed, falling back per table: \(error.publicLogShape, privacy: .public)"
224-
)
225-
return try await Self.perTableTriggerReads(tables: tables, schema: schema, using: plugin)
226-
}
227-
return triggers
228-
.filter { trigger in
229-
guard let table = trigger.table?.lowercased() else { return true }
230-
return inScope.contains(table)
231-
}
232-
.map { Self.read($0, schema: schema, fallbackTable: nil) }
233-
}
234-
}
235-
236-
nonisolated private static func perTableTriggerReads(
237-
tables: [String],
238-
schema: String?,
239-
using plugin: any PluginDatabaseDriver
240-
) async throws -> [RoutineSourceRead] {
241-
var reads: [RoutineSourceRead] = []
242-
for table in tables {
243-
try Task.checkCancellation()
244-
guard let triggers = try? await plugin.fetchTriggers(table: table, schema: schema) else { continue }
245-
reads += triggers.map { read($0, schema: schema, fallbackTable: table) }
177+
return try await Self.readTriggerDefinitions(
178+
tables: tables, schema: schema, endpointName: endpointName, using: plugin
179+
)
246180
}
247-
return reads
248-
}
249-
250-
nonisolated private static func read(
251-
_ trigger: PluginTriggerInfo,
252-
schema: String?,
253-
fallbackTable: String?
254-
) -> RoutineSourceRead {
255-
RoutineSourceRead(
256-
name: trigger.name,
257-
kind: .trigger,
258-
schema: trigger.schema ?? schema,
259-
signature: trigger.table ?? fallbackTable,
260-
source: trigger.definition ?? trigger.statement
261-
)
262181
}
263182

264183
internal func viewDefinitions(
@@ -270,22 +189,7 @@ internal struct CompareMetadataService {
270189
let schema = endpoint.schema
271190
return try await manager.withMetadataDriver(scope: endpoint.scope) { driver in
272191
guard let plugin = Self.pluginDriver(from: driver) else { return [] }
273-
var reads: [RoutineSourceRead] = []
274-
for view in views {
275-
try Task.checkCancellation()
276-
let definition = try? await plugin.fetchViewDefinition(
277-
view: view.name, schema: view.schema ?? schema
278-
)
279-
let source = definition ?? ""
280-
reads.append(RoutineSourceRead(
281-
name: view.name,
282-
kind: CompareTableKindClassifier.kind(of: view),
283-
schema: view.schema ?? schema,
284-
signature: nil,
285-
source: source
286-
))
287-
}
288-
return reads
192+
return try await Self.readViewDefinitions(views, schema: schema, using: plugin)
289193
}
290194
}
291195

0 commit comments

Comments
 (0)