diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a84de35d..444d6fefd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Last line of a helper process's output lost when it exits right after writing it. +- Structure and trigger edits committing or rolling back a transaction left open in a query tab on the same connection. - Composite, range and extension-typed PostgreSQL columns labelled `ENUM(…)` in the structure editor. - Sidebar routines, triggers and types from the previous database after switching while it was still loading. - Silent fallback order when foreign keys between the exported tables form a cycle. (#2517) diff --git a/TablePro/Core/Database/DatabaseManager+Schema.swift b/TablePro/Core/Database/DatabaseManager+Schema.swift index 4ecef9024..c614dbcec 100644 --- a/TablePro/Core/Database/DatabaseManager+Schema.swift +++ b/TablePro/Core/Database/DatabaseManager+Schema.swift @@ -13,9 +13,10 @@ import TableProPluginKit // MARK: - Schema Changes extension DatabaseManager { - /// Execute schema changes (ALTER TABLE, CREATE INDEX, etc.) in a transaction. - /// The connection, database and schema all come from the editing tab's own scope, - /// never from ambient session state that another window or tab can move. + /// Execute schema changes (ALTER TABLE, CREATE INDEX, etc.) in a transaction of their own, + /// on the schema change route rather than the session driver a query tab may have left + /// mid-transaction. The connection, database and schema all come from the editing tab's + /// own scope, never from ambient session state that another window or tab can move. /// /// Authorization sits between two scoped blocks rather than inside one: it awaits a /// confirmation sheet and Touch ID, and holding the connection's driver gate across a @@ -26,7 +27,7 @@ extension DatabaseManager { databaseType: DatabaseType, scope: DatabaseScope ) async throws { - let route = executionRoute(for: scope) + let route = schemaChangeRoute(for: scope) let statements = try await withScopedDriver( scope: scope, route: route, cancellation: .untracked diff --git a/TablePro/Core/Database/DatabaseManager+ScopedDriver.swift b/TablePro/Core/Database/DatabaseManager+ScopedDriver.swift index cc40c6f44..0dd0a748f 100644 --- a/TablePro/Core/Database/DatabaseManager+ScopedDriver.swift +++ b/TablePro/Core/Database/DatabaseManager+ScopedDriver.swift @@ -26,6 +26,16 @@ extension DatabaseManager { return canPool(session) ? .pooled : .sessionDriver } + /// A structure, trigger or enum edit is the app's own DDL with its own BEGIN and COMMIT, so + /// it must not share a connection with the user: on the session driver its BEGIN joins + /// whatever transaction a query tab left open, and its COMMIT or ROLLBACK then takes that + /// tab's uncommitted work with it. It runs on a pooled connection wherever one reaches the + /// same database, which is the metadata route, and on the session driver only where nothing + /// else can. + func schemaChangeRoute(for scope: DatabaseScope) -> ScopedDriverRoute { + metadataRoute(for: scope) + } + /// SQL the user owns stays on the session driver, which holds their transaction, /// their temp tables and the handle Stop cancels. The pool is the fallback only for /// engines that cannot change database on a live connection, where the alternative diff --git a/TablePro/Core/Database/EnumLabelEditor.swift b/TablePro/Core/Database/EnumLabelEditor.swift index 046a4bae2..599cc3333 100644 --- a/TablePro/Core/Database/EnumLabelEditor.swift +++ b/TablePro/Core/Database/EnumLabelEditor.swift @@ -83,15 +83,14 @@ struct EnumLabelEditor { throw EnumLabelEditingError.denied(decision.deniedReason ?? String(localized: "Operation not permitted")) } - /// Not the session driver: that one holds whatever transaction the user opened in a query - /// tab, and a label added inside it is unusable until the commit and gone on a rollback, - /// while the listing reloads over other connections and cannot see it at all. The - /// metadata route is a dedicated autocommit connection wherever the engine can pool one. + /// A label added inside a query tab's open transaction is unusable until the commit and + /// gone on a rollback, while the listing reloads over other connections and cannot see + /// it at all; the schema change route keeps the statement off that transaction. let startedAt = Date() let scope = scope try await DatabaseManager.shared.withScopedDriver( scope: scope, - route: DatabaseManager.shared.metadataRoute(for: scope), + route: DatabaseManager.shared.schemaChangeRoute(for: scope), cancellation: .protectedWrite ) { driver in _ = try await driver.execute(query: sql) diff --git a/TablePro/Core/Database/TriggerEditing.swift b/TablePro/Core/Database/TriggerEditing.swift index b341807cd..58454d9af 100644 --- a/TablePro/Core/Database/TriggerEditing.swift +++ b/TablePro/Core/Database/TriggerEditing.swift @@ -43,7 +43,11 @@ enum TriggerApplyStrategy: Equatable { enum TriggerEditing { nonisolated private static let logger = Logger(subsystem: "com.TablePro", category: "TriggerEditing") + /// Runs on the schema change route, never on the session driver: the trigger's own BEGIN + /// would join a transaction a query tab left open, and its COMMIT or ROLLBACK would take + /// that tab's uncommitted work with it. static func apply( + scope: DatabaseScope, connection: DatabaseConnection, tableName: String, sql: String, @@ -51,10 +55,6 @@ enum TriggerEditing { originalName: String?, originalDefinition: String? ) async throws { - guard let driver = DatabaseManager.shared.driver(for: connection.id) else { - throw TriggerEditingError.notConnected - } - let decision = await ExecutionGateProvider.shared.authorize( OperationRequest( connectionId: connection.id, @@ -72,29 +72,30 @@ enum TriggerEditing { throw TriggerEditingError.denied(decision.deniedReason ?? String(localized: "Operation not permitted")) } - let strategy = TriggerApplyStrategy.resolve( - isEdit: isEdit, - usesReplace: driver.triggerEditUsesReplace, - transactionalDDL: driver.supportsTransactionalDDL - ) - let dropSQL = originalName.flatMap { driver.generateDropTriggerSQL(name: $0, table: tableName) } - let startedAt = Date() - switch strategy { - case let .transactional(dropFirst): - try await runInTransaction(driver: driver, dropSQL: dropFirst ? dropSQL : nil, sql: sql) - case .dropThenCreate: - guard let dropSQL else { throw TriggerEditingError.dropUnavailable } - try await runDropThenCreate(driver: driver, dropSQL: dropSQL, sql: sql, rollback: originalDefinition) - case .direct: - _ = try await driver.execute(query: sql) + try await withSchemaChangeDriver(scope: scope) { driver in + let strategy = TriggerApplyStrategy.resolve( + isEdit: isEdit, + usesReplace: driver.triggerEditUsesReplace, + transactionalDDL: driver.supportsTransactionalDDL + ) + let dropSQL = originalName.flatMap { driver.generateDropTriggerSQL(name: $0, table: tableName) } + switch strategy { + case let .transactional(dropFirst): + try await runInTransaction(driver: driver, dropSQL: dropFirst ? dropSQL : nil, sql: sql) + case .dropThenCreate: + guard let dropSQL else { throw TriggerEditingError.dropUnavailable } + try await runDropThenCreate(driver: driver, dropSQL: dropSQL, sql: sql, rollback: originalDefinition) + case .direct: + _ = try await driver.execute(query: sql) + } } - await recordHistory(sql, connection: connection, executionTime: Date().timeIntervalSince(startedAt)) - AppCommands.shared.refreshData.send(DataRefreshRequest(connectionId: connection.id)) + await recordHistory(sql, scope: scope, connection: connection, executionTime: Date().timeIntervalSince(startedAt)) + AppCommands.shared.refreshData.send(DataRefreshRequest(connectionId: connection.id, scope: scope)) } - static func drop(connection: DatabaseConnection, tableName: String, name: String) async throws { + static func drop(scope: DatabaseScope, connection: DatabaseConnection, tableName: String, name: String) async throws { guard let driver = DatabaseManager.shared.driver(for: connection.id) else { throw TriggerEditingError.notConnected } @@ -118,9 +119,23 @@ enum TriggerEditing { } let startedAt = Date() - _ = try await driver.execute(query: dropSQL) - await recordHistory(dropSQL, connection: connection, executionTime: Date().timeIntervalSince(startedAt)) - AppCommands.shared.refreshData.send(DataRefreshRequest(connectionId: connection.id)) + try await withSchemaChangeDriver(scope: scope) { driver in + _ = try await driver.execute(query: dropSQL) + } + await recordHistory(dropSQL, scope: scope, connection: connection, executionTime: Date().timeIntervalSince(startedAt)) + AppCommands.shared.refreshData.send(DataRefreshRequest(connectionId: connection.id, scope: scope)) + } + + private static func withSchemaChangeDriver( + scope: DatabaseScope, + _ body: @Sendable @escaping (DatabaseDriver) async throws -> Void + ) async throws { + try await DatabaseManager.shared.withScopedDriver( + scope: scope, + route: DatabaseManager.shared.schemaChangeRoute(for: scope), + cancellation: .protectedWrite, + body + ) } static func runInTransaction(driver: DatabaseDriver, dropSQL: String?, sql: String) async throws { @@ -152,13 +167,19 @@ enum TriggerEditing { } } - private static func recordHistory(_ sql: String, connection: DatabaseConnection, executionTime: TimeInterval) async { + private static func recordHistory( + _ sql: String, + scope: DatabaseScope, + connection: DatabaseConnection, + executionTime: TimeInterval + ) async { await DatabaseManager.shared.historyRecorder.record( QueryHistoryRecordRequest( query: sql, connectionId: connection.id, - databaseName: DatabaseManager.shared.browseDatabaseName(for: connection), + databaseName: scope.database, databaseType: connection.type, + schemaName: scope.schema, source: .structureDDL, executionTime: executionTime, rowCount: -1, diff --git a/TablePro/Core/Services/Query/MetadataConnectionPool.swift b/TablePro/Core/Services/Query/MetadataConnectionPool.swift index 672a08e09..12843d1e2 100644 --- a/TablePro/Core/Services/Query/MetadataConnectionPool.swift +++ b/TablePro/Core/Services/Query/MetadataConnectionPool.swift @@ -101,6 +101,14 @@ final class MetadataConnectionPool { } } + #if DEBUG + /// Seeds a connected driver as the pooled connection for `scope`, so a test can observe + /// what runs on the pool without a plugin to open a real connection. + internal func injectEntry(_ driver: DatabaseDriver, scope: DatabaseScope, workload: Workload = .interactive) { + entries[Key(scope: scope, workload: workload)] = Entry(driver: driver) + } + #endif + private func releaseEntry(_ entry: Entry) { entry.inFlightCount -= 1 if entry.inFlightCount == 0, entry.closeWhenIdle { diff --git a/TablePro/Views/Structure/TableStructureView.swift b/TablePro/Views/Structure/TableStructureView.swift index d847f8297..412695ee2 100644 --- a/TablePro/Views/Structure/TableStructureView.swift +++ b/TablePro/Views/Structure/TableStructureView.swift @@ -410,6 +410,7 @@ struct TableStructureView: View { case .triggers: TriggerDetailView( triggers: triggers, + scope: scope, connection: connection, tableName: tableName, isLoading: !tabData.hasData(.triggers), diff --git a/TablePro/Views/Structure/TriggerDetailView.swift b/TablePro/Views/Structure/TriggerDetailView.swift index 50da9415e..1b39aae1e 100644 --- a/TablePro/Views/Structure/TriggerDetailView.swift +++ b/TablePro/Views/Structure/TriggerDetailView.swift @@ -43,6 +43,7 @@ private struct TriggerEditorSheetItem: Identifiable { struct TriggerDetailView: View { let triggers: [TriggerInfo] + let scope: DatabaseScope let connection: DatabaseConnection let tableName: String let isLoading: Bool @@ -122,6 +123,7 @@ struct TriggerDetailView: View { private func makeEditorSheet(for item: TriggerEditorSheetItem) -> some View { TriggerEditorView( + scope: scope, connection: connection, tableName: tableName, mode: item.mode, @@ -139,8 +141,12 @@ struct TriggerDetailView: View { private func editTrigger(_ trigger: TriggerInfo) { Task { - let driver = DatabaseManager.shared.driver(for: connection.id) - let fetched = try? await driver?.fetchTriggerDefinition(name: trigger.name, table: tableName) + let scope = scope + let tableName = tableName + let name = trigger.name + let fetched = try? await DatabaseManager.shared.withMetadataDriver(scope: scope) { driver in + try await driver.fetchTriggerDefinition(name: name, table: tableName) + } let sql = (fetched ?? nil) ?? trigger.statement editorSheet = TriggerEditorSheetItem( mode: .edit(originalName: trigger.name, originalDefinition: trigger.statement), @@ -152,7 +158,12 @@ struct TriggerDetailView: View { private func performDelete(_ trigger: TriggerInfo) { Task { do { - try await TriggerEditing.drop(connection: connection, tableName: tableName, name: trigger.name) + try await TriggerEditing.drop( + scope: scope, + connection: connection, + tableName: tableName, + name: trigger.name + ) } catch { actionError = error.localizedDescription } diff --git a/TablePro/Views/Structure/TriggerEditorView.swift b/TablePro/Views/Structure/TriggerEditorView.swift index 6225b3a6c..38dfd8bd2 100644 --- a/TablePro/Views/Structure/TriggerEditorView.swift +++ b/TablePro/Views/Structure/TriggerEditorView.swift @@ -16,6 +16,7 @@ struct TriggerEditorView: View { case edit(originalName: String, originalDefinition: String) } + let scope: DatabaseScope let connection: DatabaseConnection let tableName: String let mode: Mode @@ -29,7 +30,15 @@ struct TriggerEditorView: View { @Environment(\.colorScheme) private var colorScheme @AppStorage("structureCodeFontSize", store: AppStorageEnvironment.shared.defaults) private var fontSize: Double = 13 - init(connection: DatabaseConnection, tableName: String, mode: Mode, initialSQL: String, onClose: @escaping () -> Void) { + init( + scope: DatabaseScope, + connection: DatabaseConnection, + tableName: String, + mode: Mode, + initialSQL: String, + onClose: @escaping () -> Void + ) { + self.scope = scope self.connection = connection self.tableName = tableName self.mode = mode @@ -104,6 +113,7 @@ struct TriggerEditorView: View { defer { isApplying = false } do { try await TriggerEditing.apply( + scope: scope, connection: connection, tableName: tableName, sql: sql, diff --git a/TableProTests/Core/Database/DatabaseManagerSchemaChangeRoutingTests.swift b/TableProTests/Core/Database/DatabaseManagerSchemaChangeRoutingTests.swift index 302eeace2..7425ee777 100644 --- a/TableProTests/Core/Database/DatabaseManagerSchemaChangeRoutingTests.swift +++ b/TableProTests/Core/Database/DatabaseManagerSchemaChangeRoutingTests.swift @@ -5,7 +5,9 @@ // Pins the fix for #2015 and #2026: a table structure save runs its DDL on the scope // the editing tab owns, and moving the shared driver there is a mechanical detail no // UI reads. The save must not drag the sidebar, the toolbar or the saved default -// database onto the edited tab's database. +// database onto the edited tab's database. And it runs on a connection of its own +// wherever the engine can pool one, so its BEGIN never joins a transaction a query tab +// left open on the session driver. // import Combine @@ -82,6 +84,34 @@ private final class SchemaRoutingDriver: SchemaRoutingBaseDriver, PluginDatabase @Suite("DatabaseManager schema change routing", .serialized) @MainActor struct DatabaseManagerSchemaChangeRoutingTests { + /// An engine that changes database on its live connection but cannot pool a second one, + /// so a save has nowhere to run but the session driver and has to pin it. + private static let singleConnectionTypeId = "SchemaRoutingSingleConnection" + + private static var singleConnectionType: DatabaseType { + registerSingleConnectionTypeIfNeeded() + return DatabaseType(rawValue: singleConnectionTypeId) + } + + private static func registerSingleConnectionTypeIfNeeded() { + guard PluginMetadataRegistry.shared.snapshot(forRegisteredTypeId: singleConnectionTypeId) == nil else { + return + } + var capabilities = PluginMetadataSnapshot.CapabilityFlags.defaults + capabilities.supportsConnectionPooling = false + let snapshot = PluginMetadataSnapshot( + displayName: singleConnectionTypeId, iconName: "cylinder", defaultPort: 1_234, + requiresAuthentication: true, supportsForeignKeys: true, supportsSchemaEditing: true, + isDownloadable: false, primaryUrlScheme: "schemaroutingsingle", parameterStyle: .questionMark, + navigationModel: .standard, explainVariants: [], pathFieldRole: .database, + supportsHealthMonitor: false, urlSchemes: ["schemaroutingsingle"], postConnectActions: [], + brandColorHex: "#000000", queryLanguageName: "SQL", editorLanguage: .sql, + connectionMode: .network, supportsDatabaseSwitching: true, + capabilities: capabilities, schema: .defaults, editor: .defaults, connection: .defaults + ) + PluginMetadataRegistry.shared.register(snapshot: snapshot, forTypeId: singleConnectionTypeId) + } + private static func makeAddColumnChange(named name: String = "notes") -> SchemaChange { var column = EditableColumnDefinition.placeholder() column.name = name @@ -108,6 +138,26 @@ struct DatabaseManagerSchemaChangeRoutingTests { return (connection, pluginDriver) } + /// Stands in for the connection the pool would open on the scope, which the pool puts on + /// the scope's database and schema before handing it out. + private static func seedPooledDriver( + _ connection: DatabaseConnection, + scope: DatabaseScope + ) async throws -> SchemaRoutingDriver { + let pluginDriver = SchemaRoutingDriver(currentSchema: scope.schema) + let adapter = PluginDriverAdapter(connection: connection, pluginDriver: pluginDriver) + try await adapter.connect() + MetadataConnectionPool.shared.injectEntry(adapter, scope: scope) + return pluginDriver + } + + private static func tearDown(_ connections: DatabaseConnection...) { + for connection in connections { + MetadataConnectionPool.shared.closeAll(connectionId: connection.id) + DatabaseManager.shared.removeSession(for: connection.id) + } + } + private static func makeScope( _ connection: DatabaseConnection, database: String, @@ -116,18 +166,33 @@ struct DatabaseManagerSchemaChangeRoutingTests { DatabaseScope(connectionId: connection.id, database: database, schema: schema) } + @Test("A save takes a pooled connection wherever the engine can open one") + func schemaChangeRouteIsPooledWhereverTheEngineCanPool() throws { + let (pooling, _) = Self.makeSession(savedDatabase: "orders") + let (single, _) = Self.makeSession(type: Self.singleConnectionType, savedDatabase: "orders") + defer { Self.tearDown(pooling, single) } + + let poolingScope = try #require(Self.makeScope(pooling, database: "orders")) + let singleScope = try #require(Self.makeScope(single, database: "orders")) + let serverScope = try #require(Self.makeScope(pooling, database: "")) + + #expect(DatabaseManager.shared.schemaChangeRoute(for: poolingScope) == .pooled) + #expect(DatabaseManager.shared.schemaChangeRoute(for: singleScope) == .sessionDriver) + #expect(DatabaseManager.shared.schemaChangeRoute(for: serverScope) == .sessionDriver) + } + @Test("Schema changes run on the requested connection, not the last activated one") func schemaChangeUsesRequestedConnection() async throws { let (connectionA, driverA) = Self.makeSession(savedDatabase: "alpha") let (connectionB, driverB) = Self.makeSession(savedDatabase: "beta") DatabaseManager.shared.lastActiveSessionId = connectionA.id defer { - DatabaseManager.shared.removeSession(for: connectionA.id) - DatabaseManager.shared.removeSession(for: connectionB.id) + Self.tearDown(connectionA, connectionB) DatabaseManager.shared.lastActiveSessionId = nil } let scope = try #require(Self.makeScope(connectionB, database: "beta")) + let pooledB = try await Self.seedPooledDriver(connectionB, scope: scope) try await DatabaseManager.shared.executeSchemaChanges( tableName: "orders", changes: [Self.makeAddColumnChange()], @@ -135,20 +200,25 @@ struct DatabaseManagerSchemaChangeRoutingTests { scope: scope ) - #expect(driverB.executedQueries.count == 1) - #expect(driverB.executedQueries.first?.contains("ADD COLUMN") == true) + #expect(pooledB.executedQueries.count == 1) + #expect(pooledB.executedQueries.first?.contains("ADD COLUMN") == true) #expect(driverA.executedQueries.isEmpty) + #expect(driverB.executedQueries.isEmpty) } - @Test("A save runs on the tab's database without moving the browse cursor") - func schemaChangeRunsOnTheTabsDatabaseWithoutMovingTheBrowseCursor() async throws { + /// The session driver holds whatever transaction a query tab left open. A save that ran + /// there wrapped its DDL in a BEGIN that joined that transaction and a COMMIT that took the + /// tab's uncommitted work with it, or a ROLLBACK that threw it away. + @Test("A save runs on the tab's database on its own connection, leaving the session driver alone") + func schemaChangeRunsOnItsOwnConnectionWithoutMovingTheBrowseCursor() async throws { let (connection, driver) = Self.makeSession( savedDatabase: "analytics", browseDatabase: "inventory" ) - defer { DatabaseManager.shared.removeSession(for: connection.id) } + defer { Self.tearDown(connection) } let scope = try #require(Self.makeScope(connection, database: "orders")) + let pooled = try await Self.seedPooledDriver(connection, scope: scope) try await DatabaseManager.shared.executeSchemaChanges( tableName: "orders", changes: [Self.makeAddColumnChange()], @@ -156,25 +226,25 @@ struct DatabaseManagerSchemaChangeRoutingTests { scope: scope ) - #expect(driver.switchedDatabases.allSatisfy { $0 == "orders" }) - #expect(!driver.switchedDatabases.isEmpty) - #expect(driver.executedQueries.count == 1) + #expect(pooled.executedQueries.count == 1) + #expect(driver.executedQueries.isEmpty) + #expect(driver.switchedDatabases.isEmpty) let session = DatabaseManager.shared.session(for: connection.id) #expect(session?.browseDatabase == "inventory") #expect(session?.connection.database == "analytics") } - @Test("A save always pins its target database because nothing tracks where the driver is") - func schemaChangeAlwaysPinsItsTargetDatabase() async throws { - let (connection, driver) = Self.makeSession(savedDatabase: "orders") - defer { DatabaseManager.shared.removeSession(for: connection.id) } + @Test("An engine with one connection pins the session driver to the target database") + func singleConnectionEnginePinsItsTargetDatabase() async throws { + let (connection, driver) = Self.makeSession(type: Self.singleConnectionType, savedDatabase: "orders") + defer { Self.tearDown(connection) } let scope = try #require(Self.makeScope(connection, database: "orders")) try await DatabaseManager.shared.executeSchemaChanges( tableName: "orders", changes: [Self.makeAddColumnChange()], - databaseType: .mysql, + databaseType: Self.singleConnectionType, scope: scope ) @@ -186,18 +256,19 @@ struct DatabaseManagerSchemaChangeRoutingTests { @Test("A failed database pin aborts the save before any DDL runs") func failedDatabasePinAbortsSave() async throws { let (connection, driver) = Self.makeSession( + type: Self.singleConnectionType, savedDatabase: "orders", browseDatabase: "inventory" ) driver.switchDatabaseError = DatabaseError.queryFailed("unknown database") - defer { DatabaseManager.shared.removeSession(for: connection.id) } + defer { Self.tearDown(connection) } let scope = try #require(Self.makeScope(connection, database: "orders")) await #expect(throws: DatabaseError.self) { try await DatabaseManager.shared.executeSchemaChanges( tableName: "orders", changes: [Self.makeAddColumnChange()], - databaseType: .mysql, + databaseType: Self.singleConnectionType, scope: scope ) } @@ -211,9 +282,10 @@ struct DatabaseManagerSchemaChangeRoutingTests { type: .postgresql, savedDatabase: "orders" ) - defer { DatabaseManager.shared.removeSession(for: connection.id) } + defer { Self.tearDown(connection) } let scope = try #require(Self.makeScope(connection, database: "orders")) + let pooled = try await Self.seedPooledDriver(connection, scope: scope) try await DatabaseManager.shared.executeSchemaChanges( tableName: "orders", changes: [Self.makeAddColumnChange()], @@ -222,20 +294,22 @@ struct DatabaseManagerSchemaChangeRoutingTests { ) #expect(driver.switchedDatabases.isEmpty) - #expect(driver.executedQueries.count == 1) + #expect(driver.executedQueries.isEmpty) + #expect(pooled.executedQueries.count == 1) } - @Test("A schema-grouped engine keeps the edited table's schema across the database pin") + @Test("A schema-grouped engine qualifies the DDL with the edited table's schema") func schemaGroupedEngineKeepsTableSchema() async throws { let (connection, driver) = Self.makeSession( type: .mssql, savedDatabase: "orders", browseDatabase: "inventory", - browseSchema: "sales" + browseSchema: "dbo" ) - defer { DatabaseManager.shared.removeSession(for: connection.id) } + defer { Self.tearDown(connection) } let scope = try #require(Self.makeScope(connection, database: "orders", schema: "sales")) + let pooled = try await Self.seedPooledDriver(connection, scope: scope) try await DatabaseManager.shared.executeSchemaChanges( tableName: "orders", changes: [Self.makeAddColumnChange()], @@ -243,10 +317,9 @@ struct DatabaseManagerSchemaChangeRoutingTests { scope: scope ) - #expect(!driver.switchedDatabases.isEmpty) - #expect(driver.switchedDatabases.allSatisfy { $0 == "orders" }) - #expect(driver.currentSchema == "sales") - #expect(driver.executedQueries.first?.contains("`sales`.`orders`") == true) + #expect(pooled.executedQueries.first?.contains("`sales`.`orders`") == true) + #expect(driver.currentSchema == "dbo") + #expect(driver.executedQueries.isEmpty) } @Test("A save broadcasts a refresh scoped to the edited tab, not to the browse cursor") @@ -255,7 +328,7 @@ struct DatabaseManagerSchemaChangeRoutingTests { savedDatabase: "analytics", browseDatabase: "inventory" ) - defer { DatabaseManager.shared.removeSession(for: connection.id) } + defer { Self.tearDown(connection) } let recorder = RefreshRequestRecorder() let cancellable = AppCommands.shared.refreshData.sink { request in @@ -264,6 +337,7 @@ struct DatabaseManagerSchemaChangeRoutingTests { defer { cancellable.cancel() } let scope = try #require(Self.makeScope(connection, database: "orders")) + _ = try await Self.seedPooledDriver(connection, scope: scope) try await DatabaseManager.shared.executeSchemaChanges( tableName: "orders", changes: [Self.makeAddColumnChange()], diff --git a/TableProTests/Core/Database/TriggerInfoMappingTests.swift b/TableProTests/Core/Database/TriggerInfoMappingTests.swift index 2c30044a1..8fec17011 100644 --- a/TableProTests/Core/Database/TriggerInfoMappingTests.swift +++ b/TableProTests/Core/Database/TriggerInfoMappingTests.swift @@ -241,4 +241,42 @@ struct TriggerApplyExecutionTests { } #expect(stub.executedQueries == ["DROP TRIGGER t", "CREATE TRIGGER t", "RESTORE t"]) } + + /// The session driver holds whatever transaction a query tab left open, so a trigger edit + /// with a BEGIN of its own ran inside it and committed or rolled back the tab's work. + @Test("Apply and drop run on the pooled connection and leave the session driver alone") + func applyAndDropRunOnThePooledConnection() async throws { + let connection = TestFixtures.makeConnection(database: "app", type: .postgresql) + let sessionStub = StubTriggerDriver() + sessionStub.dropToReturn = "DROP TRIGGER t" + let session = ConnectionSession( + connection: connection, + driver: PluginDriverAdapter(connection: connection, pluginDriver: sessionStub) + ) + DatabaseManager.shared.injectSession(session, for: connection.id) + let scope = DatabaseScope(connectionId: connection.id, database: "app", schema: "public") + let pooledStub = StubTriggerDriver() + pooledStub.transactionalDDL = true + let pooledAdapter = PluginDriverAdapter(connection: connection, pluginDriver: pooledStub) + try await pooledAdapter.connect() + MetadataConnectionPool.shared.injectEntry(pooledAdapter, scope: scope) + defer { + MetadataConnectionPool.shared.closeAll(connectionId: connection.id) + DatabaseManager.shared.removeSession(for: connection.id) + } + + try await TriggerEditing.apply( + scope: scope, + connection: connection, + tableName: "orders", + sql: "CREATE TRIGGER t", + isEdit: false, + originalName: nil, + originalDefinition: nil + ) + try await TriggerEditing.drop(scope: scope, connection: connection, tableName: "orders", name: "t") + + #expect(pooledStub.executedQueries == ["BEGIN", "CREATE TRIGGER t", "COMMIT", "DROP TRIGGER t"]) + #expect(sessionStub.executedQueries.isEmpty) + } } diff --git a/docs/features/table-structure.mdx b/docs/features/table-structure.mdx index abd9646b4..dcc288f59 100644 --- a/docs/features/table-structure.mdx +++ b/docs/features/table-structure.mdx @@ -116,13 +116,15 @@ The queue outlives everything short of an explicit discard: closing the tab, clo Schema change preview with ALTER TABLE statements +A save runs on a connection of its own, so it never joins a transaction you have left open in a query tab. + ### When one statement fails A save is often several statements, run in order. Engines with transactional DDL roll the whole set back; MySQL, MariaDB, and Oracle commit each one as it runs, so everything before the failure has landed while the queue still holds all of it. An **Error Applying Changes** sheet reports what the server said: refresh before saving again, or the second save replays work the server already did. ## Triggers tab -Lists **Name**, **Timing** (BEFORE, AFTER, INSTEAD OF), **Event** (INSERT, UPDATE, DELETE), and **Enabled** where the engine reports it. Select one to read its `CREATE TRIGGER` statement from the catalog, with **Copy** and **Open in Editor**. **New Trigger**, **Edit**, and **Delete** sit in the action bar; the editor opens the trigger's real DDL, including the trigger function on PostgreSQL. +Lists **Name**, **Timing** (BEFORE, AFTER, INSTEAD OF), **Event** (INSERT, UPDATE, DELETE), and **Enabled** where the engine reports it. Select one to read its `CREATE TRIGGER` statement from the catalog, with **Copy** and **Open in Editor**. **New Trigger**, **Edit**, and **Delete** sit in the action bar; the editor opens the trigger's real DDL, including the trigger function on PostgreSQL. Trigger changes run on their own connection, like a structure save. Triggers are available for MySQL, MariaDB, PostgreSQL, SQLite, SQL Server, Oracle, libSQL, and Cloudflare D1; the tab is hidden elsewhere. Oracle does not return the trigger body, so the viewer and the editor start from the trigger header alone.