From 152613f8d67b0d42758d1a1d085e821ca2381301 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Tue, 29 Sep 2026 20:03:13 +0700 Subject: [PATCH 1/4] fix(editor): forget or move a table's saved settings when SQL drops or renames it --- CHANGELOG.md | 1 + .../QueryExecutionCoordinator+Batches.swift | 26 +- ...QueryExecutionCoordinator+Parameters.swift | 37 +- .../Access/DatabaseAccessBridge+Scripts.swift | 61 ++- .../Access/DatabaseAccessBridge.swift | 17 +- TablePro/Core/Events/CatalogChange.swift | 3 + .../Core/Events/SucceededStatements.swift | 83 ++++ .../Execution/BatchStatementRun.swift | 20 + .../Services/Query/CatalogChangeService.swift | 47 ++ .../Services/Query/CatalogEditAdoption.swift | 76 ++- .../Services/Query/CommittedTableEdits.swift | 196 ++++++++ .../Core/Utilities/SQL/SQLTokenCursor.swift | 13 +- .../Core/Utilities/SQL/TableEditDialect.swift | 221 +++++++++ .../SQL/TableEditStatementParser.swift | 386 +++++++++++++++ ...MainContentCoordinator+CatalogChange.swift | 11 +- .../Views/Main/MainContentCoordinator.swift | 11 +- .../Query/CommittedTableEditsTests.swift | 449 ++++++++++++++++++ .../Query/SQLTableEditAdoptionTests.swift | 205 ++++++++ .../Utilities/SQL/SQLTokenCursorTests.swift | 12 + .../SQL/TableEditStatementParserTests.swift | 410 ++++++++++++++++ docs/features/table-operations.mdx | 6 + 21 files changed, 2242 insertions(+), 49 deletions(-) create mode 100644 TablePro/Core/Events/SucceededStatements.swift create mode 100644 TablePro/Core/Services/Query/CommittedTableEdits.swift create mode 100644 TablePro/Core/Utilities/SQL/TableEditDialect.swift create mode 100644 TablePro/Core/Utilities/SQL/TableEditStatementParser.swift create mode 100644 TableProTests/Core/Services/Query/CommittedTableEditsTests.swift create mode 100644 TableProTests/Core/Services/Query/SQLTableEditAdoptionTests.swift create mode 100644 TableProTests/Core/Utilities/SQL/TableEditStatementParserTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 40eb3555b3..4fe16b1169 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - iOS row editor saving the placeholder of a long text or binary value over the full value. - Explain Analyze running write statements on Read-Only connections and skipping the Alert and Safe Mode confirmation. - **Local only** connections taking edits and deletions made on another device. +- Saved filters, layout, highlight rules, favorite and Recent entry kept by a table dropped or renamed in SQL. ## [0.76.1] - 2026-09-29 diff --git a/TablePro/Core/Coordinators/QueryExecutionCoordinator+Batches.swift b/TablePro/Core/Coordinators/QueryExecutionCoordinator+Batches.swift index fe4ba8cddb..eaa8734001 100644 --- a/TablePro/Core/Coordinators/QueryExecutionCoordinator+Batches.swift +++ b/TablePro/Core/Coordinators/QueryExecutionCoordinator+Batches.swift @@ -65,6 +65,7 @@ private struct BatchRun { let outcome: BatchStatementOutcome let plan: BatchTransactionPlan let sessionState: PluginSessionTransactionState + let startState: PluginSessionTransactionState var failureOutput: PluginServerOutput = .none } @@ -137,7 +138,7 @@ extension QueryExecutionCoordinator { let batchTask = Task { [weak self, parent] in guard let self else { return } let run = await runBatches(prepared, scope: scope, mode: mode, claim: claim, lease: lease) - postRanStatements(of: prepared, outcome: run.outcome, connection: conn) + postRanStatements(of: prepared, run: run, scope: scope, connection: conn) let sessionNotice = Self.runNotice(outcome: run.outcome, sessionState: run.sessionState) switch run.outcome { @@ -236,7 +237,8 @@ extension QueryExecutionCoordinator { route: DatabaseManager.shared.executionRoute(for: scope), cancellation: .cancellableRead(lease) ) { driver in - let plan = BatchTransactionPlan.autocommit.joining(await driver.heldSessionTransactionState()) + let startState = await driver.heldSessionTransactionState() + let plan = BatchTransactionPlan.autocommit.joining(startState) let outcome = await BatchStatementRun.run( prepared, plan: plan, @@ -251,18 +253,21 @@ extension QueryExecutionCoordinator { try await Self.runBatch(batch, driver: driver, failureOutput: failureOutput) } let sessionState = await driver.heldSessionTransactionState() - return BatchRun(outcome: outcome, plan: plan, sessionState: sessionState) + return BatchRun(outcome: outcome, plan: plan, sessionState: sessionState, startState: startState) } run.failureOutput = failureOutput.output return run } catch { if DatabaseCancellationDiagnosis.isCancellation(error) || Task.isCancelled { - return BatchRun(outcome: .cancelled(results: []), plan: .autocommit, sessionState: .unknown) + return BatchRun( + outcome: .cancelled(results: []), plan: .autocommit, sessionState: .unknown, startState: .unknown + ) } return BatchRun( outcome: .failed(results: [], failure: .connection, errorDescription: error.localizedDescription), plan: .autocommit, - sessionState: .unknown + sessionState: .unknown, + startState: .unknown ) } } @@ -316,11 +321,12 @@ extension QueryExecutionCoordinator { /// fetch, missing what did leaves the sidebar wrong. private func postRanStatements( of prepared: [PreparedBatch], - outcome: BatchStatementOutcome, + run: BatchRun, + scope: DatabaseScope, connection: DatabaseConnection ) { let ranCount: Int - switch outcome { + switch run.outcome { case .completed, .cancelled: ranCount = prepared.count case .failed(let outputs, let failure, _): @@ -330,6 +336,12 @@ extension QueryExecutionCoordinator { CatalogChangeService.post( .statementsRan(connectionId: connection.id, statements: statements, databaseType: connection.type) ) + CatalogChangeService.post(.statementsSucceeded(SucceededStatements( + scope: scope, + databaseType: connection.type, + statements: prepared.prefix(run.outcome.succeededCount).flatMap { $0.batch.statements.map(\.sql) }, + commit: .run(startedIn: run.startState, plan: run.plan, completed: run.outcome.isCompleted) + ))) } // MARK: - Results diff --git a/TablePro/Core/Coordinators/QueryExecutionCoordinator+Parameters.swift b/TablePro/Core/Coordinators/QueryExecutionCoordinator+Parameters.swift index 8195fae44c..01355fc2b9 100644 --- a/TablePro/Core/Coordinators/QueryExecutionCoordinator+Parameters.swift +++ b/TablePro/Core/Coordinators/QueryExecutionCoordinator+Parameters.swift @@ -27,6 +27,7 @@ private struct MultiStatementRun { let outcome: BatchStatementOutcome let plan: BatchTransactionPlan let sessionState: PluginSessionTransactionState + let startState: PluginSessionTransactionState var failureOutput: PluginServerOutput = .none } @@ -153,28 +154,31 @@ extension QueryExecutionCoordinator { let boundValues = BoundParameterValues(values: parameters) let failureOutput = ServerOutputBox() + let grammar = parent.lexicalGrammar let parameterizedTask = Task { [weak self, parent] in guard let self else { return } let schemaTask = QueryExecutor.schemaFetch(tableName: needsMetadataFetch ? tableName : nil, scope: scope) do { - let fetchResult = try await DatabaseManager.shared.withScopedDriver( + let (fetchResult, tableEdits) = try await DatabaseManager.shared.withScopedDriver( scope: scope, route: DatabaseManager.shared.executionRoute(for: scope), cancellation: .cancellableRead(lease) ) { [queryExecutor = parent.queryExecutor, boundValues] driver in - try await queryExecutor.executeQuery( + let fetched = try await queryExecutor.executeQuery( driver: driver, sql: statement.sql, parameters: boundValues.values, rowCap: rowCap, capturingOutputInto: failureOutput ) + let edits = await SucceededStatements.single( + statement.sql, scope: scope, databaseType: conn.type, grammar: grammar, ranOn: driver + ) + return (fetched, edits) } - CatalogChangeService.post( - .statementsRan(connectionId: conn.id, statements: [statement.sql], databaseType: conn.type) - ) + MainContentCoordinator.postStatementRan(statement.sql, on: conn, succeeded: tableEdits) guard !Task.isCancelled else { schemaTask?.cancel() @@ -341,6 +345,12 @@ extension QueryExecutionCoordinator { CatalogChangeService.post( .statementsRan(connectionId: conn.id, statements: ranStatements, databaseType: conn.type) ) + CatalogChangeService.post(.statementsSucceeded(SucceededStatements( + scope: scope, + databaseType: conn.type, + statements: prepared.prefix(outcome.succeededCount).map(\.sentSQL), + commit: .run(startedIn: run.startState, plan: run.plan, completed: outcome.isCompleted) + ))) switch outcome { case .cancelled(let results): @@ -436,7 +446,8 @@ extension QueryExecutionCoordinator { route: DatabaseManager.shared.executionRoute(for: scope), cancellation: .cancellableRead(lease) ) { driver in - let sessionPlan = plan.joining(await driver.heldSessionTransactionState()) + let startState = await driver.heldSessionTransactionState() + let sessionPlan = plan.joining(startState) let outcome = await BatchStatementRun.run( prepared, plan: sessionPlan, @@ -457,24 +468,30 @@ extension QueryExecutionCoordinator { } } guard sessionPlan == .sessionTransaction else { - return MultiStatementRun(outcome: outcome, plan: sessionPlan, sessionState: .idle) + return MultiStatementRun( + outcome: outcome, plan: sessionPlan, sessionState: .idle, startState: startState + ) } return MultiStatementRun( outcome: outcome, plan: sessionPlan, - sessionState: await driver.heldSessionTransactionState() + sessionState: await driver.heldSessionTransactionState(), + startState: startState ) } run.failureOutput = failureOutput.output return run } catch { if DatabaseCancellationDiagnosis.isCancellation(error) || Task.isCancelled { - return MultiStatementRun(outcome: .cancelled(results: []), plan: plan, sessionState: .unknown) + return MultiStatementRun( + outcome: .cancelled(results: []), plan: plan, sessionState: .unknown, startState: .unknown + ) } return MultiStatementRun( outcome: .failed(results: [], failure: .connection, errorDescription: error.localizedDescription), plan: plan, - sessionState: .unknown + sessionState: .unknown, + startState: .unknown ) } } diff --git a/TablePro/Core/Database/Access/DatabaseAccessBridge+Scripts.swift b/TablePro/Core/Database/Access/DatabaseAccessBridge+Scripts.swift index 91d447414a..163483993f 100644 --- a/TablePro/Core/Database/Access/DatabaseAccessBridge+Scripts.swift +++ b/TablePro/Core/Database/Access/DatabaseAccessBridge+Scripts.swift @@ -4,6 +4,7 @@ // import Foundation +import os import TableProPluginKit import TableProSQLGrammar @@ -115,6 +116,7 @@ extension DatabaseAccessBridge { databaseType: databaseType ) + let progress = ScriptBatchProgress() let run: ScriptBatchRun do { run = try await runRacingTimeout( @@ -124,18 +126,67 @@ extension DatabaseAccessBridge { owner: owner, timeoutSeconds: timeoutSeconds ) { driver in - try await ScriptBatchRun.run(batches, startLines: startLines, rowCap: rowCap, driver: driver) + try await ScriptBatchRun.run( + batches, startLines: startLines, rowCap: rowCap, driver: driver, progress: progress + ) } } catch { if classification.tier != .safe { CatalogChangeService.post(statementsRan) } + Self.postSucceededBatches(of: batches, progress: progress, scope: scope, databaseType: databaseType) throw error } CatalogChangeService.post(statementsRan) + Self.postSucceededBatches(of: batches, progress: progress, scope: scope, databaseType: databaseType) return run.outcome(executionTimeMs: (CFAbsoluteTimeGetCurrent() - startTime) * 1_000) } + + /// A batch that finished before a later one failed has already committed on SQL Server, whose + /// scripts run with no transaction of the app's around them, so what it dropped or renamed is + /// reported whatever became of the rest. + private static func postSucceededBatches( + of batches: [ExecutableBatch], + progress: ScriptBatchProgress, + scope: DatabaseScope, + databaseType: DatabaseType + ) { + let completed = progress.completedBatchCount + guard completed > 0 else { return } + CatalogChangeService.post(.statementsSucceeded(SucceededStatements( + scope: scope, + databaseType: databaseType, + statements: batches.prefix(completed).flatMap { $0.statements.map(\.sql) }, + commit: .runStartedIn(progress.startState, appTransaction: .none) + ))) + } +} + +/// How far a script got, readable after it threw. +final class ScriptBatchProgress: Sendable { + private struct State { + var startState: PluginSessionTransactionState = .unknown + var completedBatchCount = 0 + } + + private let state = OSAllocatedUnfairLock(initialState: State()) + + var startState: PluginSessionTransactionState { + state.withLock { $0.startState } + } + + var completedBatchCount: Int { + state.withLock { $0.completedBatchCount } + } + + func begin(in startState: PluginSessionTransactionState) { + state.withLock { $0.startState = startState } + } + + func completeBatch() { + state.withLock { $0.completedBatchCount += 1 } + } } /// What a script's batches answered with, and what the session held once they had all run. @@ -150,9 +201,12 @@ struct ScriptBatchRun: Sendable { _ batches: [ExecutableBatch], startLines: [Int], rowCap: Int, - driver: DatabaseDriver + driver: DatabaseDriver, + progress: ScriptBatchProgress ) async throws -> ScriptBatchRun { - let plan = BatchTransactionPlan.autocommit.joining(await driver.heldSessionTransactionState()) + let startState = await driver.heldSessionTransactionState() + progress.begin(in: startState) + let plan = BatchTransactionPlan.autocommit.joining(startState) var answers: [QueryBatchResult] = [] for (batch, startLine) in zip(batches, startLines) { try Task.checkCancellation() @@ -170,6 +224,7 @@ struct ScriptBatchRun: Sendable { ) throw DatabaseError.queryFailed(context.report().message) } + progress.completeBatch() } return ScriptBatchRun(answers: answers, sessionState: await driver.heldSessionTransactionState()) } diff --git a/TablePro/Core/Database/Access/DatabaseAccessBridge.swift b/TablePro/Core/Database/Access/DatabaseAccessBridge.swift index c262b16b59..19e60b76f8 100644 --- a/TablePro/Core/Database/Access/DatabaseAccessBridge.swift +++ b/TablePro/Core/Database/Access/DatabaseAccessBridge.swift @@ -217,23 +217,31 @@ internal actor DatabaseAccessBridge { /// A write that timed out or failed may still have committed: a group only returns once /// every child has, so by the time the error arrives the driver call has finished one way or /// the other, and a catalog that might have changed is refreshed rather than trusted. + let grammar = databaseType.lexicalGrammar let result: QueryResult + let tableEdits: SucceededStatements? do { - result = try await runRacingTimeout( + (result, tableEdits) = try await runRacingTimeout( scope: scope, route: route, policy: policy, owner: owner, timeoutSeconds: timeoutSeconds ) { driver in + let answer: QueryResult if shouldCap { - return try await driver.executeUserQuery( + answer = try await driver.executeUserQuery( query: statement.sql, rowCap: statement.rowCap ?? maxRows, parameters: nil ) + } else { + answer = try await driver.execute(query: normalizedQuery) } - return try await driver.execute(query: normalizedQuery) + let edits = await SucceededStatements.single( + normalizedQuery, scope: scope, databaseType: databaseType, grammar: grammar, ranOn: driver + ) + return (answer, edits) } } catch { if classification.tier != .safe { @@ -243,6 +251,9 @@ internal actor DatabaseAccessBridge { } CatalogChangeService.post(statementRan) + if let tableEdits { + CatalogChangeService.post(.statementsSucceeded(tableEdits)) + } return StatementOutcome(result: result, executionTimeMs: (CFAbsoluteTimeGetCurrent() - startTime) * 1_000) } diff --git a/TablePro/Core/Events/CatalogChange.swift b/TablePro/Core/Events/CatalogChange.swift index cbc1891393..e9bdecff7f 100644 --- a/TablePro/Core/Events/CatalogChange.swift +++ b/TablePro/Core/Events/CatalogChange.swift @@ -64,6 +64,7 @@ struct CatalogChange: Sendable, Equatable { /// Something that happened to a connection's catalog, as the caller that caused it knows it. enum CatalogEvent: Sendable { case statementsRan(connectionId: UUID, statements: [String], databaseType: DatabaseType) + case statementsSucceeded(SucceededStatements) case transactionEnded(connectionId: UUID) case tablesDropped([DatabaseTreeTableRef], connectionId: UUID) case tableRenamed(DatabaseTreeTableRef, to: String, connectionId: UUID) @@ -80,6 +81,8 @@ enum CatalogEvent: Sendable { .containerDropped(_, let connectionId), .containerRenamed(_, _, let connectionId): return connectionId + case .statementsSucceeded(let succeeded): + return succeeded.scope.connectionId case .changed(let change): return change.connectionId } diff --git a/TablePro/Core/Events/SucceededStatements.swift b/TablePro/Core/Events/SucceededStatements.swift new file mode 100644 index 0000000000..046e942d4b --- /dev/null +++ b/TablePro/Core/Events/SucceededStatements.swift @@ -0,0 +1,83 @@ +// +// SucceededStatements.swift +// TablePro +// + +import Foundation +import TableProPluginKit +import TableProSQLGrammar + +/// What became of a transaction the app opened around a run. +enum AppTransactionOutcome: Sendable, Equatable { + /// The app opened none. + case none + /// The app opened one and its `COMMIT` answered. A `ROLLBACK` in the script's own text can + /// still have ended it earlier, which is why the edits inside it wait for that `COMMIT`. + case committed + case rolledBack +} + +/// What the session said about committing statements that ran without an error. +enum StatementCommitEvidence: Sendable, Equatable { + /// One statement, and what the session held once it had run. + case statementLeftSession(PluginSessionTransactionState) + /// Several statements, what the session held before the first of them, and what became of a + /// transaction the app opened around them. + case runStartedIn(PluginSessionTransactionState, appTransaction: AppTransactionOutcome) + + static func run( + startedIn state: PluginSessionTransactionState, + plan: BatchTransactionPlan, + completed: Bool + ) -> StatementCommitEvidence { + guard plan.opensTransaction else { return .runStartedIn(state, appTransaction: .none) } + return .runStartedIn(state, appTransaction: completed ? .committed : .rolledBack) + } +} + +/// SQL a user or an MCP client ran, the statements of it that succeeded, in order, and the scope +/// they started in. +/// +/// Kept apart from `CatalogEvent.statementsRan`, which reports a failed statement as well because +/// a refresh errs toward running. What this one carries is acted on as a drop or a rename, so it +/// holds only what the server accepted. +struct SucceededStatements: Sendable, Equatable { + let scope: DatabaseScope + let databaseType: DatabaseType + let statements: [String] + let commit: StatementCommitEvidence + + /// A statement that ran alone, when it drops or renames a table, or changes how a name + /// resolves. The session is asked what it holds only where the answer decides anything, which + /// an engine that commits DDL as it runs never needs. + static func single( + _ sql: String, + scope: DatabaseScope, + databaseType: DatabaseType, + grammar: SQLLexicalGrammar, + ranOn driver: DatabaseDriver + ) async -> SucceededStatements? { + guard let dialect = TableEditDialect.of(databaseType) else { return nil } + let statement = TableEditStatementParser.parse(sql, dialect: dialect, grammar: grammar) + guard statement.editsTable || statement.changesNameHazards else { return nil } + let asksSession = statement.editsTable && !dialect.commitsDDLImplicitly + let state: PluginSessionTransactionState = asksSession ? await driver.heldSessionTransactionState() : .unknown + return SucceededStatements( + scope: scope, databaseType: databaseType, statements: [sql], commit: .statementLeftSession(state) + ) + } +} + +extension PluginSessionTransactionState { + /// Whether whatever ran before this answer has been committed. + var holdsNoTransaction: Bool { + switch self { + case .idle, .holdsSessionLocks: + return true + case .inTransaction, .abortedTransaction, .unknown: + return false + @unknown default: + return false + } + } +} diff --git a/TablePro/Core/Services/Execution/BatchStatementRun.swift b/TablePro/Core/Services/Execution/BatchStatementRun.swift index 09183da87e..d9b5202280 100644 --- a/TablePro/Core/Services/Execution/BatchStatementRun.swift +++ b/TablePro/Core/Services/Execution/BatchStatementRun.swift @@ -26,6 +26,26 @@ internal enum BatchStatementOutcome { extension BatchStatementOutcome: Sendable where Output: Sendable {} +internal extension BatchStatementOutcome { + /// How many of the run's statements or batches, from the first, ran to the end without an + /// error. A batch that answered with a server error is among the results but is not one of them. + var succeededCount: Int { + switch self { + case .completed(let results), .cancelled(let results): + return results.count + case .failed(let results, .batch, _): + return max(results.count - 1, 0) + case .failed(let results, _, _): + return results.count + } + } + + var isCompleted: Bool { + guard case .completed = self else { return false } + return true + } +} + /// The driver calls one multi-statement run makes, in order, for one ``BatchTransactionPlan``. /// /// Extracted from the coordinator so the order is testable without a window, a tab or a server: diff --git a/TablePro/Core/Services/Query/CatalogChangeService.swift b/TablePro/Core/Services/Query/CatalogChangeService.swift index 26986b7ad1..cef96020fd 100644 --- a/TablePro/Core/Services/Query/CatalogChangeService.swift +++ b/TablePro/Core/Services/Query/CatalogChangeService.swift @@ -6,6 +6,7 @@ import Combine import Foundation import os +import TableProSQLGrammar /// A store that describes some part of a connection's catalog and can bring that part up to date. @MainActor @@ -38,6 +39,7 @@ final class CatalogChangeService { private let isSessionLive: @MainActor (UUID) -> Bool private var pendingChanges: [UUID: CatalogChange] = [:] + private var nameHazards: [UUID: TableNameHazards] = [:] private var drains: [UUID: Task] = [:] init( @@ -84,6 +86,8 @@ final class CatalogChangeService { switch event { case .statementsRan(_, let statements, let databaseType): recordStatements(statements, databaseType: databaseType, connectionId: connectionId) + case .statementsSucceeded(let succeeded): + recordCommittedTableEdits(of: succeeded) case .transactionEnded: schedule(CatalogChange(connectionId: connectionId, kinds: Self.transactionEndKinds)) case .tablesDropped(let refs, _): @@ -114,6 +118,7 @@ final class CatalogChangeService { } private func recordStatements(_ statements: [String], databaseType: DatabaseType, connectionId: UUID) { + recordNameHazards(of: statements, databaseType: databaseType, connectionId: connectionId) let effect = CatalogChangeClassifier.effect(ofStatements: statements, databaseType: databaseType) var kinds = effect.kinds if effect.endsTransaction { @@ -123,6 +128,48 @@ final class CatalogChangeService { schedule(CatalogChange(connectionId: connectionId, kinds: kinds)) } + /// A table dropped or renamed by SQL the user or an MCP client ran is adopted exactly as the + /// sidebar's own Drop and Rename are, one edit at a time in the order they ran, so a rename + /// chain and a drop of a name another statement just freed both land on the right table. + private func recordCommittedTableEdits(of succeeded: SucceededStatements) { + let connectionId = succeeded.scope.connectionId + let grammar = SQLLexicalResolver.executionGrammar(for: succeeded.databaseType, connectionId: connectionId) + var hazards = nameHazards[connectionId] ?? TableNameHazards() + let edits = CommittedTableEdits.edits(in: succeeded, grammar: grammar, hazards: &hazards) + nameHazards[connectionId] = hazards + guard !edits.isEmpty else { return } + Self.logger.debug( + "[catalog] adopting \(edits.count) table edit(s) from SQL connId=\(connectionId, privacy: .public)" + ) + for edit in edits { + switch edit { + case .dropped(let table, let kind): + recordDroppedTables( + [adoption.tableRef(for: table, kind: kind, connectionId: connectionId)], + connectionId: connectionId + ) + case .renamed(let table, let newName, let kind): + recordRenamedTable( + adoption.tableRef(for: table, kind: kind, connectionId: connectionId), + to: newName, + connectionId: connectionId + ) + } + } + } + + /// Read from every statement that may have run, failed ones included, because a procedure that + /// failed part way can have created a temporary table first. + private func recordNameHazards(of statements: [String], databaseType: DatabaseType, connectionId: UUID) { + guard let dialect = TableEditDialect.of(databaseType) else { return } + let grammar = SQLLexicalResolver.executionGrammar(for: databaseType, connectionId: connectionId) + var hazards = nameHazards[connectionId] ?? TableNameHazards() + for statement in statements { + hazards.record(TableEditStatementParser.parse(statement, dialect: dialect, grammar: grammar), dialect: dialect) + } + nameHazards[connectionId] = hazards + } + private func recordDroppedTables(_ refs: [DatabaseTreeTableRef], connectionId: UUID) { guard !refs.isEmpty else { return } adoption.adoptDroppedTables(refs, connectionId: connectionId) diff --git a/TablePro/Core/Services/Query/CatalogEditAdoption.swift b/TablePro/Core/Services/Query/CatalogEditAdoption.swift index d71e2326e2..3bfa8516d1 100644 --- a/TablePro/Core/Services/Query/CatalogEditAdoption.swift +++ b/TablePro/Core/Services/Query/CatalogEditAdoption.swift @@ -44,17 +44,23 @@ struct CatalogEditAdoption { private let schemaService: SchemaService private let connectionStorage: ConnectionStorage private let appSettings: AppSettingsStorage + private let settingsStores: [any TableScopedSettingsStore] + private let favoriteTables: FavoriteTablesStorage init( databaseManager: DatabaseManager = .shared, schemaService: SchemaService = .shared, connectionStorage: ConnectionStorage = .shared, - appSettings: AppSettingsStorage = .shared + appSettings: AppSettingsStorage = .shared, + settingsStores: [any TableScopedSettingsStore]? = nil, + favoriteTables: FavoriteTablesStorage = .shared ) { self.databaseManager = databaseManager self.schemaService = schemaService self.connectionStorage = connectionStorage self.appSettings = appSettings + self.settingsStores = settingsStores ?? TableScopedSettingsRegistry.stores + self.favoriteTables = favoriteTables } /// Where the object lives. A reference without a database means the one being browsed, and the @@ -73,20 +79,21 @@ struct CatalogEditAdoption { /// Left behind, they outlive the table and come back on a table that is recreated with the same /// name: a filter on a column the new table does not have opens the tab on a server error. func adoptDroppedTables(_ refs: [DatabaseTreeTableRef], connectionId: UUID) { - let dropped = Set(refs) - updatePendingOperations(connectionId: connectionId) { dropped.contains($0) ? nil : $0 } + let dropped = Set(refs.compactMap { tableScope(for: $0, connectionId: connectionId) }) + updatePendingOperations(connectionId: connectionId) { ref in + guard let identity = tableScope(for: ref, connectionId: connectionId), + dropped.contains(identity) else { return ref } + return nil + } let sidebarState = SharedSidebarState.forConnection(connectionId) for ref in refs { sidebarState.removeRecentTable(database: ref.database, schema: ref.schema, name: ref.table.name) - FavoriteTablesStorage.shared.removeFavorite( + favoriteTables.removeFavorite( name: ref.table.name, schema: ref.favoriteSchema, database: ref.database, connectionId: connectionId ) - guard let scope = objectScope(for: ref, connectionId: connectionId) else { continue } - let tableScope = TableScope( - connectionId: connectionId, database: scope.database, schema: scope.schema, table: ref.table.name - ) - for store in TableScopedSettingsRegistry.stores { - store.dropTable(tableScope) + guard let droppedScope = tableScope(for: ref, connectionId: connectionId) else { continue } + for store in settingsStores { + store.dropTable(droppedScope) } } } @@ -95,17 +102,46 @@ struct CatalogEditAdoption { /// that name, reach the wrong object. It is dropped rather than moved, because the confirmation /// the user gave named the object they were looking at. func adoptTableRename(_ ref: DatabaseTreeTableRef, to newName: String, connectionId: UUID) { - guard let scope = objectScope(for: ref, connectionId: connectionId) else { return } - let oldScope = TableScope(connectionId: connectionId, database: scope.database, schema: scope.schema, table: ref.table.name) - let newScope = TableScope(connectionId: connectionId, database: scope.database, schema: scope.schema, table: newName) - for store in TableScopedSettingsRegistry.stores { + guard let oldScope = tableScope(for: ref, connectionId: connectionId) else { return } + let newScope = TableScope( + connectionId: connectionId, database: oldScope.database, schema: oldScope.schema, table: newName + ) + for store in settingsStores { store.renameTable(from: oldScope, to: newScope) } moveFavorite(ref, to: newName, connectionId: connectionId) SharedSidebarState.forConnection(connectionId).renameRecentTable( database: ref.database, schema: ref.schema, from: ref.table.name, to: newName ) - updatePendingOperations(connectionId: connectionId) { $0 == ref ? nil : $0 } + updatePendingOperations(connectionId: connectionId) { queued in + tableScope(for: queued, connectionId: connectionId) == oldScope ? nil : queued + } + } + + /// The object a reference names, which is what a queued operation and the saved settings are + /// matched on. Two references to one table can differ in how their `TableInfo` spells its + /// schema, and a table named by SQL carries no row the sidebar built at all. + private func tableScope(for ref: DatabaseTreeTableRef, connectionId: UUID) -> TableScope? { + guard let scope = objectScope(for: ref, connectionId: connectionId) else { return nil } + return TableScope(connectionId: connectionId, database: scope.database, schema: scope.schema, table: ref.table.name) + } + + /// A table a statement named, as a reference spelled the way the sidebar spells its row, so + /// the favorite and the Recent entry it keys are the ones found. The sidebar takes a favorite's + /// schema from the driver's listing, which Oracle, MySQL and SQLite leave empty, so an entry + /// already saved without one is matched in that spelling. + func tableRef(for table: TablePlacement, kind: TableInfo.TableType, connectionId: UUID) -> DatabaseTreeTableRef { + let saved = favoriteTables.favorites(for: connectionId).filter { + $0.name == table.name && $0.database == table.database.nilIfEmpty + } + let listsWithoutSchema = !saved.contains { $0.schema == table.schema } && saved.contains { $0.schema == nil } + return DatabaseTreeTableRef( + database: table.database, + schema: table.schema, + table: TableInfo( + name: table.name, type: kind, rowCount: nil, schema: listsWithoutSchema ? nil : table.schema + ) + ) } func adoptContainerRename(_ container: DatabaseContainerRef, to newName: String, connectionId: UUID) { @@ -148,10 +184,10 @@ struct CatalogEditAdoption { /// Every table inside the container loses its saved settings and its favorite, for the /// reason a dropped table does. Swept by prefix rather than by table, because the table /// list is lazy and a table nobody opened this session still has settings on disk. - for store in TableScopedSettingsRegistry.stores { + for store in settingsStores { store.dropContainer(connectionId: connectionId, database: database, schema: schema) } - FavoriteTablesStorage.shared.removeFavorites( + favoriteTables.removeFavorites( inDatabase: database, schema: schema, connectionId: connectionId ) @@ -235,7 +271,7 @@ struct CatalogEditAdoption { /// hierarchical tree, where the schema hangs on the node and not on the `TableInfo`, so a /// renamed table silently lost its star. private func moveFavorite(_ ref: DatabaseTreeTableRef, to newName: String, connectionId: UUID) { - let storage = FavoriteTablesStorage.shared + let storage = favoriteTables let schema = ref.favoriteSchema guard storage.isFavorite( name: ref.table.name, schema: schema, database: ref.database, connectionId: connectionId @@ -260,13 +296,13 @@ struct CatalogEditAdoption { table: ref.table ) } - for store in TableScopedSettingsRegistry.stores { + for store in settingsStores { store.renameContainer( connectionId: connectionId, fromDatabase: database, fromSchema: schema, toDatabase: toDatabase, toSchema: toSchema ) } - let storage = FavoriteTablesStorage.shared + let storage = favoriteTables for entry in storage.favorites(for: connectionId) where entry.database == database { if let schema, entry.schema != schema { continue } storage.removeFavorite( diff --git a/TablePro/Core/Services/Query/CommittedTableEdits.swift b/TablePro/Core/Services/Query/CommittedTableEdits.swift new file mode 100644 index 0000000000..aa0a4e44a7 --- /dev/null +++ b/TablePro/Core/Services/Query/CommittedTableEdits.swift @@ -0,0 +1,196 @@ +// +// CommittedTableEdits.swift +// TablePro +// + +import Foundation +import TableProPluginKit +import TableProSQLGrammar + +/// A table that a committed statement dropped or renamed, placed where it lived. +enum TableCatalogEdit: Equatable, Sendable { + case dropped(TablePlacement, kind: TableInfo.TableType) + case renamed(TablePlacement, to: String, kind: TableInfo.TableType) +} + +/// What a connection's own SQL has done that can make a name resolve somewhere other than the +/// scope a tab records: a temporary table that shadows a real one, a schema moved by hand, or code +/// the server ran that the text does not show. +/// +/// It only ever grows. Taking an entry back needs knowing that the temporary table is gone from +/// every session the connection runs SQL on, including one a rolled-back `DROP` left it in, and a +/// wrong entry only keeps a real table's settings where they are. +struct TableNameHazards: Sendable, Equatable { + /// Lowercased, because SQLite, and MySQL on a case-insensitive file system, find a temporary + /// `People` under `people`. Matching more loosely than an engine does only skips more. + var temporaryNames: Set = [] + var namesMayBeShadowed = false + + /// Records what a statement that ran, whether or not it succeeded, did to how names resolve. A + /// failed procedure can have created a temporary table before it failed. + mutating func record(_ statement: TableEditStatement, dialect: TableEditDialect) { + switch statement { + case .createsTemporaryTable(let name): + guard dialect.temporaryTablesShadowRealOnes, + let table = name.parts.last.flatMap(dialect.folded) else { return } + temporaryNames.insert(table.lowercased()) + case .losesSchemaContext, .runsUnseenCode: + namesMayBeShadowed = true + case .drop, .rename, .beginsTransaction, .commits, .rollsBack, .rollsBackToSavepoint, + .losesTransactionTracking, .selectsDatabase, .other: + break + } + } + + /// Whether `name`, placed as `table`, may be something other than the table the app keeps + /// settings for. + func mayShadow(_ name: SQLObjectName, placedAs table: TablePlacement, dialect: TableEditDialect) -> Bool { + guard name.parts.count == 1 || dialect.temporaryTablesShadowQualifiedNames else { return false } + return namesMayBeShadowed || temporaryNames.contains(table.name.lowercased()) + } +} + +/// The drops and renames among statements that succeeded which are also committed, in the order +/// they ran. +/// +/// Success is not enough on an engine whose DDL is transactional: `BEGIN; DROP TABLE people; +/// ROLLBACK` succeeds three times and leaves the table where it was. So the text's own `BEGIN`, +/// `COMMIT` and `ROLLBACK` are followed from a session known to hold no transaction, and an edit +/// still inside one when the statements end is dropped, because nothing here will see how it ends. +/// A run that began inside a transaction, or on a session that could not say, adopts nothing. +enum CommittedTableEdits { + static func edits( + in succeeded: SucceededStatements, + grammar: SQLLexicalGrammar, + hazards: inout TableNameHazards + ) -> [TableCatalogEdit] { + guard let dialect = TableEditDialect.of(succeeded.databaseType) else { return [] } + var walk = TableEditWalk(dialect: dialect, scope: succeeded.scope, commit: succeeded.commit, hazards: hazards) + for statement in succeeded.statements { + walk.read(TableEditStatementParser.parse(statement, dialect: dialect, grammar: grammar)) + } + walk.finish(succeeded.commit) + hazards = walk.hazards + return walk.committed + } +} + +private struct TableEditWalk { + private let dialect: TableEditDialect + private var context: TableNameContext + /// Transaction nesting the text opened. A `COMMIT` commits only once it is back at zero, which + /// is SQL Server's `@@TRANCOUNT`; on engines where one `COMMIT` ends everything this can only + /// hold an edit back, never let one through early. + private var depth = 0 + /// False when the session held a transaction the text cannot see the end of, in which case + /// the walk only keeps the hazards current. + private let adopts: Bool + private var tracksTransactions = true + private var pending: [TableCatalogEdit] = [] + private(set) var committed: [TableCatalogEdit] = [] + private(set) var hazards: TableNameHazards + + init(dialect: TableEditDialect, scope: DatabaseScope, commit: StatementCommitEvidence, hazards: TableNameHazards) { + self.dialect = dialect + context = TableNameContext(database: scope.database.nilIfEmpty, schema: scope.schema) + adopts = Self.startsOutsideTransactions(commit, dialect: dialect) + self.hazards = hazards + if case .runStartedIn(_, .committed) = commit { + depth = 1 + } + } + + private static func startsOutsideTransactions(_ evidence: StatementCommitEvidence, dialect: TableEditDialect) -> Bool { + guard !dialect.commitsDDLImplicitly else { return true } + switch evidence { + case .statementLeftSession(let state): + return state.holdsNoTransaction + case .runStartedIn(let state, let appTransaction): + return state.holdsNoTransaction && appTransaction != .rolledBack + } + } + + /// The app's own `COMMIT`, which closes the transaction the walk opened for it, unless a + /// `ROLLBACK` or `COMMIT` in the text already ended it. + mutating func finish(_ evidence: StatementCommitEvidence) { + guard case .runStartedIn(_, .committed) = evidence else { return } + read(.commits) + } + + mutating func read(_ statement: TableEditStatement) { + hazards.record(statement, dialect: dialect) + switch statement { + case .drop(let names, let kind): + for name in names { + guard let table = place(name) else { continue } + record(.dropped(table, kind: kind)) + } + case .rename(let pairs, let kind): + readRenames(pairs, kind: kind) + case .beginsTransaction: + depth += 1 + case .commits: + guard depth > 0 else { return } + depth -= 1 + guard depth == 0 else { return } + committed += pending + pending.removeAll() + case .rollsBack: + pending.removeAll() + depth = 0 + case .rollsBackToSavepoint: + pending.removeAll() + case .losesTransactionTracking: + pending.removeAll() + tracksTransactions = false + case .selectsDatabase(let name): + context = dialect.context(afterUsing: name) + case .losesSchemaContext: + context.schema = nil + case .createsTemporaryTable, .runsUnseenCode, .other: + break + } + } + + private func place(_ name: SQLObjectName) -> TablePlacement? { + guard let table = dialect.resolve(name, in: context), + !hazards.mayShadow(name, placedAs: table, dialect: dialect) else { return nil } + return table + } + + /// Every pair or none: a chain such as `a TO tmp, b TO a, tmp TO b` applied in part would move + /// one table's settings onto another. A rename that moves the table to another database or + /// schema is left alone too, since the sidebar's own rename never does that. + private mutating func readRenames(_ pairs: [SQLRenamePair], kind: TableInfo.TableType) { + var edits: [TableCatalogEdit] = [] + for pair in pairs { + guard let source = place(pair.from), + let target = renameTarget(pair.to, of: source), + target.container == source.container else { return } + edits.append(.renamed(source, to: target.name, kind: kind)) + } + for edit in edits { + record(edit) + } + } + + private func renameTarget(_ target: SQLObjectName, of source: TablePlacement) -> TablePlacement? { + guard dialect.renameKeepsContainer else { return dialect.resolve(target, in: context) } + guard target.parts.count == 1, let name = dialect.folded(target.parts[0]) else { return nil } + return TablePlacement(database: source.database, schema: source.schema, name: name) + } + + private mutating func record(_ edit: TableCatalogEdit) { + guard adopts else { return } + if dialect.commitsDDLImplicitly { + committed.append(edit) + return + } + guard tracksTransactions else { return } + if depth == 0 { + committed.append(edit) + } else { + pending.append(edit) + } + } +} diff --git a/TablePro/Core/Utilities/SQL/SQLTokenCursor.swift b/TablePro/Core/Utilities/SQL/SQLTokenCursor.swift index 9e1319b97b..e0fd269825 100644 --- a/TablePro/Core/Utilities/SQL/SQLTokenCursor.swift +++ b/TablePro/Core/Utilities/SQL/SQLTokenCursor.swift @@ -46,8 +46,16 @@ internal struct SQLTokenCursor { internal private(set) var parenDepth = 0 + /// Where the token `next()` returned last sits in the text, for a rule that needs a word as it + /// was written rather than uppercased. + internal private(set) var lastTokenRange = NSRange(location: 0, length: 0) + internal var location: Int { index } + internal var lastTokenText: String { + text.substring(with: lastTokenRange) + } + internal init(_ text: NSString, grammar: SQLLexicalGrammar) { self.text = text self.grammar = grammar @@ -68,7 +76,10 @@ internal struct SQLTokenCursor { } if skipsNonCode(character) { continue } if character == SqlLexer.semicolon, parenDepth == 0 { return nil } - return token(startingWith: character) + let start = index + let token = token(startingWith: character) + lastTokenRange = NSRange(location: start, length: index - start) + return token } return nil } diff --git a/TablePro/Core/Utilities/SQL/TableEditDialect.swift b/TablePro/Core/Utilities/SQL/TableEditDialect.swift new file mode 100644 index 0000000000..290880b7b4 --- /dev/null +++ b/TablePro/Core/Utilities/SQL/TableEditDialect.swift @@ -0,0 +1,221 @@ +// +// TableEditDialect.swift +// TablePro +// + +import Foundation + +/// How one engine names a table in `DROP` and rename statements, and whether such a statement is +/// final the moment it succeeds. +/// +/// Only engines whose rules are known are covered. Everything else answers nil and a statement +/// run on it changes nothing the app keeps about its tables, because a name resolved the wrong way +/// would take one table's saved settings from another that still exists. +struct TableEditDialect: Sendable, Equatable { + enum Folding: Sendable, Equatable { + case lowercase + case uppercase + case preserve + } + + /// What the first part of a two-part name is. + enum TwoPartQualifier: Sendable, Equatable { + case schema + case database + /// The engine tries more than one reading, so the name cannot be placed. + case ambiguous + } + + let folding: Folding + /// Whether the app's scope keys a table by schema as well as by database. + let keysBySchema: Bool + /// Whether a bare name can be placed in the scope the statement ran in. SQL Server resolves one + /// against the login's default schema, which the app does not track. PostgreSQL and DuckDB + /// resolve one through a search path and a temporary namespace that a function, `set_config` + /// or `SELECT ... INTO TEMP` can change without the text saying so, and the driver re-pins the + /// schema only when its own record of it differs. On all three only a qualified name is placed. + let resolvesUnqualifiedNames: Bool + let twoPartQualifier: TwoPartQualifier + let acceptsDatabaseSchemaTable: Bool + /// MySQL, MariaDB, Oracle and ClickHouse commit DDL as it runs, so a `ROLLBACK` cannot bring a + /// dropped table back there. + let commitsDDLImplicitly: Bool + let endCommits: Bool + /// `USE x` moves the connection onto database `x`, rather than onto a catalog or a schema that + /// a single name cannot tell apart. + let useSelectsDatabase: Bool + /// PostgreSQL, SQLite, Oracle, DuckDB and SQL Server's `sp_rename` keep a renamed table in its + /// schema and take a bare new name. MySQL and ClickHouse resolve the new name like any other, + /// which can move the table. + let renameKeepsContainer: Bool + /// MySQL's `ALTER TABLE a RENAME b`, with neither `TO` nor `AS`. + let renamesWithoutTo: Bool + /// A MySQL temporary table hides the real one under its qualified name too, so `DROP TABLE + /// shop.people` drops the temporary `people` created in `shop`. + let temporaryTablesShadowQualifiedNames: Bool + /// Whether a temporary table here shadows a real one of the same name. An Oracle global + /// temporary table is a permanent object with temporary rows, and shadows nothing. + let temporaryTablesShadowRealOnes: Bool + /// The containers that hold a session's own temporary objects, `pg_temp` and `pg_temp_3` or + /// `temp`. A name inside one is never a table the app keeps settings for. + let temporaryContainers: Set + + static func of(_ type: DatabaseType) -> TableEditDialect? { + if type == .clickhouse { return .clickHouse } + switch TransactionEngineFamily.of(type) { + case .postgres, .redshift: + return .postgreSQL + case .mysql: + return .mySQL + case .sqlite: + return .sqlite + case .duckdb: + return .duckDB + case .sqlServer: + return .sqlServer + case .oracle: + return .oracle + case .cockroach, .redis, .other: + return nil + } + } + + /// A bare word holding `@` is a variable or an Oracle database link, never a table here. + func folded(_ part: SQLNamePart) -> String? { + guard !part.text.isEmpty else { return nil } + guard !part.isQuoted else { return part.text } + guard !part.text.contains("@") else { return nil } + switch folding { + case .preserve: + return part.text + case .lowercase, .uppercase: + /// A server folds only ASCII letters for certain; what it does with the rest depends on + /// its encoding and locale, so such a name is left unplaced. + guard part.text.unicodeScalars.allSatisfy(\.isASCII) else { return nil } + return folding == .lowercase ? part.text.lowercased() : part.text.uppercased() + } + } + + func namesTemporaryContainer(_ container: String) -> Bool { + let lowered = container.lowercased() + return temporaryContainers.contains { base in + guard lowered != base else { return true } + guard lowered.hasPrefix(base + "_") else { return false } + let suffix = lowered.dropFirst(base.count + 1) + return !suffix.isEmpty && suffix.allSatisfy(\.isNumber) + } + } + + func resolve(_ name: SQLObjectName, in context: TableNameContext) -> TablePlacement? { + let parts = name.parts.compactMap(folded) + guard parts.count == name.parts.count, let table = parts.last else { return nil } + guard !parts.dropLast().contains(where: namesTemporaryContainer) else { return nil } + switch parts.count { + case 1: + guard resolvesUnqualifiedNames, let database = context.database else { return nil } + guard keysBySchema else { return TablePlacement(database: database, schema: nil, name: table) } + guard let schema = context.schema else { return nil } + return TablePlacement(database: database, schema: schema, name: table) + case 2: + switch twoPartQualifier { + case .schema: + guard let database = context.database else { return nil } + return TablePlacement(database: database, schema: parts[0], name: table) + case .database: + return TablePlacement(database: parts[0], schema: nil, name: table) + case .ambiguous: + return nil + } + case 3: + guard acceptsDatabaseSchemaTable else { return nil } + return TablePlacement(database: parts[0], schema: parts[1], name: table) + default: + return nil + } + } + + func context(afterUsing name: SQLObjectName?) -> TableNameContext { + guard useSelectsDatabase, let name, name.parts.count == 1, let database = folded(name.parts[0]) else { + return TableNameContext(database: nil, schema: nil) + } + return TableNameContext(database: database, schema: nil) + } + + static let postgreSQL = TableEditDialect( + folding: .lowercase, keysBySchema: true, resolvesUnqualifiedNames: false, twoPartQualifier: .schema, + acceptsDatabaseSchemaTable: true, commitsDDLImplicitly: false, endCommits: true, + useSelectsDatabase: false, renameKeepsContainer: true, renamesWithoutTo: false, + temporaryTablesShadowQualifiedNames: false, temporaryTablesShadowRealOnes: true, + temporaryContainers: ["pg_temp"] + ) + + static let mySQL = TableEditDialect( + folding: .preserve, keysBySchema: false, resolvesUnqualifiedNames: true, twoPartQualifier: .database, + acceptsDatabaseSchemaTable: false, commitsDDLImplicitly: true, endCommits: false, + useSelectsDatabase: true, renameKeepsContainer: false, renamesWithoutTo: true, + temporaryTablesShadowQualifiedNames: true, temporaryTablesShadowRealOnes: true, + temporaryContainers: [] + ) + + static let clickHouse = TableEditDialect( + folding: .preserve, keysBySchema: false, resolvesUnqualifiedNames: true, twoPartQualifier: .database, + acceptsDatabaseSchemaTable: false, commitsDDLImplicitly: true, endCommits: false, + useSelectsDatabase: true, renameKeepsContainer: false, renamesWithoutTo: false, + temporaryTablesShadowQualifiedNames: false, temporaryTablesShadowRealOnes: true, + temporaryContainers: [] + ) + + /// An attached database is a schema to SQLite, and a bare name can resolve into one, so only a + /// bare name is placed, on the connection's own file. + static let sqlite = TableEditDialect( + folding: .preserve, keysBySchema: false, resolvesUnqualifiedNames: true, twoPartQualifier: .ambiguous, + acceptsDatabaseSchemaTable: false, commitsDDLImplicitly: false, endCommits: true, + useSelectsDatabase: false, renameKeepsContainer: true, renamesWithoutTo: false, + temporaryTablesShadowQualifiedNames: false, temporaryTablesShadowRealOnes: true, + temporaryContainers: ["temp"] + ) + + /// `a.b` is a schema in the current catalog or the default schema of catalog `a`, whichever + /// exists, so a two-part name is not placed. + static let duckDB = TableEditDialect( + folding: .preserve, keysBySchema: true, resolvesUnqualifiedNames: false, twoPartQualifier: .ambiguous, + acceptsDatabaseSchemaTable: true, commitsDDLImplicitly: false, endCommits: true, + useSelectsDatabase: false, renameKeepsContainer: true, renamesWithoutTo: false, + temporaryTablesShadowQualifiedNames: false, temporaryTablesShadowRealOnes: true, + temporaryContainers: ["temp"] + ) + + static let sqlServer = TableEditDialect( + folding: .preserve, keysBySchema: true, resolvesUnqualifiedNames: false, twoPartQualifier: .schema, + acceptsDatabaseSchemaTable: true, commitsDDLImplicitly: false, endCommits: false, + useSelectsDatabase: true, renameKeepsContainer: true, renamesWithoutTo: false, + temporaryTablesShadowQualifiedNames: false, temporaryTablesShadowRealOnes: true, + temporaryContainers: ["tempdb"] + ) + + static let oracle = TableEditDialect( + folding: .uppercase, keysBySchema: true, resolvesUnqualifiedNames: true, twoPartQualifier: .schema, + acceptsDatabaseSchemaTable: false, commitsDDLImplicitly: true, endCommits: false, + useSelectsDatabase: false, renameKeepsContainer: true, renamesWithoutTo: false, + temporaryTablesShadowQualifiedNames: false, temporaryTablesShadowRealOnes: false, + temporaryContainers: [] + ) +} + +/// The database and schema a bare name resolves in. Nil means the statements before this one +/// moved it somewhere the text does not say. +struct TableNameContext: Sendable, Equatable { + var database: String? + var schema: String? +} + +/// Where a statement's table lives, in the app's own terms. +struct TablePlacement: Hashable, Sendable { + let database: String + let schema: String? + let name: String + + var container: TableNameContext { + TableNameContext(database: database, schema: schema) + } +} diff --git a/TablePro/Core/Utilities/SQL/TableEditStatementParser.swift b/TablePro/Core/Utilities/SQL/TableEditStatementParser.swift new file mode 100644 index 0000000000..5a17b07acb --- /dev/null +++ b/TablePro/Core/Utilities/SQL/TableEditStatementParser.swift @@ -0,0 +1,386 @@ +// +// TableEditStatementParser.swift +// TablePro +// + +import Foundation +import TableProSQLGrammar + +/// One dot-separated part of a name, as the statement wrote it. +struct SQLNamePart: Equatable, Sendable { + let text: String + let isQuoted: Bool +} + +struct SQLObjectName: Equatable, Sendable { + let parts: [SQLNamePart] +} + +struct SQLRenamePair: Equatable, Sendable { + let from: SQLObjectName + let to: SQLObjectName +} + +/// What one statement means for the tables the app keeps settings about, and for the transaction +/// and the name context of the statements after it. +enum TableEditStatement: Equatable, Sendable { + case drop([SQLObjectName], kind: TableInfo.TableType) + case rename([SQLRenamePair], kind: TableInfo.TableType) + case beginsTransaction + case commits + case rollsBack + case rollsBackToSavepoint + /// `SET IMPLICIT_TRANSACTIONS`, `PREPARE TRANSACTION`, `COMMIT AND CHAIN`: the transaction + /// state after it is no longer something the text shows. + case losesTransactionTracking + /// `USE`, with the name it moved to, or nil when that name could not be read. + case selectsDatabase(SQLObjectName?) + case losesSchemaContext + case createsTemporaryTable(SQLObjectName) + /// A procedure, a prepared statement or an anonymous block: code the server runs that the text + /// does not show, and that can create a temporary table or move the schema. + case runsUnseenCode + case other + + var editsTable: Bool { + switch self { + case .drop, .rename: + return true + case .beginsTransaction, .commits, .rollsBack, .rollsBackToSavepoint, .losesTransactionTracking, + .selectsDatabase, .losesSchemaContext, .createsTemporaryTable, .runsUnseenCode, .other: + return false + } + } + + /// Whether this can leave the session resolving a bare name somewhere other than the scope a + /// tab records, for this run and every later one. + var changesNameHazards: Bool { + switch self { + case .createsTemporaryTable, .losesSchemaContext, .runsUnseenCode: + return true + case .drop, .rename, .beginsTransaction, .commits, .rollsBack, .rollsBackToSavepoint, + .losesTransactionTracking, .selectsDatabase, .other: + return false + } + } +} + +/// Reads the statements that drop or rename a table, and the ones that decide whether such an +/// edit is committed and where a bare name points. +/// +/// A statement is read only when it is exactly one of those forms. Anything left over, such as a +/// column rename, a clause this does not know, or a second command SQL Server runs without a `;`, +/// makes the whole statement `.other`, because a partial reading is a guess. +enum TableEditStatementParser { + /// MySQL skips the body of a `/*!NNNNN ... */` comment on a server older than `NNNNN` and still + /// answers success, so a drop or rename written inside one is not read. What such a comment + /// may create still counts as a hazard, since counting one that never ran only skips more. + static func parse(_ sql: String, dialect: TableEditDialect, grammar: SQLLexicalGrammar) -> TableEditStatement { + let statement = read(sql, dialect: dialect, grammar: grammar) + guard statement.editsTable, grammar.contains(.executableComments), + sql.contains("/*!") || sql.contains("/*M!") else { return statement } + return .other + } + + private static func read(_ sql: String, dialect: TableEditDialect, grammar: SQLLexicalGrammar) -> TableEditStatement { + var reader = Reader(SQLTokenCursor(sql, grammar: grammar)) + guard let keyword = reader.nextWord() else { return .other } + switch keyword { + case "DROP": + return reader.drop() + case "ALTER": + return reader.alter(dialect: dialect) + case "RENAME": + return reader.renameTables() + case "BEGIN": + if grammar.contains(.plsqlBlocks) || reader.opensCompoundStatement() { return .runsUnseenCode } + return SqlBlockStructure.beginStartsTransaction(followedBy: reader.peekWord()) ? .beginsTransaction : .other + case "DECLARE": + return grammar.contains(.plsqlBlocks) ? .runsUnseenCode : .other + case "CALL", "DO": + return .runsUnseenCode + case "START": + return reader.accept("TRANSACTION") ? .beginsTransaction : .other + case "SAVEPOINT": + return .beginsTransaction + case "SAVE": + return reader.accept("TRAN") || reader.accept("TRANSACTION") ? .beginsTransaction : .other + case "COMMIT": + return reader.commit() + case "RELEASE": + return .commits + case "END": + return dialect.endCommits ? reader.commit() : .other + case "ROLLBACK", "ABORT": + return reader.rollback() + case "PREPARE": + return reader.accept("TRANSACTION") ? .losesTransactionTracking : .other + case "SET": + return reader.set() + case "RESET": + return reader.accept("SEARCH_PATH") || reader.accept("ALL") ? .losesSchemaContext : .other + case "DISCARD": + return reader.accept("ALL") ? .losesSchemaContext : .other + case "USE": + return reader.use() + case "EXEC", "EXECUTE": + return reader.accept("AS") ? .losesSchemaContext : reader.storedProcedureCall(grammar: grammar) + case "REVERT", "SETUSER": + return .losesSchemaContext + case "CREATE": + return reader.createTemporaryTable(dialect: dialect) + default: + return .other + } + } +} + +private struct Reader { + private static let dropOptions: Set = ["CASCADE", "RESTRICT", "CONSTRAINTS", "PURGE", "SYNC", "NO", "DELAY"] + private static let transactionNouns: Set = ["WORK", "TRANSACTION", "TRAN"] + private static let schemaSettings: Set = ["SEARCH_PATH", "SCHEMA", "CURRENT_SCHEMA"] + private static let commitModeSettings: Set = ["IMPLICIT_TRANSACTIONS", "ANSI_DEFAULTS", "AUTOCOMMIT"] + + private var cursor: SQLTokenCursor + + init(_ cursor: SQLTokenCursor) { + self.cursor = cursor + } + + mutating func nextWord() -> String? { + cursor.next()?.word + } + + func peekWord() -> String? { + cursor.peek()?.word + } + + mutating func accept(_ word: String) -> Bool { + guard peekWord() == word else { return false } + _ = cursor.next() + return true + } + + private mutating func acceptSymbol(_ symbol: UInt16) -> Bool { + guard cursor.peek()?.isSymbol(symbol) == true else { return false } + _ = cursor.next() + return true + } + + private var isAtEnd: Bool { + cursor.peek() == nil + } + + mutating func name() -> SQLObjectName? { + var parts: [SQLNamePart] = [] + repeat { + switch cursor.next() { + case .word: + parts.append(SQLNamePart(text: cursor.lastTokenText, isQuoted: false)) + case .quotedIdentifier(let body): + parts.append(SQLNamePart(text: body, isQuoted: true)) + case .literal, .symbol, nil: + return nil + } + } while acceptSymbol(SQLTokenCursor.period) + return SQLObjectName(parts: parts) + } + + // MARK: - Drop and rename + + private mutating func objectKind() -> TableInfo.TableType? { + switch nextWord() { + case "TABLE": + return .table + case "VIEW": + return .view + case "MATERIALIZED": + return accept("VIEW") ? .materializedView : nil + case "FOREIGN": + return accept("TABLE") ? .foreignTable : nil + default: + return nil + } + } + + private mutating func acceptIfExists() { + var lookahead = self + guard lookahead.accept("IF"), lookahead.accept("EXISTS") else { return } + self = lookahead + } + + /// `DROP TEMPORARY TABLE` never reaches a table the app lists. + mutating func drop() -> TableEditStatement { + guard let kind = objectKind() else { return .other } + acceptIfExists() + guard let names = names() else { return .other } + while let token = cursor.next() { + guard let word = token.word else { return .other } + if Self.dropOptions.contains(word) { continue } + guard word == "ON", acceptOnClusterTail() else { return .other } + } + return .drop(names, kind: kind) + } + + private mutating func names() -> [SQLObjectName]? { + var names: [SQLObjectName] = [] + repeat { + guard let name = name() else { return nil } + names.append(name) + } while acceptSymbol(SQLTokenCursor.comma) + return names + } + + /// ClickHouse's `ON CLUSTER name`, with `ON` already read. + private mutating func acceptOnClusterTail() -> Bool { + accept("CLUSTER") && name() != nil + } + + mutating func alter(dialect: TableEditDialect) -> TableEditStatement { + if accept("SESSION") { + return accept("SET") && accept("CURRENT_SCHEMA") ? .losesSchemaContext : .other + } + guard let kind = objectKind() else { return .other } + /// `ALTER TABLE IF EXISTS missing RENAME TO live` succeeds having renamed nothing, and read + /// as a rename it would move stale settings over the live table's own. + var conditional = self + if conditional.accept("IF"), conditional.accept("EXISTS") { return .other } + guard let source = name(), accept("RENAME") else { return .other } + let namedTarget = accept("TO") || accept("AS") + guard namedTarget || dialect.renamesWithoutTo, let target = name(), isAtEnd else { return .other } + return .rename([SQLRenamePair(from: source, to: target)], kind: kind) + } + + /// MySQL's and ClickHouse's `RENAME TABLE a TO b, c TO d`, applied pair by pair in order. + mutating func renameTables() -> TableEditStatement { + guard accept("TABLE") || accept("TABLES") else { return .other } + var pairs: [SQLRenamePair] = [] + repeat { + guard let source = name(), accept("TO"), let target = name() else { return .other } + pairs.append(SQLRenamePair(from: source, to: target)) + } while acceptSymbol(SQLTokenCursor.comma) + if accept("ON"), !acceptOnClusterTail() { return .other } + return isAtEnd ? .rename(pairs, kind: .table) : .other + } + + /// `CREATE TEMPORARY TABLE people`, or SQLite's `CREATE TABLE temp.people`, shadows the real + /// `people` for the rest of the session, so a bare `DROP TABLE people` after it drops the + /// temporary one. + mutating func createTemporaryTable(dialect: TableEditDialect) -> TableEditStatement { + if accept("OR"), !accept("REPLACE") { return .other } + _ = accept("GLOBAL") || accept("LOCAL") || accept("PRIVATE") + let saysTemporary = accept("TEMPORARY") || accept("TEMP") + guard accept("TABLE") || accept("VIEW") else { return .other } + var lookahead = self + if lookahead.accept("IF"), lookahead.accept("NOT"), lookahead.accept("EXISTS") { + self = lookahead + } + guard let name = name() else { return .other } + let inTemporaryContainer = name.parts.dropLast().contains { dialect.namesTemporaryContainer($0.text) } + return saysTemporary || inTemporaryContainer ? .createsTemporaryTable(name) : .other + } + + /// MariaDB's `BEGIN NOT ATOMIC ... END` runs a compound statement, not a transaction. + mutating func opensCompoundStatement() -> Bool { + var lookahead = self + return lookahead.accept("NOT") && lookahead.accept("ATOMIC") + } + + /// A procedure call, which is unseen code, unless it is SQL Server's only table rename, + /// `EXEC sp_rename 'schema.old', 'new'`, optionally typed `'OBJECT'`. The old name is itself a + /// multipart name inside a string. The new one is taken exactly as written, because the + /// procedure names the object with the literal text. + mutating func storedProcedureCall(grammar: SQLLexicalGrammar) -> TableEditStatement { + guard let procedure = name(), Self.isRenameProcedure(procedure) else { return .runsUnseenCode } + guard let source = stringLiteral().flatMap({ Self.multipartName(in: $0, grammar: grammar) }), + acceptSymbol(SQLTokenCursor.comma), + let target = stringLiteral(), !target.isEmpty else { return .other } + if acceptSymbol(SQLTokenCursor.comma) { + guard stringLiteral()?.uppercased() == "OBJECT" else { return .other } + } + guard isAtEnd else { return .other } + let renamed = SQLObjectName(parts: [SQLNamePart(text: target, isQuoted: true)]) + return .rename([SQLRenamePair(from: source, to: renamed)], kind: .table) + } + + private static func isRenameProcedure(_ name: SQLObjectName) -> Bool { + let parts = name.parts.map { $0.text.lowercased() } + return parts == ["sp_rename"] || parts == ["sys", "sp_rename"] + } + + private static func multipartName(in text: String, grammar: SQLLexicalGrammar) -> SQLObjectName? { + var reader = Reader(SQLTokenCursor(text, grammar: grammar)) + guard let name = reader.name(), reader.isAtEnd else { return nil } + return name + } + + /// A `'...'` or `N'...'` literal's text, with doubled quotes undone. + private mutating func stringLiteral() -> String? { + if peekWord() == "N" { + var lookahead = self + _ = lookahead.cursor.next() + if case .literal = lookahead.cursor.peek() { self = lookahead } + } + guard case .literal = cursor.next() else { return nil } + var text = cursor.lastTokenText + if text.first == "N" || text.first == "n" { text.removeFirst() } + guard text.count >= 2, text.hasPrefix("'"), text.hasSuffix("'") else { return nil } + return String(text.dropFirst().dropLast()).replacingOccurrences(of: "''", with: "'") + } + + // MARK: - Transactions and context + + mutating func commit() -> TableEditStatement { + guard !accept("PREPARED") else { return .other } + _ = acceptTransactionNoun() + return chains() ? .losesTransactionTracking : .commits + } + + /// T-SQL's `ROLLBACK TRAN name` may name the transaction or a savepoint inside it, and only + /// the savepoint leaves the transaction open, so a named rollback is read as the savepoint. + mutating func rollback() -> TableEditStatement { + guard !accept("PREPARED") else { return .other } + _ = acceptTransactionNoun() + if chains() { return .losesTransactionTracking } + guard let next = cursor.peek() else { return .rollsBack } + if next.word == "AND" { return .rollsBack } + return .rollsBackToSavepoint + } + + private mutating func acceptTransactionNoun() -> Bool { + guard let word = peekWord(), Self.transactionNouns.contains(word) else { return false } + _ = cursor.next() + return true + } + + /// `AND CHAIN` opens the next transaction the moment this one ends. `AND NO CHAIN` does not. + private mutating func chains() -> Bool { + var lookahead = self + guard lookahead.accept("AND") else { return false } + return !lookahead.accept("NO") + } + + mutating func set() -> TableEditStatement { + _ = accept("SESSION") || accept("LOCAL") || accept("GLOBAL") + guard let first = nextWord() else { return .other } + if Self.schemaSettings.contains(first) { return .losesSchemaContext } + var word: String? = first + while let current = word { + if Self.commitModeSettings.contains(current) { return .losesTransactionTracking } + word = nextIdentifier() + } + return .other + } + + private mutating func nextIdentifier() -> String? { + while let token = cursor.next() { + if let identifier = token.identifier { return identifier } + } + return nil + } + + mutating func use() -> TableEditStatement { + guard let name = name(), isAtEnd else { return .selectsDatabase(nil) } + return .selectsDatabase(name) + } +} diff --git a/TablePro/Views/Main/Extensions/MainContentCoordinator+CatalogChange.swift b/TablePro/Views/Main/Extensions/MainContentCoordinator+CatalogChange.swift index e9b2cbd626..102cf49894 100644 --- a/TablePro/Views/Main/Extensions/MainContentCoordinator+CatalogChange.swift +++ b/TablePro/Views/Main/Extensions/MainContentCoordinator+CatalogChange.swift @@ -11,11 +11,18 @@ import TableProPluginKit /// selection and its cached columns. extension MainContentCoordinator { /// Reported whether the statement succeeded or failed: DDL that commits as it runs, a procedure - /// or a dropped connection can leave the catalog changed behind an error. - nonisolated static func postStatementRan(_ sql: String, on connection: DatabaseConnection) { + /// or a dropped connection can leave the catalog changed behind an error. A table it dropped or + /// renamed is reported apart, and only by a statement that succeeded. + nonisolated static func postStatementRan( + _ sql: String, + on connection: DatabaseConnection, + succeeded: SucceededStatements? = nil + ) { CatalogChangeService.post( .statementsRan(connectionId: connection.id, statements: [sql], databaseType: connection.type) ) + guard let succeeded else { return } + CatalogChangeService.post(.statementsSucceeded(succeeded)) } func applyCatalogChange(_ change: CatalogChange) { diff --git a/TablePro/Views/Main/MainContentCoordinator.swift b/TablePro/Views/Main/MainContentCoordinator.swift index 5a11c7b127..746e369070 100644 --- a/TablePro/Views/Main/MainContentCoordinator.swift +++ b/TablePro/Views/Main/MainContentCoordinator.swift @@ -1307,6 +1307,7 @@ final class MainContentCoordinator: ObservableObject { return } let isTableTab = tab.tabType == .table + let grammar = lexicalGrammar let failureOutput = ServerOutputBox() let queryTask = Task { [weak self] in @@ -1331,21 +1332,25 @@ final class MainContentCoordinator: ObservableObject { let fetchBeganAt = ContinuousClock.now do { - let fetchResult = try await withExecutionDriver( + let (fetchResult, tableEdits) = try await withExecutionDriver( scope: scope, isTableTab: isTableTab, lease: lease ) { [queryExecutor] driver in - try await queryExecutor.executeQuery( + let fetched = try await queryExecutor.executeQuery( driver: driver, sql: statement.sql, parameters: nil, rowCap: rowCap, capturingOutputInto: isTableTab ? nil : failureOutput ) + let edits = isAutoLoad ? nil : await SucceededStatements.single( + statement.sql, scope: scope, databaseType: conn.type, grammar: grammar, ranOn: driver + ) + return (fetched, edits) } let fetchEndedAt = ContinuousClock.now - if !isAutoLoad { Self.postStatementRan(statement.sql, on: conn) } + if !isAutoLoad { Self.postStatementRan(statement.sql, on: conn, succeeded: tableEdits) } guard !Task.isCancelled else { schemaTask?.cancel() diff --git a/TableProTests/Core/Services/Query/CommittedTableEditsTests.swift b/TableProTests/Core/Services/Query/CommittedTableEditsTests.swift new file mode 100644 index 0000000000..49f61d91d5 --- /dev/null +++ b/TableProTests/Core/Services/Query/CommittedTableEditsTests.swift @@ -0,0 +1,449 @@ +// +// CommittedTableEditsTests.swift +// TableProTests +// + +import Foundation +@testable import TablePro +import TableProPluginKit +import Testing + +struct CommittedTableEditsTests { + private static let connectionId = UUID() + + private static func edits( + _ statements: [String], + on type: DatabaseType = .postgresql, + database: String = "shop", + schema: String? = "public", + commit: StatementCommitEvidence = .runStartedIn(.idle, appTransaction: .none) + ) -> [TableCatalogEdit] { + var hazards = TableNameHazards() + return edits(statements, on: type, database: database, schema: schema, commit: commit, hazards: &hazards) + } + + private static func edits( + _ statements: [String], + on type: DatabaseType = .postgresql, + database: String = "shop", + schema: String? = "public", + commit: StatementCommitEvidence = .runStartedIn(.idle, appTransaction: .none), + hazards: inout TableNameHazards + ) -> [TableCatalogEdit] { + let succeeded = SucceededStatements( + scope: DatabaseScope(connectionId: connectionId, database: database, schema: schema), + databaseType: type, + statements: statements, + commit: commit + ) + return CommittedTableEdits.edits(in: succeeded, grammar: type.lexicalGrammar, hazards: &hazards) + } + + private static func dropped(_ name: String, database: String = "shop", schema: String? = "public") -> TableCatalogEdit { + .dropped(TablePlacement(database: database, schema: schema, name: name), kind: .table) + } + + private static func renamed( + _ name: String, to newName: String, database: String = "shop", schema: String? = "public" + ) -> TableCatalogEdit { + .renamed(TablePlacement(database: database, schema: schema, name: name), to: newName, kind: .table) + } + + // MARK: - The reported case + + @Test("A table dropped and created again under the same name is a drop of the old one") + func dropAndRecreate() { + #expect(Self.edits(["DROP TABLE public.people", "CREATE TABLE public.people (id int)"]) == [Self.dropped("people")]) + #expect( + Self.edits(["DROP TABLE people", "CREATE TABLE people (id int)"], on: .mysql, schema: nil) + == [Self.dropped("people", schema: nil)] + ) + #expect( + Self.edits(["DROP TABLE people", "CREATE TABLE people (id int)"], on: .sqlite, database: "/tmp/app.db", schema: nil) + == [Self.dropped("people", database: "/tmp/app.db", schema: nil)] + ) + } + + @Test("Every name of a multi-table drop is placed where it lives") + func multiTableDrop() { + #expect( + Self.edits(["DROP TABLE IF EXISTS public.a, sales.b CASCADE"]) + == [Self.dropped("a"), Self.dropped("b", schema: "sales")] + ) + } + + /// PostgreSQL resolves a bare name through a search path and a temporary namespace that a + /// function or `SELECT ... INTO TEMP` can change without the text showing it. + @Test("A bare name is not placed on PostgreSQL, DuckDB or SQL Server") + func bareNamesOnSessionResolvedEngines() { + #expect(Self.edits(["DROP TABLE people"]).isEmpty) + #expect(Self.edits(["DROP TABLE people"], on: .duckdb, database: "analytics", schema: "main").isEmpty) + #expect(Self.edits(["DROP TABLE people"], on: .mssql, database: "sales", schema: "dbo").isEmpty) + } + + @Test("A name inside a session's temporary container is never placed") + func temporaryContainers() { + #expect(Self.edits(["DROP TABLE pg_temp.people", "DROP TABLE pg_temp_3.people"]).isEmpty) + #expect(Self.edits(["DROP TABLE pg_temporary.people"]) == [Self.dropped("people", schema: "pg_temporary")]) + #expect(Self.edits(["DROP TABLE temp.main.people"], on: .duckdb, database: "analytics", schema: "main").isEmpty) + } + + @Test("A rename keeps the table in its schema whatever the statement's own scope is") + func renameStaysInItsSchema() { + #expect(Self.edits(["ALTER TABLE sales.orders RENAME TO Orders_2019"]) == [ + Self.renamed("orders", to: "orders_2019", schema: "sales") + ]) + } + + // MARK: - Commit evidence + + @Test("A single statement counts only when the session holds no transaction after it") + func singleStatementNeedsAnIdleSession() { + #expect(Self.edits(["DROP TABLE public.people"], commit: .statementLeftSession(.idle)) == [Self.dropped("people")]) + #expect(Self.edits(["DROP TABLE public.people"], commit: .statementLeftSession(.inTransaction)).isEmpty) + #expect(Self.edits(["DROP TABLE public.people"], commit: .statementLeftSession(.abortedTransaction)).isEmpty) + #expect(Self.edits(["DROP TABLE public.people"], commit: .statementLeftSession(.unknown)).isEmpty) + } + + @Test("An engine that commits DDL as it runs needs no word from the session") + func implicitCommitEngines() { + #expect( + Self.edits(["DROP TABLE people"], on: .mysql, schema: nil, commit: .statementLeftSession(.unknown)) + == [Self.dropped("people", schema: nil)] + ) + #expect( + Self.edits(["BEGIN", "DROP TABLE people", "ROLLBACK"], on: .mysql, schema: nil) + == [Self.dropped("people", schema: nil)] + ) + #expect( + Self.edits(["DROP TABLE emp"], on: .oracle, database: "ORCL", schema: "HR", commit: .statementLeftSession(.inTransaction)) + == [Self.dropped("EMP", database: "ORCL", schema: "HR")] + ) + } + + @Test("A drop the script rolls back is not a drop") + func rolledBackDrop() { + #expect(Self.edits(["BEGIN", "DROP TABLE public.people", "ROLLBACK"]).isEmpty) + #expect(Self.edits(["BEGIN", "DROP TABLE public.people", "ROLLBACK TO SAVEPOINT s", "COMMIT"]).isEmpty) + } + + @Test("A drop the script commits is a drop, and one still open at the end is not") + func committedAndOpenDrops() { + #expect(Self.edits(["BEGIN", "DROP TABLE public.people", "COMMIT"]) == [Self.dropped("people")]) + #expect(Self.edits(["BEGIN", "DROP TABLE public.people", "END"]) == [Self.dropped("people")]) + #expect(Self.edits(["DROP TABLE public.a", "BEGIN", "DROP TABLE public.b"]) == [Self.dropped("a")]) + } + + @Test("A COMMIT inside a nested transaction does not commit the outer one") + func nestedTransactions() { + #expect( + Self.edits( + ["BEGIN TRAN", "BEGIN TRAN", "DROP TABLE dbo.t", "COMMIT TRAN"], on: .mssql, database: "sales", schema: "dbo" + ).isEmpty + ) + #expect( + Self.edits( + ["BEGIN TRAN", "BEGIN TRAN", "DROP TABLE dbo.t", "COMMIT TRAN", "COMMIT TRAN"], + on: .mssql, database: "sales", schema: "dbo" + ) == [Self.dropped("t", database: "sales", schema: "dbo")] + ) + #expect( + Self.edits(["SAVEPOINT s", "DROP TABLE t", "RELEASE s"], on: .sqlite, database: "/tmp/app.db", schema: nil) + == [Self.dropped("t", database: "/tmp/app.db", schema: nil)] + ) + } + + @Test("A run that started inside a transaction, or on a session that could not say, is not read") + func runStartedInTransaction() { + #expect( + Self.edits(["DROP TABLE public.people"], commit: .runStartedIn(.inTransaction, appTransaction: .none)) + .isEmpty + ) + #expect( + Self.edits(["DROP TABLE public.people", "COMMIT"], commit: .runStartedIn(.unknown, appTransaction: .none)) + .isEmpty + ) + } + + @Test("A run the app wrapped counts once the app committed it, and not once it rolled it back") + func appTransaction() { + #expect( + Self.edits( + ["DROP TABLE public.people", "INSERT INTO log VALUES (1)"], + commit: .run(startedIn: .idle, plan: .appTransaction, completed: true) + ) == [Self.dropped("people")] + ) + #expect( + Self.edits(["DROP TABLE public.people"], commit: .run(startedIn: .idle, plan: .appTransaction, completed: false)) + .isEmpty + ) + #expect( + Self.edits(["DROP TABLE public.people"], commit: .run(startedIn: .idle, plan: .autocommit, completed: false)) + == [Self.dropped("people")] + ) + } + + /// A bare `ROLLBACK` does not make a script manage its own transaction, so the app still wraps + /// it, and that `ROLLBACK` is what ends the app's transaction. + @Test("A ROLLBACK inside a run the app wrapped takes back what came before it") + func rollbackInsideTheAppTransaction() { + let wrapped = StatementCommitEvidence.run(startedIn: .idle, plan: .appTransaction, completed: true) + #expect(Self.edits(["DROP TABLE public.t", "ROLLBACK"], commit: wrapped).isEmpty) + #expect( + Self.edits(["DROP TABLE public.t", "ROLLBACK", "DROP TABLE public.u"], commit: wrapped) + == [Self.dropped("u")] + ) + #expect(Self.edits(["DROP TABLE public.t", "COMMIT"], commit: wrapped) == [Self.dropped("t")]) + } + + @Test("Turning implicit transactions on leaves nothing after it that can be judged") + func implicitTransactions() { + #expect( + Self.edits( + ["SET IMPLICIT_TRANSACTIONS ON", "DROP TABLE dbo.t", "ROLLBACK"], on: .mssql, database: "sales", schema: "dbo" + ).isEmpty + ) + } + + // MARK: - Name context and hazards + + @Test("USE moves a MySQL bare name onto the database it selected") + func useMovesMySQLNames() { + #expect( + Self.edits(["USE archive", "DROP TABLE orders"], on: .mysql, schema: nil) + == [Self.dropped("orders", database: "archive", schema: nil)] + ) + } + + @Test("A schema moved by hand leaves bare names unplaced from then on, in this run and every later one") + func schemaMovedByHand() { + var hazards = TableNameHazards() + #expect( + Self.edits( + ["ALTER SESSION SET CURRENT_SCHEMA = SCOTT", "DROP TABLE emp", "DROP TABLE hr.dept"], + on: .oracle, database: "ORCL", schema: "HR", hazards: &hazards + ) == [Self.dropped("DEPT", database: "ORCL", schema: "HR")] + ) + #expect(hazards.namesMayBeShadowed) + #expect(Self.edits(["DROP TABLE emp"], on: .oracle, database: "ORCL", schema: "HR", hazards: &hazards).isEmpty) + } + + @Test("A temporary table shadows the real one for good, in this run and every later one") + func temporaryShadowing() { + var hazards = TableNameHazards() + #expect( + Self.edits( + ["CREATE TEMP TABLE people (id int)", "DROP TABLE people", "DROP TABLE people"], + on: .sqlite, database: "/tmp/app.db", schema: nil, hazards: &hazards + ).isEmpty + ) + #expect(hazards.temporaryNames == ["people"]) + #expect( + Self.edits( + ["DROP TABLE people", "DROP TABLE orders"], on: .sqlite, database: "/tmp/app.db", schema: nil, hazards: &hazards + ) == [Self.dropped("orders", database: "/tmp/app.db", schema: nil)] + ) + var caseFolded = TableNameHazards() + #expect( + Self.edits( + ["CREATE TEMP TABLE People (id int)", "DROP TABLE people"], + on: .sqlite, database: "/tmp/app.db", schema: nil, hazards: &caseFolded + ).isEmpty + ) + var sqliteQualified = TableNameHazards() + _ = Self.edits( + ["CREATE TABLE temp.people (id int)"], on: .sqlite, database: "/tmp/app.db", schema: nil, hazards: &sqliteQualified + ) + #expect(sqliteQualified.temporaryNames == ["people"]) + } + + @Test("A temporary table is remembered even when its transaction is one the text cannot judge") + func temporaryTableInsideATransaction() { + var hazards = TableNameHazards() + _ = Self.edits( + ["CREATE TEMP TABLE people (id int)"], on: .sqlite, database: "/tmp/app.db", schema: nil, + commit: .statementLeftSession(.inTransaction), hazards: &hazards + ) + #expect(hazards.temporaryNames == ["people"]) + } + + @Test("A MySQL temporary table hides the real one under its qualified name too, even after it is dropped") + func mySQLTemporaryTables() { + var hazards = TableNameHazards() + #expect( + Self.edits( + ["CREATE TEMPORARY TABLE people (id int)", "DROP TABLE shop.people"], on: .mysql, schema: nil, hazards: &hazards + ).isEmpty + ) + #expect( + Self.edits(["DROP TEMPORARY TABLE people", "DROP TABLE people"], on: .mysql, schema: nil, hazards: &hazards) + .isEmpty + ) + } + + @Test("A schema-qualified PostgreSQL name never reaches a temporary table or a moved search path") + func qualifiedNamesAreNotShadowed() { + var hazards = TableNameHazards(temporaryNames: ["people"], namesMayBeShadowed: true) + #expect(Self.edits(["DROP TABLE public.people"], hazards: &hazards) == [Self.dropped("people")]) + } + + @Test("Code the server runs unseen leaves bare MySQL names, and qualified ones, unplaced") + func unseenCode() { + #expect(Self.edits(["CALL rebuild()", "DROP TABLE people", "DROP TABLE shop.orders"], on: .mysql, schema: nil).isEmpty) + #expect( + Self.edits(["CALL rebuild()", "DROP TABLE public.people"]) + == [Self.dropped("people")] + ) + } + + @Test("An Oracle global temporary table is a real table and is dropped like one") + func oracleGlobalTemporaryTable() { + #expect( + Self.edits( + ["CREATE GLOBAL TEMPORARY TABLE gtt (id NUMBER)", "DROP TABLE gtt"], on: .oracle, database: "ORCL", schema: "HR" + ) == [Self.dropped("GTT", database: "ORCL", schema: "HR")] + ) + } + + // MARK: - Renames + + @Test("SQL Server's sp_rename keeps the table in its schema and takes the new name as written") + func sqlServerStoredProcedureRename() { + #expect( + Self.edits(["EXEC sp_rename 'dbo.people', 'Persons'"], on: .mssql, database: "sales", schema: "dbo") + == [Self.renamed("people", to: "Persons", database: "sales", schema: "dbo")] + ) + #expect(Self.edits(["EXEC sp_rename 'people', 'persons'"], on: .mssql, database: "sales", schema: "dbo").isEmpty) + } + + @Test("A conditional rename is never adopted") + func conditionalRename() { + #expect(Self.edits(["ALTER TABLE IF EXISTS public.missing RENAME TO live"]).isEmpty) + } + + @Test("A rename chain is applied pair by pair in the order it was written") + func renameChain() { + #expect( + Self.edits(["RENAME TABLE a TO tmp, b TO a, tmp TO b"], on: .mysql, schema: nil) == [ + Self.renamed("a", to: "tmp", schema: nil), + Self.renamed("b", to: "a", schema: nil), + Self.renamed("tmp", to: "b", schema: nil) + ] + ) + } + + @Test("A rename that moves the table to another database is left alone, whole") + func crossDatabaseRename() { + #expect(Self.edits(["RENAME TABLE a TO b, c TO archive.c"], on: .mysql, schema: nil).isEmpty) + #expect(Self.edits(["ALTER TABLE a RENAME TO archive.a"], on: .mysql, schema: nil).isEmpty) + #expect( + Self.edits(["RENAME TABLE archive.a TO archive.b"], on: .mysql, schema: nil) + == [Self.renamed("a", to: "b", database: "archive", schema: nil)] + ) + } + + @Test("An engine with no known dialect adopts nothing") + func unknownEngine() { + #expect(Self.edits(["DROP TABLE people"], on: .snowflake).isEmpty) + #expect(Self.edits(["DROP TABLE people"], on: .cockroachdb).isEmpty) + } +} + +struct SucceededStatementsProbeTests { + private static func probe( + _ sql: String, on type: DatabaseType, state: PluginSessionTransactionState + ) async -> SucceededStatements? { + let connection = TestFixtures.makeConnection(type: type) + let driver = ScriptAnsweringDriver(connection: connection, transactionState: state) + return await SucceededStatements.single( + sql, + scope: DatabaseScope(connectionId: connection.id, database: "shop", schema: "public"), + databaseType: type, + grammar: type.lexicalGrammar, + ranOn: driver + ) + } + + @Test("A statement that edits no table reports nothing") + func readsReportNothing() async { + #expect(await Self.probe("SELECT * FROM people", on: .postgresql, state: .idle) == nil) + #expect(await Self.probe("DROP TABLE people", on: .snowflake, state: .idle) == nil) + } + + @Test("A drop on a transactional engine carries what the session held after it") + func transactionalEngineAsksTheSession() async { + let report = await Self.probe("DROP TABLE people", on: .postgresql, state: .inTransaction) + #expect(report?.commit == .statementLeftSession(.inTransaction)) + #expect(report?.statements == ["DROP TABLE people"]) + } + + @Test("An engine that commits DDL as it runs is not asked") + func implicitCommitEngineIsNotAsked() async { + let report = await Self.probe("DROP TABLE people", on: .mysql, state: .inTransaction) + #expect(report?.commit == .statementLeftSession(.unknown)) + } + + @Test("A temporary table's creation and a procedure call are reported without asking the session") + func hazardsAreReported() async { + let created = await Self.probe("CREATE TEMP TABLE people (id int)", on: .sqlite, state: .inTransaction) + #expect(created?.commit == .statementLeftSession(.unknown)) + let called = await Self.probe("CALL rebuild()", on: .mysql, state: .idle) + #expect(called?.statements == ["CALL rebuild()"]) + } +} + +@MainActor +struct ScriptBatchProgressTests { + /// SQL Server commits each batch of a script as it runs, so the batches before a failing one + /// have to be reported even though the call fails. + @Test("A script that fails part way records the batches that finished and the state it started in") + func failedScriptKeepsItsPrefix() async throws { + let connection = TestFixtures.makeConnection(database: "sales", type: .mssql) + let driver = ScriptAnsweringDriver(connection: connection, transactionState: .idle) { query in + guard query.contains("missing") else { return .empty } + return ScriptAnsweringDriver.batch( + [], + errors: [ + PluginBatchError( + message: "Invalid object name 'missing'.", code: 208, line: 1, procedure: nil, + precedingResultSetCount: 0 + ) + ] + ) + } + let grammar = DatabaseType.mssql.lexicalGrammar + let batches = QueryBatchPlanner.batches( + in: "DROP TABLE dbo.people\nGO\nSELECT * FROM missing\nGO\nDROP TABLE dbo.orders", + model: QueryStatementModel.forDatabaseType(.mssql), + grammar: grammar + ) + let progress = ScriptBatchProgress() + + await #expect(throws: DatabaseError.self) { + try await ScriptBatchRun.run( + batches, startLines: [1, 3, 5], rowCap: 100, driver: driver, progress: progress + ) + } + + #expect(batches.count == 3) + #expect(progress.completedBatchCount == 1) + #expect(progress.startState == .idle) + } +} + +struct BatchStatementOutcomeSucceededCountTests { + @Test("A statement failure leaves the statements before it, and a batch failure drops the batch that answered") + func succeededCount() { + let completed = BatchStatementOutcome.completed(results: [1, 2, 3]) + #expect(completed.succeededCount == 3) + #expect(completed.isCompleted) + let statementFailure = BatchStatementOutcome.failed( + results: [1, 2], failure: .statement(sql: "x"), errorDescription: "" + ) + #expect(statementFailure.succeededCount == 2) + #expect(!statementFailure.isCompleted) + let batchFailure = BatchStatementOutcome.failed(results: [1, 2], failure: .batch(sql: "x"), errorDescription: "") + #expect(batchFailure.succeededCount == 1) + #expect(BatchStatementOutcome.cancelled(results: [1]).succeededCount == 1) + } +} diff --git a/TableProTests/Core/Services/Query/SQLTableEditAdoptionTests.swift b/TableProTests/Core/Services/Query/SQLTableEditAdoptionTests.swift new file mode 100644 index 0000000000..1eb8f5138a --- /dev/null +++ b/TableProTests/Core/Services/Query/SQLTableEditAdoptionTests.swift @@ -0,0 +1,205 @@ +// +// SQLTableEditAdoptionTests.swift +// TableProTests +// + +import Foundation +@testable import TablePro +import TableProPluginKit +import TableProSyncTransport +import Testing + +/// A table dropped or renamed by SQL someone ran reaches the same adoption the sidebar's own Drop +/// and Rename do: the saved settings, the favorite and the queued operations all follow it. +@MainActor +struct SQLTableEditAdoptionTests { + @MainActor + private final class RecordingStore: TableScopedSettingsStore { + struct Rename: Equatable { + let from: TableScope + let to: TableScope + } + + private(set) var droppedTables: [TableScope] = [] + private(set) var renamedTables: [Rename] = [] + + func renameTable(from oldScope: TableScope, to newScope: TableScope) { + renamedTables.append(Rename(from: oldScope, to: newScope)) + } + + func renameContainer( + connectionId: UUID, + fromDatabase: String, + fromSchema: String?, + toDatabase: String, + toSchema: String? + ) {} + + func dropTable(_ scope: TableScope) { + droppedTables.append(scope) + } + + func dropContainer(connectionId: UUID, database: String, schema: String?) {} + + func purgeConnections(_ connectionIds: Set, leavesTombstones: Bool) {} + } + + @MainActor + private final class IgnoringTarget: CatalogChangeTarget { + func refreshCatalog(for change: CatalogChange) async {} + } + + @MainActor + private struct Harness { + let connection: DatabaseConnection + let store: RecordingStore + let favorites: FavoriteTablesStorage + let service: CatalogChangeService + + func run( + _ statements: [String], + schema: String? = "public", + commit: StatementCommitEvidence = .runStartedIn(.idle, appTransaction: .none) + ) { + service.record(.statementsSucceeded(SucceededStatements( + scope: DatabaseScope(connectionId: connection.id, database: "shop", schema: schema), + databaseType: connection.type, + statements: statements, + commit: commit + ))) + } + + func scope(_ table: String, schema: String? = "public") -> TableScope { + TableScope(connectionId: connection.id, database: "shop", schema: schema, table: table) + } + + func favorite(_ table: String, schema: String?) -> FavoriteTablesStorage.FavoriteEntry { + FavoriteTablesStorage.FavoriteEntry(connectionId: connection.id, database: "shop", schema: schema, name: table) + } + } + + private func makeHarness(type: DatabaseType = .postgresql, browseSchema: String? = "public") throws -> Harness { + let connection = TestFixtures.makeConnection(database: "shop", type: type) + var session = ConnectionSession(connection: connection, driver: MockDatabaseDriver(connection: connection)) + session.status = .connected + session.browseDatabase = "shop" + session.browseSchema = browseSchema + DatabaseManager.shared.injectSession(session, for: connection.id) + + let suite = "SQLTableEditAdoptionTests.\(UUID().uuidString)" + let defaults = try #require(UserDefaults(suiteName: suite)) + let syncDefaults = try #require(UserDefaults(suiteName: suite + ".sync")) + let favorites = FavoriteTablesStorage( + userDefaults: defaults, + syncTracker: SyncChangeTracker(metadataStorage: SyncMetadataStorage(userDefaults: syncDefaults)) + ) + let store = RecordingStore() + let adoption = CatalogEditAdoption(settingsStores: [store], favoriteTables: favorites) + let service = CatalogChangeService(targets: [IgnoringTarget()], adoption: adoption, isSessionLive: { _ in true }) + return Harness(connection: connection, store: store, favorites: favorites, service: service) + } + + private func tearDown(_ harness: Harness) { + DatabaseManager.shared.removeSession(for: harness.connection.id) + } + + @Test("Dropping a table in SQL forgets its saved settings and its favorite") + func sqlDropForgetsSettings() throws { + let harness = try makeHarness() + defer { tearDown(harness) } + harness.favorites.addFavorite(name: "people", schema: "public", database: "shop", connectionId: harness.connection.id) + + harness.run(["DROP TABLE public.People", "CREATE TABLE public.people (id int)"]) + + #expect(harness.store.droppedTables == [harness.scope("people")]) + #expect(harness.favorites.favorites(for: harness.connection.id).isEmpty) + } + + @Test("Renaming a table in SQL moves its saved settings and its favorite to the new name") + func sqlRenameMovesSettings() throws { + let harness = try makeHarness() + defer { tearDown(harness) } + harness.favorites.addFavorite(name: "people", schema: "public", database: "shop", connectionId: harness.connection.id) + + harness.run(["ALTER TABLE public.people RENAME TO persons"]) + + #expect(harness.store.renamedTables == [.init(from: harness.scope("people"), to: harness.scope("persons"))]) + #expect(harness.favorites.favorites(for: harness.connection.id) == [harness.favorite("persons", schema: "public")]) + } + + @Test("A drop the script rolled back leaves the settings where they were") + func rolledBackDropKeepsSettings() throws { + let harness = try makeHarness() + defer { tearDown(harness) } + + harness.run(["BEGIN", "DROP TABLE public.people", "ROLLBACK"]) + harness.run(["DROP TABLE public.people"], commit: .statementLeftSession(.inTransaction)) + + #expect(harness.store.droppedTables.isEmpty) + } + + @Test("A favorite the sidebar saved without a schema is found for a table SQL named with one") + func favoriteSavedWithoutSchema() throws { + let harness = try makeHarness(type: .oracle, browseSchema: "HR") + defer { tearDown(harness) } + harness.favorites.addFavorite(name: "EMP", schema: nil, database: "shop", connectionId: harness.connection.id) + + harness.run(["DROP TABLE emp"], schema: "HR") + + #expect(harness.store.droppedTables == [harness.scope("EMP", schema: "HR")]) + #expect(harness.favorites.favorites(for: harness.connection.id).isEmpty) + } + + /// The session keeps a temporary table between runs, so the connection has to remember it. + @Test("A temporary table made in one run keeps later drops of that name off the real table's settings") + func temporaryTableAcrossRuns() throws { + let harness = try makeHarness(type: .mysql, browseSchema: nil) + defer { tearDown(harness) } + + harness.run(["CREATE TEMPORARY TABLE people (id int)"], schema: nil) + harness.run(["DROP TABLE people"], schema: nil) + harness.run(["DROP TABLE people", "DROP TABLE orders"], schema: nil) + + #expect(harness.store.droppedTables == [harness.scope("orders", schema: nil)]) + } + + /// A procedure that failed after creating a temporary table reports only that it ran. + @Test("A procedure call that failed still keeps later bare drops off the real table's settings") + func failedProcedureCallIsAHazard() throws { + let harness = try makeHarness(type: .mysql, browseSchema: nil) + defer { tearDown(harness) } + + harness.service.record(.statementsRan( + connectionId: harness.connection.id, statements: ["CALL make_staging()"], databaseType: .mysql + )) + harness.run(["DROP TABLE people"], schema: nil) + + #expect(harness.store.droppedTables.isEmpty) + } + + @Test("A conditional rename leaves both tables' settings where they were") + func conditionalRenameMovesNothing() throws { + let harness = try makeHarness() + defer { tearDown(harness) } + + harness.run(["ALTER TABLE IF EXISTS public.missing RENAME TO live"]) + + #expect(harness.store.renamedTables.isEmpty) + } + + /// A queued Drop left in place after SQL dropped and recreated the table would drop the new + /// table at the next Save, under a confirmation that named the old one. + @Test("A Drop queued in the sidebar comes out of the queue when SQL drops the table") + func queuedDropIsUnstaged() throws { + let harness = try makeHarness() + defer { tearDown(harness) } + let queued = DatabaseTreeTableRef( + database: "shop", schema: "public", table: TestFixtures.makeTableInfo(name: "people", schema: "public") + ) + DatabaseManager.shared.updateSession(harness.connection.id) { $0.pendingDeletes = [queued] } + + harness.run(["DROP TABLE public.people", "CREATE TABLE public.people (id int)"]) + + #expect(DatabaseManager.shared.session(for: harness.connection.id)?.pendingDeletes.isEmpty == true) + } +} diff --git a/TableProTests/Core/Utilities/SQL/SQLTokenCursorTests.swift b/TableProTests/Core/Utilities/SQL/SQLTokenCursorTests.swift index c1f31692c0..2ab3f1ea9d 100644 --- a/TableProTests/Core/Utilities/SQL/SQLTokenCursorTests.swift +++ b/TableProTests/Core/Utilities/SQL/SQLTokenCursorTests.swift @@ -143,4 +143,16 @@ struct SQLTokenCursorTests { #expect(commas == [11]) #expect(cursor.location == 14) } + + @Test("The last token's text is the word as written, not the uppercased word") + func lastTokenTextKeepsTheSpelling() { + var cursor = SQLTokenCursor("DROP TABLE Sales.\"Order Items\"", grammar: TestGrammar.postgres) + _ = cursor.next() + _ = cursor.next() + #expect(cursor.next() == .word("SALES")) + #expect(cursor.lastTokenText == "Sales") + _ = cursor.next() + #expect(cursor.next() == .quotedIdentifier("Order Items")) + #expect(cursor.lastTokenText == "\"Order Items\"") + } } diff --git a/TableProTests/Core/Utilities/SQL/TableEditStatementParserTests.swift b/TableProTests/Core/Utilities/SQL/TableEditStatementParserTests.swift new file mode 100644 index 0000000000..b0b04045dd --- /dev/null +++ b/TableProTests/Core/Utilities/SQL/TableEditStatementParserTests.swift @@ -0,0 +1,410 @@ +// +// TableEditStatementParserTests.swift +// TableProTests +// + +import Foundation +@testable import TablePro +import TableProSQLGrammar +import Testing + +struct TableEditStatementParserTests { + private static func parse(_ sql: String, _ type: DatabaseType) throws -> TableEditStatement { + let dialect = try #require(TableEditDialect.of(type)) + return TableEditStatementParser.parse(sql, dialect: dialect, grammar: type.lexicalGrammar) + } + + private static func bare(_ text: String) -> SQLNamePart { + SQLNamePart(text: text, isQuoted: false) + } + + private static func quoted(_ text: String) -> SQLNamePart { + SQLNamePart(text: text, isQuoted: true) + } + + private static func name(_ parts: SQLNamePart...) -> SQLObjectName { + SQLObjectName(parts: parts) + } + + // MARK: - Drop + + @Test("A drop reads every name in its list, as written, with IF EXISTS and CASCADE around it") + func dropListWithOptions() throws { + #expect( + try Self.parse("DROP TABLE IF EXISTS People, public.\"Orders\" CASCADE", .postgresql) + == .drop([Self.name(Self.bare("People")), Self.name(Self.bare("public"), Self.quoted("Orders"))], kind: .table) + ) + } + + @Test("MySQL backticks and a database qualifier are read as parts") + func mySQLBackticks() throws { + #expect( + try Self.parse("DROP TABLE `shop`.`order items`", .mysql) + == .drop([Self.name(Self.quoted("shop"), Self.quoted("order items"))], kind: .table) + ) + } + + @Test("SQL Server brackets are read as quoted parts") + func sqlServerBrackets() throws { + #expect( + try Self.parse("DROP TABLE [sales].[dbo].[Order Items]", .mssql) + == .drop([Self.name(Self.quoted("sales"), Self.quoted("dbo"), Self.quoted("Order Items"))], kind: .table) + ) + } + + @Test("Views, materialized views and foreign tables are drops of their own kind") + func dropKinds() throws { + #expect(try Self.parse("DROP VIEW v", .postgresql) == .drop([Self.name(Self.bare("v"))], kind: .view)) + #expect( + try Self.parse("DROP MATERIALIZED VIEW IF EXISTS m", .postgresql) + == .drop([Self.name(Self.bare("m"))], kind: .materializedView) + ) + #expect( + try Self.parse("DROP FOREIGN TABLE f", .postgresql) == .drop([Self.name(Self.bare("f"))], kind: .foreignTable) + ) + } + + @Test("Oracle's CASCADE CONSTRAINTS PURGE and ClickHouse's ON CLUSTER and SYNC are accepted") + func engineSpecificTails() throws { + #expect( + try Self.parse("DROP TABLE hr.emp CASCADE CONSTRAINTS PURGE", .oracle) + == .drop([Self.name(Self.bare("hr"), Self.bare("emp"))], kind: .table) + ) + #expect( + try Self.parse("DROP TABLE IF EXISTS logs.events ON CLUSTER main SYNC", .clickhouse) + == .drop([Self.name(Self.bare("logs"), Self.bare("events"))], kind: .table) + ) + } + + @Test("A temporary drop, an index drop and a drop followed by more text are not read") + func dropsThatAreNotRead() throws { + #expect(try Self.parse("DROP TEMPORARY TABLE IF EXISTS t", .mysql) == .other) + #expect(try Self.parse("DROP INDEX idx", .postgresql) == .other) + #expect(try Self.parse("DROP TABLE dbo.t\nSELECT 1", .mssql) == .other) + #expect(try Self.parse("DROP MATERIALIZED VIEW mv PRESERVE TABLE", .oracle) == .other) + #expect(try Self.parse("DROP TABLE archive..t", .mssql) == .other) + #expect(Self.placesNothing("DROP TABLE @t", .mssql)) + } + + private static func placesNothing(_ sql: String, _ type: DatabaseType) -> Bool { + guard let dialect = TableEditDialect.of(type), + case .drop(let names, _) = TableEditStatementParser.parse(sql, dialect: dialect, grammar: type.lexicalGrammar) + else { return true } + return names.allSatisfy { dialect.resolve($0, in: TableNameContext(database: "db", schema: "dbo")) == nil } + } + + @Test("A drop inside a MySQL executable comment is not read, since an older server skips it and succeeds") + func versionGatedDropIsNotRead() throws { + #expect(try Self.parse("/*!99999 DROP TABLE people */", .mysql) == .other) + #expect(try Self.parse("DROP TABLE /*!32312 IF EXISTS*/ people", .mysql) == .other) + #expect(try Self.parse("/*M!100100 DROP TABLE people */", .mariadb) == .other) + #expect( + try Self.parse("/*!40101 CREATE TEMPORARY TABLE people (id int) */", .mysql) + == .createsTemporaryTable(Self.name(Self.bare("people"))) + ) + } + + @Test("A comment before the statement and a trailing semicolon change nothing") + func commentsAndTerminator() throws { + #expect( + try Self.parse("-- tidy up\n/* old */ DROP TABLE t;", .postgresql) == .drop([Self.name(Self.bare("t"))], kind: .table) + ) + } + + // MARK: - Rename + + @Test("ALTER TABLE ... RENAME TO reads the old and the new name") + func alterRenameTo() throws { + #expect( + try Self.parse("ALTER TABLE s.people RENAME TO persons", .postgresql) + == .rename([SQLRenamePair(from: Self.name(Self.bare("s"), Self.bare("people")), to: Self.name(Self.bare("persons")))], kind: .table) + ) + #expect( + try Self.parse("ALTER VIEW v RENAME TO w", .postgresql) + == .rename([SQLRenamePair(from: Self.name(Self.bare("v")), to: Self.name(Self.bare("w")))], kind: .view) + ) + } + + /// PostgreSQL answers a conditional rename of a missing table with a notice and no error, so + /// reading it as a rename would move stale settings over the target's own. + @Test("A conditional rename is not read, because it succeeds having renamed nothing") + func conditionalRenameIsNotRead() throws { + #expect(try Self.parse("ALTER TABLE IF EXISTS missing RENAME TO live", .postgresql) == .other) + } + + @Test("SQL Server's sp_rename reads its object name as a multipart name and its new name as written") + func sqlServerStoredProcedureRename() throws { + let expected = TableEditStatement.rename( + [SQLRenamePair(from: Self.name(Self.bare("dbo"), Self.bare("people")), to: Self.name(Self.quoted("persons")))], + kind: .table + ) + #expect(try Self.parse("EXEC sp_rename 'dbo.people', 'persons'", .mssql) == expected) + #expect(try Self.parse("EXECUTE sys.sp_rename N'dbo.people', N'persons', 'OBJECT'", .mssql) == expected) + #expect( + try Self.parse("EXEC sp_rename N'[dbo].[Order Items]', N'Order''s', N'OBJECT'", .mssql) + == .rename( + [SQLRenamePair( + from: Self.name(Self.quoted("dbo"), Self.quoted("Order Items")), to: Self.name(Self.quoted("Order's")) + )], + kind: .table + ) + ) + } + + @Test("sp_rename of a column, an index, or with named arguments is not read") + func otherStoredProcedureRenames() throws { + #expect(try Self.parse("EXEC sp_rename 'dbo.people.name', 'full_name', 'COLUMN'", .mssql) == .other) + #expect(try Self.parse("EXEC sp_rename N'dbo.people.ix_name', N'ix_full', N'INDEX'", .mssql) == .other) + #expect(try Self.parse("EXEC sp_rename @objname = N'dbo.people', @newname = N'persons'", .mssql) == .other) + #expect(try Self.parse("EXEC sp_helptext 'dbo.people'", .mssql) == .runsUnseenCode) + } + + @Test("MySQL renames with AS or with no keyword at all") + func mySQLRenameForms() throws { + let expected = TableEditStatement.rename( + [SQLRenamePair(from: Self.name(Self.bare("a")), to: Self.name(Self.bare("b")))], kind: .table + ) + #expect(try Self.parse("ALTER TABLE a RENAME AS b", .mysql) == expected) + #expect(try Self.parse("ALTER TABLE a RENAME b", .mysql) == expected) + #expect(try Self.parse("ALTER TABLE a RENAME b", .postgresql) == .other) + } + + @Test("A column, index or constraint rename is not a table rename") + func columnRenamesAreNotRead() throws { + #expect(try Self.parse("ALTER TABLE a RENAME COLUMN x TO y", .postgresql) == .other) + #expect(try Self.parse("ALTER TABLE a RENAME x TO y", .postgresql) == .other) + #expect(try Self.parse("ALTER TABLE a RENAME x TO y", .sqlite) == .other) + #expect(try Self.parse("ALTER TABLE a RENAME INDEX i TO j", .mysql) == .other) + #expect(try Self.parse("ALTER TABLE a RENAME CONSTRAINT c TO d", .postgresql) == .other) + #expect(try Self.parse("ALTER TABLE a RENAME TO b, ADD COLUMN c INT", .mysql) == .other) + } + + @Test("RENAME TABLE reads every pair in order") + func renameTablePairs() throws { + #expect( + try Self.parse("RENAME TABLE a TO tmp, b TO a, tmp TO b", .mysql) + == .rename( + [ + SQLRenamePair(from: Self.name(Self.bare("a")), to: Self.name(Self.bare("tmp"))), + SQLRenamePair(from: Self.name(Self.bare("b")), to: Self.name(Self.bare("a"))), + SQLRenamePair(from: Self.name(Self.bare("tmp")), to: Self.name(Self.bare("b"))) + ], + kind: .table + ) + ) + #expect( + try Self.parse("RENAME TABLE db.a TO db.b ON CLUSTER main", .clickhouse) + == .rename( + [SQLRenamePair(from: Self.name(Self.bare("db"), Self.bare("a")), to: Self.name(Self.bare("db"), Self.bare("b")))], + kind: .table + ) + ) + } + + @Test("RENAME USER and Oracle's keyword-less RENAME are not read") + func otherRenames() throws { + #expect(try Self.parse("RENAME USER 'a'@'h' TO 'b'@'h'", .mysql) == .other) + #expect(try Self.parse("RENAME emp TO staff", .oracle) == .other) + } + + // MARK: - Transactions and context + + @Test("Transaction control is read the way each engine writes it") + func transactionControl() throws { + #expect(try Self.parse("BEGIN", .postgresql) == .beginsTransaction) + #expect(try Self.parse("START TRANSACTION", .mysql) == .beginsTransaction) + #expect(try Self.parse("BEGIN TRAN", .mssql) == .beginsTransaction) + #expect(try Self.parse("BEGIN TRY", .mssql) == .other) + #expect(try Self.parse("BEGIN IMMEDIATE", .sqlite) == .beginsTransaction) + #expect(try Self.parse("BEGIN NULL; END", .oracle) == .runsUnseenCode) + #expect(try Self.parse("COMMIT", .postgresql) == .commits) + #expect(try Self.parse("END", .postgresql) == .commits) + #expect(try Self.parse("END", .mssql) == .other) + #expect(try Self.parse("ROLLBACK", .postgresql) == .rollsBack) + #expect(try Self.parse("ABORT", .postgresql) == .rollsBack) + #expect(try Self.parse("ROLLBACK TO SAVEPOINT s", .postgresql) == .rollsBackToSavepoint) + #expect(try Self.parse("ROLLBACK TRAN s", .mssql) == .rollsBackToSavepoint) + #expect(try Self.parse("ROLLBACK AND NO CHAIN", .mysql) == .rollsBack) + #expect(try Self.parse("COMMIT AND CHAIN", .mysql) == .losesTransactionTracking) + #expect(try Self.parse("COMMIT PREPARED 'x'", .postgresql) == .other) + #expect(try Self.parse("PREPARE TRANSACTION 'x'", .postgresql) == .losesTransactionTracking) + #expect(try Self.parse("SAVEPOINT s", .sqlite) == .beginsTransaction) + #expect(try Self.parse("RELEASE s", .sqlite) == .commits) + #expect(try Self.parse("SET IMPLICIT_TRANSACTIONS ON", .mssql) == .losesTransactionTracking) + #expect(try Self.parse("SET ANSI_NULLS, IMPLICIT_TRANSACTIONS ON", .mssql) == .losesTransactionTracking) + } + + @Test("A statement that moves where a bare name points is read as doing so") + func nameContext() throws { + #expect(try Self.parse("USE `archive`", .mysql) == .selectsDatabase(Self.name(Self.quoted("archive")))) + #expect(try Self.parse("SET search_path TO app, public", .postgresql) == .losesSchemaContext) + #expect(try Self.parse("SET LOCAL search_path = app", .postgresql) == .losesSchemaContext) + #expect(try Self.parse("RESET ALL", .postgresql) == .losesSchemaContext) + #expect(try Self.parse("ALTER SESSION SET CURRENT_SCHEMA = hr", .oracle) == .losesSchemaContext) + #expect(try Self.parse("EXECUTE AS USER = 'x'", .mssql) == .losesSchemaContext) + #expect(try Self.parse("SET NAMES utf8mb4", .mysql) == .other) + } + + @Test("A temporary table's creation is read with its name") + func temporaryTableCreation() throws { + #expect( + try Self.parse("CREATE TEMPORARY TABLE IF NOT EXISTS people (id int)", .mysql) + == .createsTemporaryTable(Self.name(Self.bare("people"))) + ) + #expect( + try Self.parse("CREATE TEMP TABLE people AS SELECT 1", .postgresql) + == .createsTemporaryTable(Self.name(Self.bare("people"))) + ) + #expect( + try Self.parse("CREATE OR REPLACE TEMP VIEW recent AS SELECT 1", .postgresql) + == .createsTemporaryTable(Self.name(Self.bare("recent"))) + ) + #expect( + try Self.parse("CREATE TABLE temp.people (id int)", .sqlite) + == .createsTemporaryTable(Self.name(Self.bare("temp"), Self.bare("people"))) + ) + #expect( + try Self.parse("CREATE TABLE pg_temp.people (id int)", .postgresql) + == .createsTemporaryTable(Self.name(Self.bare("pg_temp"), Self.bare("people"))) + ) + #expect(try Self.parse("CREATE TABLE people (id int)", .postgresql) == .other) + #expect(try Self.parse("CREATE TABLE archive.people (id int)", .sqlite) == .other) + #expect(try Self.parse("CREATE OR REPLACE VIEW v AS SELECT 1", .postgresql) == .other) + } + + @Test("A procedure, a prepared statement or an anonymous block is code the text does not show") + func unseenCode() throws { + #expect(try Self.parse("CALL rebuild_people()", .mysql) == .runsUnseenCode) + #expect(try Self.parse("DO $$ BEGIN CREATE TEMP TABLE t (id int); END $$", .postgresql) == .runsUnseenCode) + #expect(try Self.parse("EXECUTE make_temp", .postgresql) == .runsUnseenCode) + #expect(try Self.parse("EXEC dbo.load_staging", .mssql) == .runsUnseenCode) + #expect(try Self.parse("EXEC('DROP TABLE dbo.t')", .mssql) == .runsUnseenCode) + #expect(try Self.parse("BEGIN NOT ATOMIC CREATE TEMPORARY TABLE t (id int); END", .mariadb) == .runsUnseenCode) + #expect(try Self.parse("DECLARE n NUMBER; BEGIN NULL; END", .oracle) == .runsUnseenCode) + #expect(try Self.parse("DECLARE @n int", .mssql) == .other) + } + + @Test("Only a drop or a rename edits a table, and only what moves where a name points is a hazard") + func statementKinds() throws { + #expect(try Self.parse("DROP TABLE t", .postgresql).editsTable) + #expect(try Self.parse("ALTER TABLE t RENAME TO u", .postgresql).editsTable) + #expect(try !Self.parse("SELECT * FROM t", .postgresql).editsTable) + #expect(try !Self.parse("BEGIN", .postgresql).editsTable) + #expect(try Self.parse("CREATE TEMP TABLE t (id int)", .postgresql).changesNameHazards) + #expect(try Self.parse("SET search_path TO app", .postgresql).changesNameHazards) + #expect(try Self.parse("CALL p()", .mysql).changesNameHazards) + #expect(try !Self.parse("DROP TEMPORARY TABLE t", .mysql).changesNameHazards) + #expect(try !Self.parse("DROP TABLE t", .postgresql).changesNameHazards) + } +} + +struct TableEditDialectTests { + private static func part(_ text: String, quoted: Bool = false) -> SQLNamePart { + SQLNamePart(text: text, isQuoted: quoted) + } + + private static let context = TableNameContext(database: "shop", schema: "public") + + @Test("Engines with a dialect the app knows are covered, and the rest are not") + func coverage() { + for type in [DatabaseType.mysql, .mariadb, .postgresql, .redshift, .sqlite, .mssql, .oracle, .clickhouse, .duckdb] { + #expect(TableEditDialect.of(type) != nil, "\(type.rawValue) should be covered") + } + for type in [DatabaseType.cockroachdb, .snowflake, .mongodb, .redis, .bigQuery, .trino, .cassandra] { + #expect(TableEditDialect.of(type) == nil, "\(type.rawValue) should be left alone") + } + } + + @Test("PostgreSQL folds a bare word to lowercase, keeps a quoted one as written, and places no bare name") + func postgresFolding() { + let dialect = TableEditDialect.postgreSQL + #expect(dialect.resolve(SQLObjectName(parts: [Self.part("People")]), in: Self.context) == nil) + #expect( + dialect.resolve(SQLObjectName(parts: [Self.part("public"), Self.part("People", quoted: true)]), in: Self.context) + == TablePlacement(database: "shop", schema: "public", name: "People") + ) + #expect( + dialect.resolve(SQLObjectName(parts: [Self.part("Sales"), Self.part("Orders")]), in: Self.context) + == TablePlacement(database: "shop", schema: "sales", name: "orders") + ) + #expect( + dialect.resolve(SQLObjectName(parts: [Self.part("shop"), Self.part("app"), Self.part("t")]), in: Self.context) + == TablePlacement(database: "shop", schema: "app", name: "t") + ) + } + + @Test("Oracle folds a bare name to uppercase") + func oracleFolding() { + let context = TableNameContext(database: "ORCL", schema: "HR") + #expect( + TableEditDialect.oracle.resolve(SQLObjectName(parts: [Self.part("emp")]), in: context) + == TablePlacement(database: "ORCL", schema: "HR", name: "EMP") + ) + } + + @Test("A bare word with letters outside ASCII is left unplaced where the engine folds case") + func nonASCIIFoldingIsUnsure() { + let unquoted = SQLObjectName(parts: [Self.part("public"), Self.part("Ärger")]) + let quoted = SQLObjectName(parts: [Self.part("public"), Self.part("Ärger", quoted: true)]) + #expect(TableEditDialect.postgreSQL.resolve(unquoted, in: Self.context) == nil) + #expect( + TableEditDialect.postgreSQL.resolve(quoted, in: Self.context) + == TablePlacement(database: "shop", schema: "public", name: "Ärger") + ) + #expect( + TableEditDialect.mySQL.resolve(SQLObjectName(parts: [Self.part("Ärger")]), in: TableNameContext(database: "shop")) + == TablePlacement(database: "shop", schema: nil, name: "Ärger") + ) + } + + @Test("MySQL qualifies by database and keys no schema") + func mySQLQualification() { + let context = TableNameContext(database: "shop", schema: nil) + #expect( + TableEditDialect.mySQL.resolve(SQLObjectName(parts: [Self.part("Orders")]), in: context) + == TablePlacement(database: "shop", schema: nil, name: "Orders") + ) + #expect( + TableEditDialect.mySQL.resolve(SQLObjectName(parts: [Self.part("archive"), Self.part("Orders")]), in: context) + == TablePlacement(database: "archive", schema: nil, name: "Orders") + ) + #expect(TableEditDialect.mySQL.resolve(SQLObjectName(parts: [Self.part("a"), Self.part("b"), Self.part("c")]), in: context) == nil) + } + + @Test("SQL Server places only a qualified name, because a bare one follows the login's default schema") + func sqlServerNeedsQualification() { + let context = TableNameContext(database: "sales", schema: "reporting") + #expect(TableEditDialect.sqlServer.resolve(SQLObjectName(parts: [Self.part("t")]), in: context) == nil) + #expect( + TableEditDialect.sqlServer.resolve(SQLObjectName(parts: [Self.part("dbo"), Self.part("t")]), in: context) + == TablePlacement(database: "sales", schema: "dbo", name: "t") + ) + } + + @Test("A two-part name is not placed where the engine reads it more than one way") + func ambiguousTwoPartNames() { + let name = SQLObjectName(parts: [Self.part("aux"), Self.part("t")]) + #expect(TableEditDialect.duckDB.resolve(name, in: Self.context) == nil) + #expect(TableEditDialect.sqlite.resolve(name, in: TableNameContext(database: "/tmp/app.db", schema: nil)) == nil) + } + + @Test("A bare name has no place once the context it resolves in is unknown") + func unknownContext() { + #expect( + TableEditDialect.oracle.resolve( + SQLObjectName(parts: [Self.part("t")]), in: TableNameContext(database: "ORCL", schema: nil) + ) == nil + ) + #expect( + TableEditDialect.mySQL.resolve(SQLObjectName(parts: [Self.part("t")]), in: TableNameContext(database: nil)) == nil + ) + } + + @Test("USE moves the database only where a single name selects one") + func useContext() { + let archive = SQLObjectName(parts: [Self.part("archive")]) + #expect(TableEditDialect.mySQL.context(afterUsing: archive) == TableNameContext(database: "archive", schema: nil)) + #expect(TableEditDialect.duckDB.context(afterUsing: archive) == TableNameContext(database: nil, schema: nil)) + #expect(TableEditDialect.mySQL.context(afterUsing: nil) == TableNameContext(database: nil, schema: nil)) + } +} diff --git a/docs/features/table-operations.mdx b/docs/features/table-operations.mdx index 362abc5598..d31ab9ded0 100644 --- a/docs/features/table-operations.mdx +++ b/docs/features/table-operations.mdx @@ -67,6 +67,12 @@ No other engine has a rename, so the item never appears on Cassandra, DynamoDB, **Rename Database** and **Rename Schema** are absent on the container the connection is browsing. Switch to another one first, then rename the one you left. +## Drop or rename with SQL + +A `DROP TABLE`, `DROP VIEW`, `ALTER TABLE ... RENAME TO`, `RENAME TABLE` or SQL Server `sp_rename` run in a query tab, or by an MCP client, counts as a sidebar drop or rename once it commits. Tabs on the table close or take the new name, and its saved filters, column layout, highlight rules, value formats, favorite and Recent entry go with it. A drop that is rolled back, or is still inside an open transaction when the run ends, leaves all of them in place. + +This works on MySQL, MariaDB, PostgreSQL, Redshift, SQLite, SQL Server, Oracle, ClickHouse and DuckDB. On PostgreSQL, Redshift and SQL Server, name the schema, as in `public.orders`, and on DuckDB the database and the schema, because a bare name there resolves through session state such as the search path or the login's default schema. On the other engines a name stops being followed once something on the connection could have redirected it: a temporary table of the same name, a procedure call or anonymous block, or a statement that changes the current schema. A rename written with `IF EXISTS` is never followed, since it succeeds when there was nothing to rename. + ## Maintenance Right-click a table, choose **Maintenance**, and pick an operation. The sheet shows the operation's options and the statement the driver will run, rebuilt as each option changes, so what is previewed is what executes. From 2a83ab09493784a5779a7ba81c60b36325cfdb9b Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Tue, 29 Sep 2026 22:26:37 +0700 Subject: [PATCH 2/4] fix(editor): adopt a SQL drop only when the session holds no transaction after the run as well as before --- CHANGELOG.md | 2 +- .../QueryExecutionCoordinator+Batches.swift | 4 +- ...QueryExecutionCoordinator+Parameters.swift | 36 +++- .../Access/DatabaseAccessBridge+Scripts.swift | 53 ++++-- .../Core/Events/SucceededStatements.swift | 29 ++- .../Services/Query/CommittedTableEdits.swift | 71 ++++++-- .../Query/CommittedTableEditsTests.swift | 166 ++++++++++++++++-- .../Query/SQLTableEditAdoptionTests.swift | 21 ++- .../Helpers/ScriptAnsweringDriver.swift | 8 +- docs/features/table-operations.mdx | 2 +- 10 files changed, 328 insertions(+), 64 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4fe16b1169..cda64dffe9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - iOS row editor saving the placeholder of a long text or binary value over the full value. - Explain Analyze running write statements on Read-Only connections and skipping the Alert and Safe Mode confirmation. - **Local only** connections taking edits and deletions made on another device. -- Saved filters, layout, highlight rules, favorite and Recent entry kept by a table dropped or renamed in SQL. +- Saved filters, layout, favorite and Recent entry kept by a table dropped or renamed from a query tab or MCP client. ## [0.76.1] - 2026-09-29 diff --git a/TablePro/Core/Coordinators/QueryExecutionCoordinator+Batches.swift b/TablePro/Core/Coordinators/QueryExecutionCoordinator+Batches.swift index eaa8734001..bf8c446db4 100644 --- a/TablePro/Core/Coordinators/QueryExecutionCoordinator+Batches.swift +++ b/TablePro/Core/Coordinators/QueryExecutionCoordinator+Batches.swift @@ -340,7 +340,9 @@ extension QueryExecutionCoordinator { scope: scope, databaseType: connection.type, statements: prepared.prefix(run.outcome.succeededCount).flatMap { $0.batch.statements.map(\.sql) }, - commit: .run(startedIn: run.startState, plan: run.plan, completed: run.outcome.isCompleted) + commit: .run( + startedIn: run.startState, endedIn: run.sessionState, plan: run.plan, completed: run.outcome.isCompleted + ) ))) } diff --git a/TablePro/Core/Coordinators/QueryExecutionCoordinator+Parameters.swift b/TablePro/Core/Coordinators/QueryExecutionCoordinator+Parameters.swift index 01355fc2b9..c5a8d2981d 100644 --- a/TablePro/Core/Coordinators/QueryExecutionCoordinator+Parameters.swift +++ b/TablePro/Core/Coordinators/QueryExecutionCoordinator+Parameters.swift @@ -28,6 +28,9 @@ private struct MultiStatementRun { let plan: BatchTransactionPlan let sessionState: PluginSessionTransactionState let startState: PluginSessionTransactionState + /// What the session held once the run ended, asked of a run that joined the session's + /// transaction or drops or renames a table, and `.unknown` for any other. + let endState: PluginSessionTransactionState var failureOutput: PluginServerOutput = .none } @@ -317,6 +320,9 @@ extension QueryExecutionCoordinator { grammar: grammar ) } + let asksEndState = SucceededStatements.runNeedsEndState( + prepared.map(\.sentSQL), databaseType: conn.type, grammar: grammar + ) let multiStatementTask = Task { [weak self, parent] in guard let self else { return } @@ -326,6 +332,7 @@ extension QueryExecutionCoordinator { scope: scope, mode: transactionKind.transactionAccessMode, plan: plan, + asksEndState: asksEndState, claim: claim, lease: lease ) @@ -349,7 +356,9 @@ extension QueryExecutionCoordinator { scope: scope, databaseType: conn.type, statements: prepared.prefix(outcome.succeededCount).map(\.sentSQL), - commit: .run(startedIn: run.startState, plan: run.plan, completed: outcome.isCompleted) + commit: .run( + startedIn: run.startState, endedIn: run.endState, plan: run.plan, completed: outcome.isCompleted + ) ))) switch outcome { @@ -436,6 +445,7 @@ extension QueryExecutionCoordinator { scope: DatabaseScope, mode: PluginTransactionAccessMode, plan: BatchTransactionPlan, + asksEndState: Bool, claim: TabExecutionClaim, lease: DriverLeaseOwner ) async -> MultiStatementRun { @@ -468,15 +478,24 @@ extension QueryExecutionCoordinator { } } guard sessionPlan == .sessionTransaction else { + let endState: PluginSessionTransactionState = asksEndState + ? await driver.heldSessionTransactionState() + : .unknown return MultiStatementRun( - outcome: outcome, plan: sessionPlan, sessionState: .idle, startState: startState + outcome: outcome, + plan: sessionPlan, + sessionState: .idle, + startState: startState, + endState: endState ) } + let sessionState = await driver.heldSessionTransactionState() return MultiStatementRun( outcome: outcome, plan: sessionPlan, - sessionState: await driver.heldSessionTransactionState(), - startState: startState + sessionState: sessionState, + startState: startState, + endState: sessionState ) } run.failureOutput = failureOutput.output @@ -484,14 +503,19 @@ extension QueryExecutionCoordinator { } catch { if DatabaseCancellationDiagnosis.isCancellation(error) || Task.isCancelled { return MultiStatementRun( - outcome: .cancelled(results: []), plan: plan, sessionState: .unknown, startState: .unknown + outcome: .cancelled(results: []), + plan: plan, + sessionState: .unknown, + startState: .unknown, + endState: .unknown ) } return MultiStatementRun( outcome: .failed(results: [], failure: .connection, errorDescription: error.localizedDescription), plan: plan, sessionState: .unknown, - startState: .unknown + startState: .unknown, + endState: .unknown ) } } diff --git a/TablePro/Core/Database/Access/DatabaseAccessBridge+Scripts.swift b/TablePro/Core/Database/Access/DatabaseAccessBridge+Scripts.swift index 163483993f..121a607348 100644 --- a/TablePro/Core/Database/Access/DatabaseAccessBridge+Scripts.swift +++ b/TablePro/Core/Database/Access/DatabaseAccessBridge+Scripts.swift @@ -143,30 +143,24 @@ extension DatabaseAccessBridge { return run.outcome(executionTimeMs: (CFAbsoluteTimeGetCurrent() - startTime) * 1_000) } - /// A batch that finished before a later one failed has already committed on SQL Server, whose - /// scripts run with no transaction of the app's around them, so what it dropped or renamed is - /// reported whatever became of the rest. private static func postSucceededBatches( of batches: [ExecutableBatch], progress: ScriptBatchProgress, scope: DatabaseScope, databaseType: DatabaseType ) { - let completed = progress.completedBatchCount - guard completed > 0 else { return } - CatalogChangeService.post(.statementsSucceeded(SucceededStatements( - scope: scope, - databaseType: databaseType, - statements: batches.prefix(completed).flatMap { $0.statements.map(\.sql) }, - commit: .runStartedIn(progress.startState, appTransaction: .none) - ))) + guard let succeeded = progress.succeededStatements(of: batches, scope: scope, databaseType: databaseType) else { + return + } + CatalogChangeService.post(.statementsSucceeded(succeeded)) } } -/// How far a script got, readable after it threw. +/// How far a script got, and what the session held before and after it, readable after it threw. final class ScriptBatchProgress: Sendable { private struct State { var startState: PluginSessionTransactionState = .unknown + var endState: PluginSessionTransactionState = .unknown var completedBatchCount = 0 } @@ -176,6 +170,10 @@ final class ScriptBatchProgress: Sendable { state.withLock { $0.startState } } + var endState: PluginSessionTransactionState { + state.withLock { $0.endState } + } + var completedBatchCount: Int { state.withLock { $0.completedBatchCount } } @@ -184,9 +182,32 @@ final class ScriptBatchProgress: Sendable { state.withLock { $0.startState = startState } } + func end(in endState: PluginSessionTransactionState) { + state.withLock { $0.endState = endState } + } + func completeBatch() { state.withLock { $0.completedBatchCount += 1 } } + + /// A batch that finished before a later one failed has already committed on SQL Server, whose + /// scripts run with no transaction of the app's around them, so what it dropped or renamed is + /// reported whatever became of the rest. A script stopped by a timeout or a lost connection + /// never hears what the session held at the end, and adopts nothing. + func succeededStatements( + of batches: [ExecutableBatch], + scope: DatabaseScope, + databaseType: DatabaseType + ) -> SucceededStatements? { + let current = state.withLock { $0 } + guard current.completedBatchCount > 0 else { return nil } + return SucceededStatements( + scope: scope, + databaseType: databaseType, + statements: batches.prefix(current.completedBatchCount).flatMap { $0.statements.map(\.sql) }, + commit: .runStartedIn(current.startState, endedIn: current.endState, appTransaction: .none) + ) + } } /// What a script's batches answered with, and what the session held once they had all run. @@ -213,20 +234,24 @@ struct ScriptBatchRun: Sendable { let answer = try await repeatedAnswer(to: batch, rowCap: rowCap, driver: driver) answers.append(answer) guard answer.errors.isEmpty else { + let endState = await driver.heldSessionTransactionState() + progress.end(in: endState) let context = MultiStatementFailureContext( failure: .batch(sql: batch.sql), errorDescription: BatchErrorText.describe(answer.errors, batchStartLine: startLine) ?? "", executedCount: answers.count, totalCount: batches.count, plan: plan, - sessionState: await driver.heldSessionTransactionState(), + sessionState: endState, unit: .batch ) throw DatabaseError.queryFailed(context.report().message) } progress.completeBatch() } - return ScriptBatchRun(answers: answers, sessionState: await driver.heldSessionTransactionState()) + let endState = await driver.heldSessionTransactionState() + progress.end(in: endState) + return ScriptBatchRun(answers: answers, sessionState: endState) } /// `GO 5` sends the batch five times and keeps every answer. A repetition that raised an error ends the diff --git a/TablePro/Core/Events/SucceededStatements.swift b/TablePro/Core/Events/SucceededStatements.swift index 046e942d4b..156a74b96e 100644 --- a/TablePro/Core/Events/SucceededStatements.swift +++ b/TablePro/Core/Events/SucceededStatements.swift @@ -21,17 +21,22 @@ enum AppTransactionOutcome: Sendable, Equatable { enum StatementCommitEvidence: Sendable, Equatable { /// One statement, and what the session held once it had run. case statementLeftSession(PluginSessionTransactionState) - /// Several statements, what the session held before the first of them, and what became of a - /// transaction the app opened around them. - case runStartedIn(PluginSessionTransactionState, appTransaction: AppTransactionOutcome) + /// Several statements, what the session held before the first of them and once the last had + /// run, and what became of a transaction the app opened around them. + case runStartedIn( + PluginSessionTransactionState, + endedIn: PluginSessionTransactionState, + appTransaction: AppTransactionOutcome + ) static func run( - startedIn state: PluginSessionTransactionState, + startedIn start: PluginSessionTransactionState, + endedIn end: PluginSessionTransactionState, plan: BatchTransactionPlan, completed: Bool ) -> StatementCommitEvidence { - guard plan.opensTransaction else { return .runStartedIn(state, appTransaction: .none) } - return .runStartedIn(state, appTransaction: completed ? .committed : .rolledBack) + guard plan.opensTransaction else { return .runStartedIn(start, endedIn: end, appTransaction: .none) } + return .runStartedIn(start, endedIn: end, appTransaction: completed ? .committed : .rolledBack) } } @@ -66,6 +71,18 @@ struct SucceededStatements: Sendable, Equatable { scope: scope, databaseType: databaseType, statements: [sql], commit: .statementLeftSession(state) ) } + + /// Whether a run of these statements has to ask the session what it holds once the last has + /// run. Only a drop or a rename on an engine whose DDL a transaction can take back is judged by + /// the answer, and on SQL Server asking is a round trip. + static func runNeedsEndState( + _ statements: [String], + databaseType: DatabaseType, + grammar: SQLLexicalGrammar + ) -> Bool { + guard let dialect = TableEditDialect.of(databaseType), !dialect.commitsDDLImplicitly else { return false } + return statements.contains { TableEditStatementParser.parse($0, dialect: dialect, grammar: grammar).editsTable } + } } extension PluginSessionTransactionState { diff --git a/TablePro/Core/Services/Query/CommittedTableEdits.swift b/TablePro/Core/Services/Query/CommittedTableEdits.swift index aa0a4e44a7..ca5be4afbe 100644 --- a/TablePro/Core/Services/Query/CommittedTableEdits.swift +++ b/TablePro/Core/Services/Query/CommittedTableEdits.swift @@ -55,9 +55,12 @@ struct TableNameHazards: Sendable, Equatable { /// /// Success is not enough on an engine whose DDL is transactional: `BEGIN; DROP TABLE people; /// ROLLBACK` succeeds three times and leaves the table where it was. So the text's own `BEGIN`, -/// `COMMIT` and `ROLLBACK` are followed from a session known to hold no transaction, and an edit -/// still inside one when the statements end is dropped, because nothing here will see how it ends. -/// A run that began inside a transaction, or on a session that could not say, adopts nothing. +/// `COMMIT` and `ROLLBACK` are followed, and only between two answers from the session that it +/// held no transaction, one before the first statement and one after the last. The text alone +/// cannot be trusted even then: under SQL Server's `IMPLICIT_TRANSACTIONS`, set by an earlier run +/// or by the server's defaults, a `DROP` opens a transaction no statement shows. A run that ends +/// inside a transaction, closes one it never opened, or leaves one open that the session says is +/// closed has met such a transaction, and adopts nothing. enum CommittedTableEdits { static func edits( in succeeded: SucceededStatements, @@ -82,9 +85,9 @@ private struct TableEditWalk { /// is SQL Server's `@@TRANCOUNT`; on engines where one `COMMIT` ends everything this can only /// hold an edit back, never let one through early. private var depth = 0 - /// False when the session held a transaction the text cannot see the end of, in which case - /// the walk only keeps the hazards current. - private let adopts: Bool + /// False when the session held a transaction at either end, or the text and the session + /// disagreed about one, in which case the walk only keeps the hazards current. + private var adopts: Bool private var tracksTransactions = true private var pending: [TableCatalogEdit] = [] private(set) var committed: [TableCatalogEdit] = [] @@ -93,28 +96,33 @@ private struct TableEditWalk { init(dialect: TableEditDialect, scope: DatabaseScope, commit: StatementCommitEvidence, hazards: TableNameHazards) { self.dialect = dialect context = TableNameContext(database: scope.database.nilIfEmpty, schema: scope.schema) - adopts = Self.startsOutsideTransactions(commit, dialect: dialect) + adopts = Self.runsOutsideTransactions(commit, dialect: dialect) self.hazards = hazards - if case .runStartedIn(_, .committed) = commit { + if case .runStartedIn(_, _, .committed) = commit { depth = 1 } } - private static func startsOutsideTransactions(_ evidence: StatementCommitEvidence, dialect: TableEditDialect) -> Bool { + private static func runsOutsideTransactions(_ evidence: StatementCommitEvidence, dialect: TableEditDialect) -> Bool { guard !dialect.commitsDDLImplicitly else { return true } switch evidence { case .statementLeftSession(let state): return state.holdsNoTransaction - case .runStartedIn(let state, let appTransaction): - return state.holdsNoTransaction && appTransaction != .rolledBack + case .runStartedIn(let start, let end, let appTransaction): + return start.holdsNoTransaction && end.holdsNoTransaction && appTransaction != .rolledBack } } /// The app's own `COMMIT`, which closes the transaction the walk opened for it, unless a - /// `ROLLBACK` or `COMMIT` in the text already ended it. + /// `ROLLBACK` or `COMMIT` in the text already ended it. A transaction the text still holds + /// after that was closed by something it does not show, since the session says none is open. mutating func finish(_ evidence: StatementCommitEvidence) { - guard case .runStartedIn(_, .committed) = evidence else { return } - read(.commits) + if case .runStartedIn(_, _, .committed) = evidence, depth > 0 { + endTransaction() + } + if depth > 0 { + disown() + } } mutating func read(_ statement: TableEditStatement) { @@ -130,15 +138,23 @@ private struct TableEditWalk { case .beginsTransaction: depth += 1 case .commits: - guard depth > 0 else { return } - depth -= 1 - guard depth == 0 else { return } - committed += pending - pending.removeAll() + guard depth > 0 else { + disown() + return + } + endTransaction() case .rollsBack: + guard depth > 0 else { + disown() + return + } pending.removeAll() depth = 0 case .rollsBackToSavepoint: + guard depth > 0 else { + disown() + return + } pending.removeAll() case .losesTransactionTracking: pending.removeAll() @@ -152,6 +168,23 @@ private struct TableEditWalk { } } + private mutating func endTransaction() { + depth -= 1 + guard depth == 0 else { return } + committed += pending + pending.removeAll() + } + + /// The text and the session disagree about the transaction, so what the text says was + /// committed is not known to be. On PostgreSQL a stray `COMMIT` or `ROLLBACK` is only a + /// warning, and this holds back a drop that did commit, which keeps its settings in place. + private mutating func disown() { + guard !dialect.commitsDDLImplicitly else { return } + adopts = false + committed.removeAll() + pending.removeAll() + } + private func place(_ name: SQLObjectName) -> TablePlacement? { guard let table = dialect.resolve(name, in: context), !hazards.mayShadow(name, placedAs: table, dialect: dialect) else { return nil } diff --git a/TableProTests/Core/Services/Query/CommittedTableEditsTests.swift b/TableProTests/Core/Services/Query/CommittedTableEditsTests.swift index 49f61d91d5..a02d7c969f 100644 --- a/TableProTests/Core/Services/Query/CommittedTableEditsTests.swift +++ b/TableProTests/Core/Services/Query/CommittedTableEditsTests.swift @@ -16,7 +16,7 @@ struct CommittedTableEditsTests { on type: DatabaseType = .postgresql, database: String = "shop", schema: String? = "public", - commit: StatementCommitEvidence = .runStartedIn(.idle, appTransaction: .none) + commit: StatementCommitEvidence = .runStartedIn(.idle, endedIn: .idle, appTransaction: .none) ) -> [TableCatalogEdit] { var hazards = TableNameHazards() return edits(statements, on: type, database: database, schema: schema, commit: commit, hazards: &hazards) @@ -27,7 +27,7 @@ struct CommittedTableEditsTests { on type: DatabaseType = .postgresql, database: String = "shop", schema: String? = "public", - commit: StatementCommitEvidence = .runStartedIn(.idle, appTransaction: .none), + commit: StatementCommitEvidence = .runStartedIn(.idle, endedIn: .idle, appTransaction: .none), hazards: inout TableNameHazards ) -> [TableCatalogEdit] { let succeeded = SucceededStatements( @@ -49,6 +49,13 @@ struct CommittedTableEditsTests { .renamed(TablePlacement(database: database, schema: schema, name: name), to: newName, kind: .table) } + private static func sqlServerEdits( + _ statements: [String], + commit: StatementCommitEvidence = .runStartedIn(.idle, endedIn: .idle, appTransaction: .none) + ) -> [TableCatalogEdit] { + edits(statements, on: .mssql, database: "sales", schema: "dbo", commit: commit) + } + // MARK: - The reported case @Test("A table dropped and created again under the same name is a drop of the old one") @@ -127,11 +134,16 @@ struct CommittedTableEditsTests { #expect(Self.edits(["BEGIN", "DROP TABLE public.people", "ROLLBACK TO SAVEPOINT s", "COMMIT"]).isEmpty) } - @Test("A drop the script commits is a drop, and one still open at the end is not") + @Test("A drop the script commits is a drop, and a run that ends with a transaction open adopts nothing") func committedAndOpenDrops() { #expect(Self.edits(["BEGIN", "DROP TABLE public.people", "COMMIT"]) == [Self.dropped("people")]) #expect(Self.edits(["BEGIN", "DROP TABLE public.people", "END"]) == [Self.dropped("people")]) - #expect(Self.edits(["DROP TABLE public.a", "BEGIN", "DROP TABLE public.b"]) == [Self.dropped("a")]) + #expect( + Self.edits( + ["DROP TABLE public.a", "BEGIN", "DROP TABLE public.b"], + commit: .runStartedIn(.idle, endedIn: .inTransaction, appTransaction: .none) + ).isEmpty + ) } @Test("A COMMIT inside a nested transaction does not commit the outer one") @@ -153,33 +165,97 @@ struct CommittedTableEditsTests { ) } - @Test("A run that started inside a transaction, or on a session that could not say, is not read") + @Test("A run that started or ended inside a transaction, or on a session that could not say, is not read") func runStartedInTransaction() { #expect( - Self.edits(["DROP TABLE public.people"], commit: .runStartedIn(.inTransaction, appTransaction: .none)) - .isEmpty + Self.edits( + ["DROP TABLE public.people"], commit: .runStartedIn(.inTransaction, endedIn: .idle, appTransaction: .none) + ).isEmpty + ) + #expect( + Self.edits( + ["DROP TABLE public.people", "COMMIT"], commit: .runStartedIn(.unknown, endedIn: .idle, appTransaction: .none) + ).isEmpty + ) + #expect( + Self.edits( + ["DROP TABLE public.people"], commit: .runStartedIn(.idle, endedIn: .unknown, appTransaction: .none) + ).isEmpty ) #expect( - Self.edits(["DROP TABLE public.people", "COMMIT"], commit: .runStartedIn(.unknown, appTransaction: .none)) + Self.edits( + ["DROP TABLE public.people"], commit: .runStartedIn(.idle, endedIn: .abortedTransaction, appTransaction: .none) + ).isEmpty + ) + } + + /// `SET IMPLICIT_TRANSACTIONS ON` in an earlier run leaves `@@TRANCOUNT` at 0 until the next + /// `DROP`, which then opens a transaction no statement in the script shows. + @Test("A SQL Server script run under implicit transactions from an earlier run adopts nothing") + func implicitTransactionsFromAnEarlierRun() { + let endsOpen = StatementCommitEvidence.runStartedIn(.idle, endedIn: .inTransaction, appTransaction: .none) + #expect(Self.sqlServerEdits(["DROP TABLE dbo.people", "SELECT 1"], commit: endsOpen).isEmpty) + #expect(Self.sqlServerEdits(["DROP TABLE dbo.people", "ROLLBACK"]).isEmpty) + #expect(Self.sqlServerEdits(["DROP TABLE dbo.people", "COMMIT"]).isEmpty) + #expect( + Self.sqlServerEdits(["DROP TABLE dbo.a", "BEGIN TRAN", "DROP TABLE dbo.b", "COMMIT TRAN"], commit: endsOpen) .isEmpty ) + #expect( + Self.sqlServerEdits( + ["DROP TABLE dbo.people"], + commit: .run(startedIn: .idle, endedIn: .inTransaction, plan: .appTransaction, completed: true) + ).isEmpty + ) + } + + /// Under `XACT_ABORT` a failed statement rolls back everything, the `DROP` an implicit + /// transaction held included, and the run reports only the statements before it. + @Test("A transaction the text left open that the session says is closed was ended unseen") + func transactionClosedUnseen() { + #expect(Self.sqlServerEdits(["DROP TABLE dbo.a", "BEGIN TRAN"]).isEmpty) + #expect(Self.edits(["DROP TABLE public.a", "BEGIN", "DROP TABLE public.b"]).isEmpty) } + @Test("A run on a SQL Server session that commits as it goes adopts what it dropped") + func sqlServerAutocommitRun() { + #expect( + Self.sqlServerEdits(["DROP TABLE dbo.people", "SELECT 1"]) + == [Self.dropped("people", database: "sales", schema: "dbo")] + ) + } + + @Test("A stray COMMIT or ROLLBACK changes nothing on an engine that commits DDL as it runs") + func strayTransactionControlOnImplicitCommitEngines() { + #expect(Self.edits(["DROP TABLE people", "ROLLBACK"], on: .mysql, schema: nil) == [Self.dropped("people", schema: nil)]) + #expect( + Self.edits( + ["DROP TABLE people", "BEGIN"], on: .mysql, schema: nil, + commit: .runStartedIn(.idle, endedIn: .inTransaction, appTransaction: .none) + ) == [Self.dropped("people", schema: nil)] + ) + } + + @Test("A run the app wrapped counts once the app committed it, and not once it rolled it back") func appTransaction() { #expect( Self.edits( ["DROP TABLE public.people", "INSERT INTO log VALUES (1)"], - commit: .run(startedIn: .idle, plan: .appTransaction, completed: true) + commit: .run(startedIn: .idle, endedIn: .idle, plan: .appTransaction, completed: true) ) == [Self.dropped("people")] ) #expect( - Self.edits(["DROP TABLE public.people"], commit: .run(startedIn: .idle, plan: .appTransaction, completed: false)) - .isEmpty + Self.edits( + ["DROP TABLE public.people"], + commit: .run(startedIn: .idle, endedIn: .idle, plan: .appTransaction, completed: false) + ).isEmpty ) #expect( - Self.edits(["DROP TABLE public.people"], commit: .run(startedIn: .idle, plan: .autocommit, completed: false)) - == [Self.dropped("people")] + Self.edits( + ["DROP TABLE public.people"], + commit: .run(startedIn: .idle, endedIn: .idle, plan: .autocommit, completed: false) + ) == [Self.dropped("people")] ) } @@ -187,7 +263,7 @@ struct CommittedTableEditsTests { /// it, and that `ROLLBACK` is what ends the app's transaction. @Test("A ROLLBACK inside a run the app wrapped takes back what came before it") func rollbackInsideTheAppTransaction() { - let wrapped = StatementCommitEvidence.run(startedIn: .idle, plan: .appTransaction, completed: true) + let wrapped = StatementCommitEvidence.run(startedIn: .idle, endedIn: .idle, plan: .appTransaction, completed: true) #expect(Self.edits(["DROP TABLE public.t", "ROLLBACK"], commit: wrapped).isEmpty) #expect( Self.edits(["DROP TABLE public.t", "ROLLBACK", "DROP TABLE public.u"], commit: wrapped) @@ -383,6 +459,22 @@ struct SucceededStatementsProbeTests { #expect(report?.commit == .statementLeftSession(.unknown)) } + @Test("A run asks the session how it ended only when it drops or renames a table on a transactional engine") + func runNeedsEndState() { + #expect(SucceededStatements.runNeedsEndState( + ["SELECT 1", "DROP TABLE dbo.people"], databaseType: .mssql, grammar: DatabaseType.mssql.lexicalGrammar + )) + #expect(!SucceededStatements.runNeedsEndState( + ["SELECT 1", "INSERT INTO t VALUES (1)"], databaseType: .postgresql, grammar: DatabaseType.postgresql.lexicalGrammar + )) + #expect(!SucceededStatements.runNeedsEndState( + ["DROP TABLE people"], databaseType: .mysql, grammar: DatabaseType.mysql.lexicalGrammar + )) + #expect(!SucceededStatements.runNeedsEndState( + ["DROP TABLE people"], databaseType: .snowflake, grammar: DatabaseType.snowflake.lexicalGrammar + )) + } + @Test("A temporary table's creation and a procedure call are reported without asking the session") func hazardsAreReported() async { let created = await Self.probe("CREATE TEMP TABLE people (id int)", on: .sqlite, state: .inTransaction) @@ -428,6 +520,52 @@ struct ScriptBatchProgressTests { #expect(batches.count == 3) #expect(progress.completedBatchCount == 1) #expect(progress.startState == .idle) + #expect(progress.endState == .idle) + let succeeded = progress.succeededStatements( + of: batches, scope: Self.scope(of: connection), databaseType: .mssql + ) + #expect(succeeded?.statements == ["DROP TABLE dbo.people"]) + var hazards = TableNameHazards() + let edits = succeeded.map { CommittedTableEdits.edits(in: $0, grammar: grammar, hazards: &hazards) } + #expect(edits == [.dropped(TablePlacement(database: "sales", schema: "dbo", name: "people"), kind: .table)]) + } + + /// `SET IMPLICIT_TRANSACTIONS ON` in an earlier call leaves `@@TRANCOUNT` at 0 until the + /// script's `DROP` opens a transaction that nothing in the script commits. + @Test("A script that leaves the session inside a transaction records it, and adopts nothing") + func scriptEndingInsideATransaction() async throws { + let connection = TestFixtures.makeConnection(database: "sales", type: .mssql) + let driver = ScriptAnsweringDriver( + connection: connection, transactionState: .idle, transactionStateAfterBatches: .inTransaction + ) + let grammar = DatabaseType.mssql.lexicalGrammar + let batches = QueryBatchPlanner.batches( + in: "DROP TABLE dbo.people\nGO\nSELECT 1", + model: QueryStatementModel.forDatabaseType(.mssql), + grammar: grammar + ) + let progress = ScriptBatchProgress() + + _ = try await ScriptBatchRun.run(batches, startLines: [1, 3], rowCap: 100, driver: driver, progress: progress) + + #expect(progress.startState == .idle) + #expect(progress.endState == .inTransaction) + let succeeded = try #require( + progress.succeededStatements(of: batches, scope: Self.scope(of: connection), databaseType: .mssql) + ) + var hazards = TableNameHazards() + #expect(CommittedTableEdits.edits(in: succeeded, grammar: grammar, hazards: &hazards).isEmpty) + } + + @Test("A script that never started reports nothing") + func scriptThatNeverStarted() { + let connection = TestFixtures.makeConnection(database: "sales", type: .mssql) + let progress = ScriptBatchProgress() + #expect(progress.succeededStatements(of: [], scope: Self.scope(of: connection), databaseType: .mssql) == nil) + } + + private static func scope(of connection: DatabaseConnection) -> DatabaseScope { + DatabaseScope(connectionId: connection.id, database: "sales", schema: "dbo") } } diff --git a/TableProTests/Core/Services/Query/SQLTableEditAdoptionTests.swift b/TableProTests/Core/Services/Query/SQLTableEditAdoptionTests.swift index 1eb8f5138a..21a36e7f5d 100644 --- a/TableProTests/Core/Services/Query/SQLTableEditAdoptionTests.swift +++ b/TableProTests/Core/Services/Query/SQLTableEditAdoptionTests.swift @@ -59,7 +59,7 @@ struct SQLTableEditAdoptionTests { func run( _ statements: [String], schema: String? = "public", - commit: StatementCommitEvidence = .runStartedIn(.idle, appTransaction: .none) + commit: StatementCommitEvidence = .runStartedIn(.idle, endedIn: .idle, appTransaction: .none) ) { service.record(.statementsSucceeded(SucceededStatements( scope: DatabaseScope(connectionId: connection.id, database: "shop", schema: schema), @@ -138,6 +138,25 @@ struct SQLTableEditAdoptionTests { #expect(harness.store.droppedTables.isEmpty) } + /// `SET IMPLICIT_TRANSACTIONS ON` in an earlier run leaves the session holding nothing until + /// the script's `DROP`, which opens a transaction a later `ROLLBACK` can still undo. + @Test("A SQL Server drop left uncommitted by an earlier run's implicit transactions keeps the settings") + func implicitTransactionDropKeepsSettings() throws { + let harness = try makeHarness(type: .mssql, browseSchema: "dbo") + defer { tearDown(harness) } + harness.favorites.addFavorite(name: "people", schema: "dbo", database: "shop", connectionId: harness.connection.id) + + harness.run( + ["DROP TABLE dbo.people", "SELECT 1"], + schema: "dbo", + commit: .runStartedIn(.idle, endedIn: .inTransaction, appTransaction: .none) + ) + harness.run(["DROP TABLE dbo.people", "ROLLBACK"], schema: "dbo") + + #expect(harness.store.droppedTables.isEmpty) + #expect(harness.favorites.favorites(for: harness.connection.id) == [harness.favorite("people", schema: "dbo")]) + } + @Test("A favorite the sidebar saved without a schema is found for a table SQL named with one") func favoriteSavedWithoutSchema() throws { let harness = try makeHarness(type: .oracle, browseSchema: "HR") diff --git a/TableProTests/Helpers/ScriptAnsweringDriver.swift b/TableProTests/Helpers/ScriptAnsweringDriver.swift index ce5af223e8..6135891b92 100644 --- a/TableProTests/Helpers/ScriptAnsweringDriver.swift +++ b/TableProTests/Helpers/ScriptAnsweringDriver.swift @@ -21,6 +21,7 @@ final class ScriptAnsweringDriver: DatabaseDriver, @unchecked Sendable { private let sendsBatchesWhole: Bool private let transactionState: PluginSessionTransactionState + private let transactionStateAfterBatches: PluginSessionTransactionState? private let answer: @Sendable (String) -> QueryBatchResult private let lock = NSLock() private var batches: [SentBatch] = [] @@ -30,11 +31,13 @@ final class ScriptAnsweringDriver: DatabaseDriver, @unchecked Sendable { connection: DatabaseConnection, sendsBatchesWhole: Bool = true, transactionState: PluginSessionTransactionState = .idle, + transactionStateAfterBatches: PluginSessionTransactionState? = nil, answer: @escaping @Sendable (String) -> QueryBatchResult = { _ in .empty } ) { self.connection = connection self.sendsBatchesWhole = sendsBatchesWhole self.transactionState = transactionState + self.transactionStateAfterBatches = transactionStateAfterBatches self.answer = answer } @@ -54,8 +57,11 @@ final class ScriptAnsweringDriver: DatabaseDriver, @unchecked Sendable { return answer(query) } + /// What the session holds, and once a batch has reached it, what it holds after one when a test + /// says that differs, the way a SQL Server `DROP` under implicit transactions opens one. func sessionTransactionState() async -> PluginSessionTransactionState { - transactionState + guard let transactionStateAfterBatches, !sentBatches.isEmpty else { return transactionState } + return transactionStateAfterBatches } private func record(_ query: String) -> QueryResult { diff --git a/docs/features/table-operations.mdx b/docs/features/table-operations.mdx index d31ab9ded0..4998628756 100644 --- a/docs/features/table-operations.mdx +++ b/docs/features/table-operations.mdx @@ -69,7 +69,7 @@ No other engine has a rename, so the item never appears on Cassandra, DynamoDB, ## Drop or rename with SQL -A `DROP TABLE`, `DROP VIEW`, `ALTER TABLE ... RENAME TO`, `RENAME TABLE` or SQL Server `sp_rename` run in a query tab, or by an MCP client, counts as a sidebar drop or rename once it commits. Tabs on the table close or take the new name, and its saved filters, column layout, highlight rules, value formats, favorite and Recent entry go with it. A drop that is rolled back, or is still inside an open transaction when the run ends, leaves all of them in place. +A `DROP TABLE`, `DROP VIEW`, `ALTER TABLE ... RENAME TO`, `RENAME TABLE` or SQL Server `sp_rename` run in a query tab, or by an MCP client, counts as a sidebar drop or rename once it commits. Tabs on the table close or take the new name, and its saved filters, column layout, highlight rules, value formats, favorite and Recent entry go with it. A drop that is rolled back leaves all of them in place. So does every drop in a run that ends inside a transaction, and every drop on a SQL Server session with `IMPLICIT_TRANSACTIONS` on. A SQL file run through **Import Data**, or a restored backup, refreshes the sidebar and keeps every table's settings. This works on MySQL, MariaDB, PostgreSQL, Redshift, SQLite, SQL Server, Oracle, ClickHouse and DuckDB. On PostgreSQL, Redshift and SQL Server, name the schema, as in `public.orders`, and on DuckDB the database and the schema, because a bare name there resolves through session state such as the search path or the login's default schema. On the other engines a name stops being followed once something on the connection could have redirected it: a temporary table of the same name, a procedure call or anonymous block, or a statement that changes the current schema. A rename written with `IF EXISTS` is never followed, since it succeeds when there was nothing to rename. From fea8eb002aff42dac1bfa57d222545eb3d195a37 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Tue, 29 Sep 2026 23:03:15 +0700 Subject: [PATCH 3/4] fix(editor): skip SQL table edits hidden by T-SQL control flow, GO repeats, startup commands or imports --- .../QueryExecutionCoordinator+Batches.swift | 2 +- .../Access/DatabaseAccessBridge+Scripts.swift | 2 +- .../Database/DatabaseManager+Startup.swift | 19 +++--- .../Core/Plugins/ImportDataSinkAdapter.swift | 24 ++++++++ .../Services/Execution/ExecutableBatch.swift | 7 +++ .../Core/Services/Export/ImportService.swift | 10 ++++ .../Services/Query/CatalogChangeService.swift | 15 ++++- .../Services/Query/CommittedTableEdits.swift | 9 ++- .../Core/Utilities/SQL/SQLTokenCursor.swift | 2 +- .../Core/Utilities/SQL/TableEditDialect.swift | 17 +++--- .../SQL/TableEditStatementParser.swift | 24 +++++++- .../Query/CommittedTableEditsTests.swift | 59 +++++++++++++++++++ .../Query/SQLTableEditAdoptionTests.swift | 27 ++++++++- .../SQL/TableEditStatementParserTests.swift | 25 +++++++- docs/features/table-operations.mdx | 4 +- 15 files changed, 218 insertions(+), 28 deletions(-) diff --git a/TablePro/Core/Coordinators/QueryExecutionCoordinator+Batches.swift b/TablePro/Core/Coordinators/QueryExecutionCoordinator+Batches.swift index bf8c446db4..d29e638be4 100644 --- a/TablePro/Core/Coordinators/QueryExecutionCoordinator+Batches.swift +++ b/TablePro/Core/Coordinators/QueryExecutionCoordinator+Batches.swift @@ -339,7 +339,7 @@ extension QueryExecutionCoordinator { CatalogChangeService.post(.statementsSucceeded(SucceededStatements( scope: scope, databaseType: connection.type, - statements: prepared.prefix(run.outcome.succeededCount).flatMap { $0.batch.statements.map(\.sql) }, + statements: prepared.prefix(run.outcome.succeededCount).flatMap(\.batch.executedStatementTexts), commit: .run( startedIn: run.startState, endedIn: run.sessionState, plan: run.plan, completed: run.outcome.isCompleted ) diff --git a/TablePro/Core/Database/Access/DatabaseAccessBridge+Scripts.swift b/TablePro/Core/Database/Access/DatabaseAccessBridge+Scripts.swift index 121a607348..7ac0704868 100644 --- a/TablePro/Core/Database/Access/DatabaseAccessBridge+Scripts.swift +++ b/TablePro/Core/Database/Access/DatabaseAccessBridge+Scripts.swift @@ -204,7 +204,7 @@ final class ScriptBatchProgress: Sendable { return SucceededStatements( scope: scope, databaseType: databaseType, - statements: batches.prefix(current.completedBatchCount).flatMap { $0.statements.map(\.sql) }, + statements: batches.prefix(current.completedBatchCount).flatMap(\.executedStatementTexts), commit: .runStartedIn(current.startState, endedIn: current.endState, appTransaction: .none) ) } diff --git a/TablePro/Core/Database/DatabaseManager+Startup.swift b/TablePro/Core/Database/DatabaseManager+Startup.swift index 1749f3099b..c2c71ef82e 100644 --- a/TablePro/Core/Database/DatabaseManager+Startup.swift +++ b/TablePro/Core/Database/DatabaseManager+Startup.swift @@ -22,19 +22,22 @@ extension DatabaseManager { return !commands.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty } - @discardableResult - nonisolated internal func executeStartupCommands( - _ commands: String?, on driver: DatabaseDriver, connectionName: String - ) async -> [(statement: String, error: String)] { - guard Self.hasStartupCommands(commands), let commands else { return [] } - - let statements = commands + /// The statements `executeStartupCommands` sends, one per line or `;`. Also read by the catalog, + /// because a temporary table a startup command creates shadows a real one for the whole session. + nonisolated internal static func startupStatements(from commands: String?) -> [String] { + guard hasStartupCommands(commands), let commands else { return [] } + return commands .components(separatedBy: CharacterSet(charactersIn: ";\n")) .map { $0.trimmingCharacters(in: .whitespacesAndNewlines) } .filter { !$0.isEmpty } + } + @discardableResult + nonisolated internal func executeStartupCommands( + _ commands: String?, on driver: DatabaseDriver, connectionName: String + ) async -> [(statement: String, error: String)] { var failures: [(statement: String, error: String)] = [] - for statement in statements { + for statement in Self.startupStatements(from: commands) { do { _ = try await driver.execute(query: statement) Self.startupLogger.info( diff --git a/TablePro/Core/Plugins/ImportDataSinkAdapter.swift b/TablePro/Core/Plugins/ImportDataSinkAdapter.swift index 1626b7b891..9123a5fec0 100644 --- a/TablePro/Core/Plugins/ImportDataSinkAdapter.swift +++ b/TablePro/Core/Plugins/ImportDataSinkAdapter.swift @@ -24,6 +24,9 @@ final class ImportDataSinkAdapter: PluginImportDataSink, @unchecked Sendable { /// the sink, so without this a Stop was followed by every remaining INSERT of the batch. private let isCancelled: @Sendable () -> Bool + private let tableEditDialect: TableEditDialect? + private let nameHazards = OSAllocatedUnfairLock<[String]>(initialState: []) + private static let logger = Logger(subsystem: "com.TablePro", category: "ImportDataSinkAdapter") /// Rows go in as SQL `INSERT`s, so an engine with no SQL dialect can take none of them. @@ -44,6 +47,7 @@ final class ImportDataSinkAdapter: PluginImportDataSink, @unchecked Sendable { self.driver = driver self.databaseType = databaseType self.grammar = databaseType.lexicalGrammar + self.tableEditDialect = TableEditDialect.of(databaseType) self.databaseTypeId = databaseType.rawValue self.targetTable = targetTable self.columnMapping = Dictionary( @@ -66,11 +70,31 @@ final class ImportDataSinkAdapter: PluginImportDataSink, @unchecked Sendable { try await execute(statement: statement, line: 1) } + /// What the file ran that changes where a name resolves for the rest of the session, such as a temporary table, so + /// the catalog can stop following a later drop of that name onto the real table. + var nameHazardStatements: [String] { + nameHazards.withLock { $0 } + } + + /// Read before the statement runs, because one that fails part way can already have made a temporary table. + private func noteNameHazards(in text: String) { + guard let tableEditDialect else { return } + let statements = grammar.contains(.batchSeparatorLines) + ? SQLStatementScanner.executableStatements(in: text, grammar: grammar).map(\.sql) + : [text] + let hazards = statements.filter { + TableEditStatementParser.parse($0, dialect: tableEditDialect, grammar: grammar).changesNameHazards + } + guard !hazards.isEmpty else { return } + nameHazards.withLock { $0 += hazards } + } + /// A SQL Server file arrives a batch at a time, as sqlcmd reads it, or a statement at a time when it holds no `GO` /// line, and each goes to the server whole: T-SQL scopes a variable, a table variable and a `TRY...CATCH` to one /// batch, and a routine's body runs to the end of its batch. A driver that cannot send a batch whole runs its /// statements one by one, as the editor does. func execute(statement: String, line: Int) async throws { + noteNameHazards(in: statement) guard grammar.contains(.batchSeparatorLines) else { _ = try await driver.execute(query: statement) return diff --git a/TablePro/Core/Services/Execution/ExecutableBatch.swift b/TablePro/Core/Services/Execution/ExecutableBatch.swift index e0379ebc8b..1d81673d2a 100644 --- a/TablePro/Core/Services/Execution/ExecutableBatch.swift +++ b/TablePro/Core/Services/Execution/ExecutableBatch.swift @@ -29,6 +29,13 @@ struct ExecutableBatch: Sendable { return NSRange(location: first.range.location, length: last.range.upperBound - first.range.location) } + /// Every statement the server ran for the batch, in the order it ran them: `GO 3` runs all of them three + /// times, and a rename chain run twice leaves each table where it started. + var executedStatementTexts: [String] { + let once = statements.map(\.sql) + return (0.. Bool + private let startupCommands: @MainActor (UUID) -> String? private var pendingChanges: [UUID: CatalogChange] = [:] private var nameHazards: [UUID: TableNameHazards] = [:] @@ -45,7 +46,8 @@ final class CatalogChangeService { init( targets: [any CatalogChangeTarget]? = nil, adoption: CatalogEditAdoption? = nil, - isSessionLive: (@MainActor (UUID) -> Bool)? = nil + isSessionLive: (@MainActor (UUID) -> Bool)? = nil, + startupCommands: (@MainActor (UUID) -> String?)? = nil ) { self.targets = targets ?? [ SchemaRefreshService.shared, @@ -58,6 +60,9 @@ final class CatalogChangeService { guard let session = DatabaseManager.shared.session(for: connectionId) else { return false } return Self.acceptsChanges(from: session) } + self.startupCommands = startupCommands ?? { connectionId in + DatabaseManager.shared.session(for: connectionId)?.connection.startupCommands + } } /// A session mid-way through a database switch reports `.connecting` while its driver still works, @@ -133,6 +138,11 @@ final class CatalogChangeService { /// chain and a drop of a name another statement just freed both land on the right table. private func recordCommittedTableEdits(of succeeded: SucceededStatements) { let connectionId = succeeded.scope.connectionId + recordNameHazards( + of: DatabaseManager.startupStatements(from: startupCommands(connectionId)), + databaseType: succeeded.databaseType, + connectionId: connectionId + ) let grammar = SQLLexicalResolver.executionGrammar(for: succeeded.databaseType, connectionId: connectionId) var hazards = nameHazards[connectionId] ?? TableNameHazards() let edits = CommittedTableEdits.edits(in: succeeded, grammar: grammar, hazards: &hazards) @@ -159,7 +169,8 @@ final class CatalogChangeService { } /// Read from every statement that may have run, failed ones included, because a procedure that - /// failed part way can have created a temporary table first. + /// failed part way can have created a temporary table first. That includes the connection's + /// startup commands, which run on every connect before anything here hears of the session. private func recordNameHazards(of statements: [String], databaseType: DatabaseType, connectionId: UUID) { guard let dialect = TableEditDialect.of(databaseType) else { return } let grammar = SQLLexicalResolver.executionGrammar(for: databaseType, connectionId: connectionId) diff --git a/TablePro/Core/Services/Query/CommittedTableEdits.swift b/TablePro/Core/Services/Query/CommittedTableEdits.swift index ca5be4afbe..24d4c92751 100644 --- a/TablePro/Core/Services/Query/CommittedTableEdits.swift +++ b/TablePro/Core/Services/Query/CommittedTableEdits.swift @@ -37,7 +37,7 @@ struct TableNameHazards: Sendable, Equatable { case .losesSchemaContext, .runsUnseenCode: namesMayBeShadowed = true case .drop, .rename, .beginsTransaction, .commits, .rollsBack, .rollsBackToSavepoint, - .losesTransactionTracking, .selectsDatabase, .other: + .losesTransactionTracking, .selectsDatabase, .controlsFlow, .other: break } } @@ -163,6 +163,8 @@ private struct TableEditWalk { context = dialect.context(afterUsing: name) case .losesSchemaContext: context.schema = nil + case .controlsFlow: + stopAdopting() case .createsTemporaryTable, .runsUnseenCode, .other: break } @@ -180,6 +182,11 @@ private struct TableEditWalk { /// warning, and this holds back a drop that did commit, which keeps its settings in place. private mutating func disown() { guard !dialect.commitsDDLImplicitly else { return } + stopAdopting() + } + + /// Nothing the run did is known to have happened, the edits before this point included. + private mutating func stopAdopting() { adopts = false committed.removeAll() pending.removeAll() diff --git a/TablePro/Core/Utilities/SQL/SQLTokenCursor.swift b/TablePro/Core/Utilities/SQL/SQLTokenCursor.swift index e0fd269825..4680127e11 100644 --- a/TablePro/Core/Utilities/SQL/SQLTokenCursor.swift +++ b/TablePro/Core/Utilities/SQL/SQLTokenCursor.swift @@ -31,11 +31,11 @@ internal struct SQLTokenCursor { internal static let equals = UInt16(UnicodeScalar("=").value) internal static let comma = UInt16(UnicodeScalar(",").value) internal static let period = UInt16(UnicodeScalar(".").value) + internal static let colon = UInt16(UnicodeScalar(":").value) internal static let openParen = SqlLexer.openParen internal static let closeParen = SqlLexer.closeParen private static let at = UInt16(UnicodeScalar("@").value) - private static let colon = UInt16(UnicodeScalar(":").value) private static let openBracket = UInt16(UnicodeScalar("[").value) private let text: NSString diff --git a/TablePro/Core/Utilities/SQL/TableEditDialect.swift b/TablePro/Core/Utilities/SQL/TableEditDialect.swift index 290880b7b4..214686aad0 100644 --- a/TablePro/Core/Utilities/SQL/TableEditDialect.swift +++ b/TablePro/Core/Utilities/SQL/TableEditDialect.swift @@ -59,6 +59,9 @@ struct TableEditDialect: Sendable, Equatable { /// The containers that hold a session's own temporary objects, `pg_temp` and `pg_temp_3` or /// `temp`. A name inside one is never a table the app keeps settings for. let temporaryContainers: Set + /// SQL Server runs a batch whole, and T-SQL's `IF`, `GOTO` and `TRY...CATCH` can skip a statement + /// inside it or swallow its error, so a batch that succeeded is not a list of statements that did. + let branchesInsideBatches: Bool static func of(_ type: DatabaseType) -> TableEditDialect? { if type == .clickhouse { return .clickHouse } @@ -146,7 +149,7 @@ struct TableEditDialect: Sendable, Equatable { acceptsDatabaseSchemaTable: true, commitsDDLImplicitly: false, endCommits: true, useSelectsDatabase: false, renameKeepsContainer: true, renamesWithoutTo: false, temporaryTablesShadowQualifiedNames: false, temporaryTablesShadowRealOnes: true, - temporaryContainers: ["pg_temp"] + temporaryContainers: ["pg_temp"], branchesInsideBatches: false ) static let mySQL = TableEditDialect( @@ -154,7 +157,7 @@ struct TableEditDialect: Sendable, Equatable { acceptsDatabaseSchemaTable: false, commitsDDLImplicitly: true, endCommits: false, useSelectsDatabase: true, renameKeepsContainer: false, renamesWithoutTo: true, temporaryTablesShadowQualifiedNames: true, temporaryTablesShadowRealOnes: true, - temporaryContainers: [] + temporaryContainers: [], branchesInsideBatches: false ) static let clickHouse = TableEditDialect( @@ -162,7 +165,7 @@ struct TableEditDialect: Sendable, Equatable { acceptsDatabaseSchemaTable: false, commitsDDLImplicitly: true, endCommits: false, useSelectsDatabase: true, renameKeepsContainer: false, renamesWithoutTo: false, temporaryTablesShadowQualifiedNames: false, temporaryTablesShadowRealOnes: true, - temporaryContainers: [] + temporaryContainers: [], branchesInsideBatches: false ) /// An attached database is a schema to SQLite, and a bare name can resolve into one, so only a @@ -172,7 +175,7 @@ struct TableEditDialect: Sendable, Equatable { acceptsDatabaseSchemaTable: false, commitsDDLImplicitly: false, endCommits: true, useSelectsDatabase: false, renameKeepsContainer: true, renamesWithoutTo: false, temporaryTablesShadowQualifiedNames: false, temporaryTablesShadowRealOnes: true, - temporaryContainers: ["temp"] + temporaryContainers: ["temp"], branchesInsideBatches: false ) /// `a.b` is a schema in the current catalog or the default schema of catalog `a`, whichever @@ -182,7 +185,7 @@ struct TableEditDialect: Sendable, Equatable { acceptsDatabaseSchemaTable: true, commitsDDLImplicitly: false, endCommits: true, useSelectsDatabase: false, renameKeepsContainer: true, renamesWithoutTo: false, temporaryTablesShadowQualifiedNames: false, temporaryTablesShadowRealOnes: true, - temporaryContainers: ["temp"] + temporaryContainers: ["temp"], branchesInsideBatches: false ) static let sqlServer = TableEditDialect( @@ -190,7 +193,7 @@ struct TableEditDialect: Sendable, Equatable { acceptsDatabaseSchemaTable: true, commitsDDLImplicitly: false, endCommits: false, useSelectsDatabase: true, renameKeepsContainer: true, renamesWithoutTo: false, temporaryTablesShadowQualifiedNames: false, temporaryTablesShadowRealOnes: true, - temporaryContainers: ["tempdb"] + temporaryContainers: ["tempdb"], branchesInsideBatches: true ) static let oracle = TableEditDialect( @@ -198,7 +201,7 @@ struct TableEditDialect: Sendable, Equatable { acceptsDatabaseSchemaTable: false, commitsDDLImplicitly: true, endCommits: false, useSelectsDatabase: false, renameKeepsContainer: true, renamesWithoutTo: false, temporaryTablesShadowQualifiedNames: false, temporaryTablesShadowRealOnes: false, - temporaryContainers: [] + temporaryContainers: [], branchesInsideBatches: false ) } diff --git a/TablePro/Core/Utilities/SQL/TableEditStatementParser.swift b/TablePro/Core/Utilities/SQL/TableEditStatementParser.swift index 5a17b07acb..161b7e0aa2 100644 --- a/TablePro/Core/Utilities/SQL/TableEditStatementParser.swift +++ b/TablePro/Core/Utilities/SQL/TableEditStatementParser.swift @@ -40,6 +40,10 @@ enum TableEditStatement: Equatable, Sendable { /// A procedure, a prepared statement or an anonymous block: code the server runs that the text /// does not show, and that can create a temporary table or move the schema. case runsUnseenCode + /// T-SQL's `IF`, `ELSE`, `WHILE`, `GOTO`, `RETURN`, a label, or a `TRY...CATCH` block. SQL Server + /// runs a batch whole, so the statements after one of these may have been skipped, or may have + /// failed into a `CATCH` without the batch failing. + case controlsFlow case other var editsTable: Bool { @@ -47,7 +51,7 @@ enum TableEditStatement: Equatable, Sendable { case .drop, .rename: return true case .beginsTransaction, .commits, .rollsBack, .rollsBackToSavepoint, .losesTransactionTracking, - .selectsDatabase, .losesSchemaContext, .createsTemporaryTable, .runsUnseenCode, .other: + .selectsDatabase, .losesSchemaContext, .createsTemporaryTable, .runsUnseenCode, .controlsFlow, .other: return false } } @@ -59,7 +63,7 @@ enum TableEditStatement: Equatable, Sendable { case .createsTemporaryTable, .losesSchemaContext, .runsUnseenCode: return true case .drop, .rename, .beginsTransaction, .commits, .rollsBack, .rollsBackToSavepoint, - .losesTransactionTracking, .selectsDatabase, .other: + .losesTransactionTracking, .selectsDatabase, .controlsFlow, .other: return false } } @@ -82,9 +86,14 @@ enum TableEditStatementParser { return .other } + private static let controlFlowKeywords: Set = ["IF", "ELSE", "WHILE", "BREAK", "CONTINUE", "GOTO", "RETURN"] + private static func read(_ sql: String, dialect: TableEditDialect, grammar: SQLLexicalGrammar) -> TableEditStatement { var reader = Reader(SQLTokenCursor(sql, grammar: grammar)) guard let keyword = reader.nextWord() else { return .other } + if dialect.branchesInsideBatches, controlFlowKeywords.contains(keyword) || reader.startsLabel() { + return .controlsFlow + } switch keyword { case "DROP": return reader.drop() @@ -94,6 +103,7 @@ enum TableEditStatementParser { return reader.renameTables() case "BEGIN": if grammar.contains(.plsqlBlocks) || reader.opensCompoundStatement() { return .runsUnseenCode } + if dialect.branchesInsideBatches, reader.namesTryOrCatchBlock() { return .controlsFlow } return SqlBlockStructure.beginStartsTransaction(followedBy: reader.peekWord()) ? .beginsTransaction : .other case "DECLARE": return grammar.contains(.plsqlBlocks) ? .runsUnseenCode : .other @@ -110,6 +120,7 @@ enum TableEditStatementParser { case "RELEASE": return .commits case "END": + if dialect.branchesInsideBatches, reader.namesTryOrCatchBlock() { return .controlsFlow } return dialect.endCommits ? reader.commit() : .other case "ROLLBACK", "ABORT": return reader.rollback() @@ -280,6 +291,15 @@ private struct Reader { return saysTemporary || inTemporaryContainer ? .createsTemporaryTable(name) : .other } + /// `done:` before a statement, which a T-SQL `GOTO` can jump to or over. + func startsLabel() -> Bool { + cursor.peek()?.isSymbol(SQLTokenCursor.colon) == true + } + + func namesTryOrCatchBlock() -> Bool { + peekWord() == "TRY" || peekWord() == "CATCH" + } + /// MariaDB's `BEGIN NOT ATOMIC ... END` runs a compound statement, not a transaction. mutating func opensCompoundStatement() -> Bool { var lookahead = self diff --git a/TableProTests/Core/Services/Query/CommittedTableEditsTests.swift b/TableProTests/Core/Services/Query/CommittedTableEditsTests.swift index a02d7c969f..504ae4cf1d 100644 --- a/TableProTests/Core/Services/Query/CommittedTableEditsTests.swift +++ b/TableProTests/Core/Services/Query/CommittedTableEditsTests.swift @@ -281,6 +281,13 @@ struct CommittedTableEditsTests { ) } + @Test("A SQL Server run holding control flow adopts nothing, since a statement in its batch may have been skipped") + func controlFlowInsideABatch() { + #expect(Self.sqlServerEdits(["GOTO done", "DROP TABLE dbo.people", "done: SELECT 1"]).isEmpty) + #expect(Self.sqlServerEdits(["DROP TABLE dbo.a", "IF 1 = 0 BEGIN", "DROP TABLE dbo.b", "END"]).isEmpty) + #expect(Self.sqlServerEdits(["BEGIN TRY", "DROP TABLE dbo.people", "END TRY BEGIN CATCH", "END CATCH"]).isEmpty) + } + // MARK: - Name context and hazards @Test("USE moves a MySQL bare name onto the database it selected") @@ -484,6 +491,58 @@ struct SucceededStatementsProbeTests { } } +struct ExecutedStatementTextsTests { + /// `GO 2` runs a rename chain twice, which leaves every table where it started, so the chain has + /// to be replayed twice too. + @Test("A batch run more than once with GO n reports its statements once per run") + func repeatedBatch() { + let grammar = DatabaseType.mssql.lexicalGrammar + let batches = QueryBatchPlanner.batches( + in: "EXEC sp_rename 'dbo.a', 'tmp'; EXEC sp_rename 'dbo.b', 'a'; EXEC sp_rename 'dbo.tmp', 'b'\nGO 2", + model: QueryStatementModel.forDatabaseType(.mssql), + grammar: grammar + ) + #expect(batches.count == 1) + let statements = batches.flatMap(\.executedStatementTexts) + #expect(statements.count == 6) + var hazards = TableNameHazards() + let edits = CommittedTableEdits.edits( + in: SucceededStatements( + scope: DatabaseScope(connectionId: UUID(), database: "sales", schema: "dbo"), + databaseType: .mssql, + statements: statements, + commit: .runStartedIn(.idle, endedIn: .idle, appTransaction: .none) + ), + grammar: grammar, + hazards: &hazards + ) + let placed = { (name: String) in TablePlacement(database: "sales", schema: "dbo", name: name) } + let swap: [TableCatalogEdit] = [ + .renamed(placed("a"), to: "tmp", kind: .table), + .renamed(placed("b"), to: "a", kind: .table), + .renamed(placed("tmp"), to: "b", kind: .table) + ] + #expect(edits == swap + swap) + } +} + +struct ImportNameHazardTests { + /// The import runs on the editor's own session, so a temporary table it makes shadows the real + /// one there for every later run. + @Test("An imported file's temporary tables and procedure calls are kept for the catalog, and nothing else") + func importRecordsHazards() async throws { + let connection = TestFixtures.makeConnection(type: .mysql) + let sink = ImportDataSinkAdapter(driver: ScriptAnsweringDriver(connection: connection), databaseType: .mysql) + + try await sink.execute(statement: "CREATE TEMPORARY TABLE people (id int)") + try await sink.execute(statement: "INSERT INTO people VALUES (1)") + try await sink.execute(statement: "DROP TABLE IF EXISTS orders") + try await sink.execute(statement: "CALL rebuild()") + + #expect(sink.nameHazardStatements == ["CREATE TEMPORARY TABLE people (id int)", "CALL rebuild()"]) + } +} + @MainActor struct ScriptBatchProgressTests { /// SQL Server commits each batch of a script as it runs, so the batches before a failing one diff --git a/TableProTests/Core/Services/Query/SQLTableEditAdoptionTests.swift b/TableProTests/Core/Services/Query/SQLTableEditAdoptionTests.swift index 21a36e7f5d..5e074df83a 100644 --- a/TableProTests/Core/Services/Query/SQLTableEditAdoptionTests.swift +++ b/TableProTests/Core/Services/Query/SQLTableEditAdoptionTests.swift @@ -78,7 +78,11 @@ struct SQLTableEditAdoptionTests { } } - private func makeHarness(type: DatabaseType = .postgresql, browseSchema: String? = "public") throws -> Harness { + private func makeHarness( + type: DatabaseType = .postgresql, + browseSchema: String? = "public", + startupCommands: String? = nil + ) throws -> Harness { let connection = TestFixtures.makeConnection(database: "shop", type: type) var session = ConnectionSession(connection: connection, driver: MockDatabaseDriver(connection: connection)) session.status = .connected @@ -95,7 +99,12 @@ struct SQLTableEditAdoptionTests { ) let store = RecordingStore() let adoption = CatalogEditAdoption(settingsStores: [store], favoriteTables: favorites) - let service = CatalogChangeService(targets: [IgnoringTarget()], adoption: adoption, isSessionLive: { _ in true }) + let service = CatalogChangeService( + targets: [IgnoringTarget()], + adoption: adoption, + isSessionLive: { _ in true }, + startupCommands: { _ in startupCommands } + ) return Harness(connection: connection, store: store, favorites: favorites, service: service) } @@ -182,6 +191,20 @@ struct SQLTableEditAdoptionTests { #expect(harness.store.droppedTables == [harness.scope("orders", schema: nil)]) } + /// Startup commands run on every connect, before the session reports anything, and a MySQL + /// temporary table hides the real one under its qualified name as well. + @Test("A temporary table the startup commands create keeps drops of that name off the real table's settings") + func startupCommandTemporaryTable() throws { + let harness = try makeHarness( + type: .mysql, browseSchema: nil, startupCommands: "SET NAMES utf8mb4;\nCREATE TEMPORARY TABLE people (id int)" + ) + defer { tearDown(harness) } + + harness.run(["DROP TABLE shop.people", "DROP TABLE shop.orders"], schema: nil) + + #expect(harness.store.droppedTables == [harness.scope("orders", schema: nil)]) + } + /// A procedure that failed after creating a temporary table reports only that it ran. @Test("A procedure call that failed still keeps later bare drops off the real table's settings") func failedProcedureCallIsAHazard() throws { diff --git a/TableProTests/Core/Utilities/SQL/TableEditStatementParserTests.swift b/TableProTests/Core/Utilities/SQL/TableEditStatementParserTests.swift index b0b04045dd..6c238681ca 100644 --- a/TableProTests/Core/Utilities/SQL/TableEditStatementParserTests.swift +++ b/TableProTests/Core/Utilities/SQL/TableEditStatementParserTests.swift @@ -214,7 +214,7 @@ struct TableEditStatementParserTests { #expect(try Self.parse("BEGIN", .postgresql) == .beginsTransaction) #expect(try Self.parse("START TRANSACTION", .mysql) == .beginsTransaction) #expect(try Self.parse("BEGIN TRAN", .mssql) == .beginsTransaction) - #expect(try Self.parse("BEGIN TRY", .mssql) == .other) + #expect(try Self.parse("BEGIN TRY", .mssql) == .controlsFlow) #expect(try Self.parse("BEGIN IMMEDIATE", .sqlite) == .beginsTransaction) #expect(try Self.parse("BEGIN NULL; END", .oracle) == .runsUnseenCode) #expect(try Self.parse("COMMIT", .postgresql) == .commits) @@ -234,6 +234,29 @@ struct TableEditStatementParserTests { #expect(try Self.parse("SET ANSI_NULLS, IMPLICIT_TRANSACTIONS ON", .mssql) == .losesTransactionTracking) } + /// SQL Server runs a batch whole, so a statement after one of these may never have run. + @Test("T-SQL control flow is read as such, and only on SQL Server") + func sqlServerControlFlow() throws { + for sql in [ + "IF OBJECT_ID('dbo.people') IS NOT NULL DROP TABLE dbo.people", + "ELSE SELECT 1", + "WHILE @i < 3 SET @i += 1", + "GOTO done", + "RETURN", + "BREAK", + "done: SELECT 1", + "BEGIN TRY DROP TABLE dbo.people", + "END TRY BEGIN CATCH SELECT 1", + "END CATCH" + ] { + #expect(try Self.parse(sql, .mssql) == .controlsFlow, "\(sql)") + } + #expect(try Self.parse("BEGIN TRANSACTION", .mssql) == .beginsTransaction) + #expect(try Self.parse("SELECT :id FROM dual", .oracle) == .other) + #expect(try Self.parse("IF 1 = 1 THEN SELECT 1", .mysql) == .other) + #expect(try Self.parse("END TRY", .postgresql) == .commits) + } + @Test("A statement that moves where a bare name points is read as doing so") func nameContext() throws { #expect(try Self.parse("USE `archive`", .mysql) == .selectsDatabase(Self.name(Self.quoted("archive")))) diff --git a/docs/features/table-operations.mdx b/docs/features/table-operations.mdx index 4998628756..aed79933f6 100644 --- a/docs/features/table-operations.mdx +++ b/docs/features/table-operations.mdx @@ -69,9 +69,9 @@ No other engine has a rename, so the item never appears on Cassandra, DynamoDB, ## Drop or rename with SQL -A `DROP TABLE`, `DROP VIEW`, `ALTER TABLE ... RENAME TO`, `RENAME TABLE` or SQL Server `sp_rename` run in a query tab, or by an MCP client, counts as a sidebar drop or rename once it commits. Tabs on the table close or take the new name, and its saved filters, column layout, highlight rules, value formats, favorite and Recent entry go with it. A drop that is rolled back leaves all of them in place. So does every drop in a run that ends inside a transaction, and every drop on a SQL Server session with `IMPLICIT_TRANSACTIONS` on. A SQL file run through **Import Data**, or a restored backup, refreshes the sidebar and keeps every table's settings. +A `DROP TABLE`, `DROP VIEW`, `ALTER TABLE ... RENAME TO`, `RENAME TABLE` or SQL Server `sp_rename` run in a query tab, or by an MCP client, counts as a sidebar drop or rename once it commits. Tabs on the table close or take the new name, and its saved filters, column layout, highlight rules, value formats, favorite and Recent entry go with it. A drop that is rolled back leaves all of them in place. So does every drop in a run that ends inside a transaction, every drop on a SQL Server session with `IMPLICIT_TRANSACTIONS` on, and every drop in a SQL Server script that uses `IF`, `WHILE`, `GOTO`, `RETURN` or `TRY...CATCH`. A SQL file run through **Import Data**, or a restored backup, refreshes the sidebar and keeps every table's settings. -This works on MySQL, MariaDB, PostgreSQL, Redshift, SQLite, SQL Server, Oracle, ClickHouse and DuckDB. On PostgreSQL, Redshift and SQL Server, name the schema, as in `public.orders`, and on DuckDB the database and the schema, because a bare name there resolves through session state such as the search path or the login's default schema. On the other engines a name stops being followed once something on the connection could have redirected it: a temporary table of the same name, a procedure call or anonymous block, or a statement that changes the current schema. A rename written with `IF EXISTS` is never followed, since it succeeds when there was nothing to rename. +This works on MySQL, MariaDB, PostgreSQL, Redshift, SQLite, SQL Server, Oracle, ClickHouse and DuckDB. On PostgreSQL, Redshift and SQL Server, name the schema, as in `public.orders`, and on DuckDB the database and the schema, because a bare name there resolves through session state such as the search path or the login's default schema. On the other engines a name stops being followed once something on the connection could have redirected it: a temporary table of the same name, a procedure call or anonymous block, or a statement that changes the current schema, whether it ran in a query tab, in the connection's **Startup Commands** or in an imported SQL file. A rename written with `IF EXISTS` is never followed, since it succeeds when there was nothing to rename. ## Maintenance From a20d619a16201e613dbbb1930dbcf041b6cac464 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Tue, 29 Sep 2026 23:27:14 +0700 Subject: [PATCH 4/4] fix(editor): keep a renamed or virtual temporary table shadowing the real table's name --- .../Services/Query/CommittedTableEdits.swift | 16 +++++++++++++++- .../SQL/TableEditStatementParser.swift | 7 ++++--- .../Query/CommittedTableEditsTests.swift | 18 ++++++++++++++++++ .../SQL/TableEditStatementParserTests.swift | 5 +++++ 4 files changed, 42 insertions(+), 4 deletions(-) diff --git a/TablePro/Core/Services/Query/CommittedTableEdits.swift b/TablePro/Core/Services/Query/CommittedTableEdits.swift index 24d4c92751..560fd3a932 100644 --- a/TablePro/Core/Services/Query/CommittedTableEdits.swift +++ b/TablePro/Core/Services/Query/CommittedTableEdits.swift @@ -34,14 +34,28 @@ struct TableNameHazards: Sendable, Equatable { guard dialect.temporaryTablesShadowRealOnes, let table = name.parts.last.flatMap(dialect.folded) else { return } temporaryNames.insert(table.lowercased()) + case .rename(let pairs, _): + guard dialect.temporaryTablesShadowRealOnes else { return } + for pair in pairs where isTemporary(pair.from, dialect: dialect) { + guard let target = pair.to.parts.last.flatMap(dialect.folded) else { continue } + temporaryNames.insert(target.lowercased()) + } case .losesSchemaContext, .runsUnseenCode: namesMayBeShadowed = true - case .drop, .rename, .beginsTransaction, .commits, .rollsBack, .rollsBackToSavepoint, + case .drop, .beginsTransaction, .commits, .rollsBack, .rollsBackToSavepoint, .losesTransactionTracking, .selectsDatabase, .controlsFlow, .other: break } } + /// A temporary table keeps shadowing under the name it is renamed to, as SQLite's + /// `ALTER TABLE scratch RENAME TO people` does. + private func isTemporary(_ name: SQLObjectName, dialect: TableEditDialect) -> Bool { + if name.parts.dropLast().contains(where: { dialect.namesTemporaryContainer($0.text) }) { return true } + guard let table = name.parts.last.flatMap(dialect.folded) else { return false } + return temporaryNames.contains(table.lowercased()) + } + /// Whether `name`, placed as `table`, may be something other than the table the app keeps /// settings for. func mayShadow(_ name: SQLObjectName, placedAs table: TablePlacement, dialect: TableEditDialect) -> Bool { diff --git a/TablePro/Core/Utilities/SQL/TableEditStatementParser.swift b/TablePro/Core/Utilities/SQL/TableEditStatementParser.swift index 161b7e0aa2..afb6d092f8 100644 --- a/TablePro/Core/Utilities/SQL/TableEditStatementParser.swift +++ b/TablePro/Core/Utilities/SQL/TableEditStatementParser.swift @@ -274,13 +274,14 @@ private struct Reader { return isAtEnd ? .rename(pairs, kind: .table) : .other } - /// `CREATE TEMPORARY TABLE people`, or SQLite's `CREATE TABLE temp.people`, shadows the real - /// `people` for the rest of the session, so a bare `DROP TABLE people` after it drops the - /// temporary one. + /// `CREATE TEMPORARY TABLE people`, or SQLite's `CREATE TABLE temp.people` and + /// `CREATE VIRTUAL TABLE temp.people USING fts5(...)`, shadows the real `people` for the rest of + /// the session, so a bare `DROP TABLE people` after it drops the temporary one. mutating func createTemporaryTable(dialect: TableEditDialect) -> TableEditStatement { if accept("OR"), !accept("REPLACE") { return .other } _ = accept("GLOBAL") || accept("LOCAL") || accept("PRIVATE") let saysTemporary = accept("TEMPORARY") || accept("TEMP") + _ = accept("VIRTUAL") guard accept("TABLE") || accept("VIEW") else { return .other } var lookahead = self if lookahead.accept("IF"), lookahead.accept("NOT"), lookahead.accept("EXISTS") { diff --git a/TableProTests/Core/Services/Query/CommittedTableEditsTests.swift b/TableProTests/Core/Services/Query/CommittedTableEditsTests.swift index 504ae4cf1d..a8e9e0f8a5 100644 --- a/TableProTests/Core/Services/Query/CommittedTableEditsTests.swift +++ b/TableProTests/Core/Services/Query/CommittedTableEditsTests.swift @@ -340,6 +340,24 @@ struct CommittedTableEditsTests { #expect(sqliteQualified.temporaryNames == ["people"]) } + @Test("A temporary table renamed onto a real table's name keeps shadowing it") + func renamedTemporaryTable() { + var hazards = TableNameHazards() + #expect( + Self.edits( + ["CREATE TEMP TABLE scratch (id int)", "ALTER TABLE scratch RENAME TO people", "DROP TABLE people"], + on: .sqlite, database: "/tmp/app.db", schema: nil, hazards: &hazards + ).isEmpty + ) + #expect(hazards.temporaryNames == ["scratch", "people"]) + var qualified = TableNameHazards() + _ = Self.edits( + ["ALTER TABLE temp.scratch RENAME TO orders"], on: .sqlite, database: "/tmp/app.db", schema: nil, + hazards: &qualified + ) + #expect(qualified.temporaryNames == ["orders"]) + } + @Test("A temporary table is remembered even when its transaction is one the text cannot judge") func temporaryTableInsideATransaction() { var hazards = TableNameHazards() diff --git a/TableProTests/Core/Utilities/SQL/TableEditStatementParserTests.swift b/TableProTests/Core/Utilities/SQL/TableEditStatementParserTests.swift index 6c238681ca..f26a30d62e 100644 --- a/TableProTests/Core/Utilities/SQL/TableEditStatementParserTests.swift +++ b/TableProTests/Core/Utilities/SQL/TableEditStatementParserTests.swift @@ -270,6 +270,11 @@ struct TableEditStatementParserTests { @Test("A temporary table's creation is read with its name") func temporaryTableCreation() throws { + #expect( + try Self.parse("CREATE VIRTUAL TABLE temp.people USING fts5(body)", .sqlite) + == .createsTemporaryTable(Self.name(Self.bare("temp"), Self.bare("people"))) + ) + #expect(try Self.parse("CREATE VIRTUAL TABLE people USING fts5(body)", .sqlite) == .other) #expect( try Self.parse("CREATE TEMPORARY TABLE IF NOT EXISTS people (id int)", .mysql) == .createsTemporaryTable(Self.name(Self.bare("people")))