From 3834232600dbb5bf8a2d3ee572550e44fac80ec8 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Mon, 9 Mar 2026 13:00:55 +0700 Subject: [PATCH 1/2] refactor: delegate transaction handling to plugin drivers --- Plugins/MSSQLDriverPlugin/MSSQLPlugin.swift | 6 +++ .../MongoDBPluginDriver.swift | 1 + .../MySQLDriverPlugin/MySQLPluginDriver.swift | 6 +++ Plugins/OracleDriverPlugin/OraclePlugin.swift | 14 ++++++ TablePro/Core/Database/DatabaseDriver.swift | 17 ------- .../Core/Plugins/PluginDriverAdapter.swift | 6 +-- .../Core/Services/Export/ImportService.swift | 34 ++------------ .../Connection/DatabaseConnection.swift | 10 ----- .../MainContentCoordinator+Discard.swift | 21 ++------- ...ainContentCoordinator+MultiStatement.swift | 9 ++-- .../MainContentCoordinator+SQLPreview.swift | 17 +------ ...inContentCoordinator+TableOperations.swift | 12 ----- .../Views/Main/MainContentCoordinator.swift | 44 +++++++++++-------- 13 files changed, 69 insertions(+), 128 deletions(-) diff --git a/Plugins/MSSQLDriverPlugin/MSSQLPlugin.swift b/Plugins/MSSQLDriverPlugin/MSSQLPlugin.swift index 2ecdd12a..1a4ea406 100644 --- a/Plugins/MSSQLDriverPlugin/MSSQLPlugin.swift +++ b/Plugins/MSSQLDriverPlugin/MSSQLPlugin.swift @@ -374,6 +374,12 @@ final class MSSQLPluginDriver: PluginDatabaseDriver, @unchecked Sendable { _ = try await execute(query: "SELECT 1") } + // MARK: - Transaction Management + + func beginTransaction() async throws { + _ = try await execute(query: "BEGIN TRANSACTION") + } + // MARK: - Query Execution func execute(query: String) async throws -> PluginQueryResult { diff --git a/Plugins/MongoDBDriverPlugin/MongoDBPluginDriver.swift b/Plugins/MongoDBDriverPlugin/MongoDBPluginDriver.swift index afb2c962..742bf26b 100644 --- a/Plugins/MongoDBDriverPlugin/MongoDBPluginDriver.swift +++ b/Plugins/MongoDBDriverPlugin/MongoDBPluginDriver.swift @@ -18,6 +18,7 @@ final class MongoDBPluginDriver: PluginDatabaseDriver { var serverVersion: String? { mongoConnection?.serverVersion() } var currentSchema: String? { nil } + var supportsTransactions: Bool { false } init(config: DriverConnectionConfig) { self.config = config diff --git a/Plugins/MySQLDriverPlugin/MySQLPluginDriver.swift b/Plugins/MySQLDriverPlugin/MySQLPluginDriver.swift index 8f7715ab..431a58df 100644 --- a/Plugins/MySQLDriverPlugin/MySQLPluginDriver.swift +++ b/Plugins/MySQLDriverPlugin/MySQLPluginDriver.swift @@ -66,6 +66,12 @@ final class MySQLPluginDriver: PluginDatabaseDriver, @unchecked Sendable { _ = try await execute(query: "SELECT 1") } + // MARK: - Transaction Management + + func beginTransaction() async throws { + _ = try await execute(query: "START TRANSACTION") + } + // MARK: - Query Execution func execute(query: String) async throws -> PluginQueryResult { diff --git a/Plugins/OracleDriverPlugin/OraclePlugin.swift b/Plugins/OracleDriverPlugin/OraclePlugin.swift index 96aa412d..d0f1e3d6 100644 --- a/Plugins/OracleDriverPlugin/OraclePlugin.swift +++ b/Plugins/OracleDriverPlugin/OraclePlugin.swift @@ -80,6 +80,20 @@ final class OraclePluginDriver: PluginDatabaseDriver, @unchecked Sendable { _ = try await execute(query: "SELECT 1 FROM DUAL") } + // MARK: - Transaction Management + + func beginTransaction() async throws { + // Oracle uses implicit transactions — no explicit BEGIN needed + } + + func commitTransaction() async throws { + _ = try await execute(query: "COMMIT") + } + + func rollbackTransaction() async throws { + _ = try await execute(query: "ROLLBACK") + } + // MARK: - Query Execution func execute(query: String) async throws -> PluginQueryResult { diff --git a/TablePro/Core/Database/DatabaseDriver.swift b/TablePro/Core/Database/DatabaseDriver.swift index 4b8321d2..c38f2187 100644 --- a/TablePro/Core/Database/DatabaseDriver.swift +++ b/TablePro/Core/Database/DatabaseDriver.swift @@ -290,23 +290,6 @@ extension DatabaseDriver { .warning("Failed to set query timeout: \(error.localizedDescription)") } } - - // MARK: - Default Transaction Implementation - - /// Default transaction implementation using database-specific SQL - func beginTransaction() async throws { - let sql = connection.type.beginTransactionSQL - guard !sql.isEmpty else { return } - _ = try await execute(query: sql) - } - - func commitTransaction() async throws { - _ = try await execute(query: "COMMIT") - } - - func rollbackTransaction() async throws { - _ = try await execute(query: "ROLLBACK") - } } /// Factory for creating database drivers via plugin lookup diff --git a/TablePro/Core/Plugins/PluginDriverAdapter.swift b/TablePro/Core/Plugins/PluginDriverAdapter.swift index 7a39ef9a..81c12dc7 100644 --- a/TablePro/Core/Plugins/PluginDriverAdapter.swift +++ b/TablePro/Core/Plugins/PluginDriverAdapter.swift @@ -239,15 +239,15 @@ final class PluginDriverAdapter: DatabaseDriver, SchemaSwitchable { // MARK: - Transaction Management func beginTransaction() async throws { - _ = try await pluginDriver.execute(query: "BEGIN") + try await pluginDriver.beginTransaction() } func commitTransaction() async throws { - _ = try await pluginDriver.execute(query: "COMMIT") + try await pluginDriver.commitTransaction() } func rollbackTransaction() async throws { - _ = try await pluginDriver.execute(query: "ROLLBACK") + try await pluginDriver.rollbackTransaction() } // MARK: - Schema Switching diff --git a/TablePro/Core/Services/Export/ImportService.swift b/TablePro/Core/Services/Export/ImportService.swift index 7cc39514..19c591fd 100644 --- a/TablePro/Core/Services/Export/ImportService.swift +++ b/TablePro/Core/Services/Export/ImportService.swift @@ -133,10 +133,7 @@ final class ImportService { // 5. Begin transaction (if enabled) if config.wrapInTransaction { - let beginStmt = connection.type.beginTransactionSQL - if !beginStmt.isEmpty { - _ = try await driver.execute(query: beginStmt) - } + try await driver.beginTransaction() } // 6. Parse and execute statements (single pass — no prior counting pass) @@ -174,10 +171,7 @@ final class ImportService { // 7. Commit transaction (if enabled) if config.wrapInTransaction { - let commitStmt = commitStatement(for: connection.type) - if !commitStmt.isEmpty { - _ = try await driver.execute(query: commitStmt) - } + try await driver.commitTransaction() } // 8. Re-enable FK checks (if enabled) - AFTER transaction @@ -191,10 +185,7 @@ final class ImportService { // Rollback on error - this is CRITICAL and must not fail silently if config.wrapInTransaction { do { - let rollbackStmt = rollbackStatement(for: connection.type) - if !rollbackStmt.isEmpty { - _ = try await driver.execute(query: rollbackStmt) - } + try await driver.rollbackTransaction() } catch let rollbackError { throw ImportError.rollbackFailed(rollbackError.localizedDescription) } @@ -311,23 +302,4 @@ final class ImportService { return [] } } - - - private func commitStatement(for dbType: DatabaseType) -> String { - switch dbType { - case .mongodb, .redis, .clickhouse: - return "" - default: - return "COMMIT" - } - } - - private func rollbackStatement(for dbType: DatabaseType) -> String { - switch dbType { - case .mongodb, .redis, .clickhouse: - return "" - default: - return "ROLLBACK" - } - } } diff --git a/TablePro/Models/Connection/DatabaseConnection.swift b/TablePro/Models/Connection/DatabaseConnection.swift index 5116b48d..16620162 100644 --- a/TablePro/Models/Connection/DatabaseConnection.swift +++ b/TablePro/Models/Connection/DatabaseConnection.swift @@ -287,16 +287,6 @@ enum DatabaseType: String, CaseIterable, Identifiable, Codable { } } - var beginTransactionSQL: String { - switch self { - case .mysql, .mariadb: return "START TRANSACTION" - case .postgresql, .redshift, .sqlite: return "BEGIN" - case .mssql: return "BEGIN TRANSACTION" - case .oracle: return "" - case .mongodb, .redis, .clickhouse: return "" - } - } - /// Whether this database type supports SQL-based schema editing (ALTER TABLE etc.) var supportsSchemaEditing: Bool { switch self { diff --git a/TablePro/Views/Main/Extensions/MainContentCoordinator+Discard.swift b/TablePro/Views/Main/Extensions/MainContentCoordinator+Discard.swift index bd67fe43..c6153331 100644 --- a/TablePro/Views/Main/Extensions/MainContentCoordinator+Discard.swift +++ b/TablePro/Views/Main/Extensions/MainContentCoordinator+Discard.swift @@ -16,32 +16,19 @@ extension MainContentCoordinator { throw DatabaseError.notConnected } - let dbType = connection.type - let supportsTransactions = dbType != .redis && dbType != .mongodb && dbType != .clickhouse - var allStatements: [ParameterizedStatement] = [] - - if supportsTransactions { - allStatements.append(ParameterizedStatement(sql: dbType.beginTransactionSQL, parameters: [])) - } - - allStatements.append(contentsOf: statements) - - if supportsTransactions { - allStatements.append(ParameterizedStatement(sql: "COMMIT", parameters: [])) - } + try await driver.beginTransaction() do { - for stmt in allStatements { + for stmt in statements { if stmt.parameters.isEmpty { _ = try await driver.execute(query: stmt.sql) } else { _ = try await driver.executeParameterized(query: stmt.sql, parameters: stmt.parameters) } } + try await driver.commitTransaction() } catch { - if supportsTransactions { - _ = try? await driver.execute(query: "ROLLBACK") - } + try? await driver.rollbackTransaction() throw error } } diff --git a/TablePro/Views/Main/Extensions/MainContentCoordinator+MultiStatement.swift b/TablePro/Views/Main/Extensions/MainContentCoordinator+MultiStatement.swift index 0042b3ff..d4a3e65e 100644 --- a/TablePro/Views/Main/Extensions/MainContentCoordinator+MultiStatement.swift +++ b/TablePro/Views/Main/Extensions/MainContentCoordinator+MultiStatement.swift @@ -33,7 +33,6 @@ extension MainContentCoordinator { let conn = connection let tabId = tabManager.tabs[index].id let totalCount = statements.count - let dbType = connection.type currentQueryTask = Task { var cumulativeTime: TimeInterval = 0 @@ -49,12 +48,12 @@ extension MainContentCoordinator { } // Wrap in a transaction for atomicity - _ = try await driver.execute(query: dbType.beginTransactionSQL) + try await driver.beginTransaction() for (stmtIndex, sql) in statements.enumerated() { guard !Task.isCancelled else { break } guard capturedGeneration == queryGeneration else { - _ = try? await driver.execute(query: "ROLLBACK") + try? await driver.rollbackTransaction() return } @@ -86,7 +85,7 @@ extension MainContentCoordinator { } // Commit the transaction - _ = try await driver.execute(query: "COMMIT") + try await driver.commitTransaction() // All statements succeeded — update tab with results await MainActor.run { @@ -140,7 +139,7 @@ extension MainContentCoordinator { } catch { // Rollback on failure if let driver = DatabaseManager.shared.driver(for: conn.id) { - _ = try? await driver.execute(query: "ROLLBACK") + try? await driver.rollbackTransaction() } guard capturedGeneration == queryGeneration else { return } diff --git a/TablePro/Views/Main/Extensions/MainContentCoordinator+SQLPreview.swift b/TablePro/Views/Main/Extensions/MainContentCoordinator+SQLPreview.swift index 22edfe10..61265432 100644 --- a/TablePro/Views/Main/Extensions/MainContentCoordinator+SQLPreview.swift +++ b/TablePro/Views/Main/Extensions/MainContentCoordinator+SQLPreview.swift @@ -51,6 +51,7 @@ extension MainContentCoordinator { /// Assembles all pending SQL statements (cell edits + table operations) in execution order. /// Used by both `saveChanges()` and `generatePreviewSQL()` to ensure consistency. + /// Transaction wrapping is handled by the caller using driver protocol methods. func assemblePendingStatements( pendingTruncates: Set, pendingDeletes: Set, @@ -59,7 +60,6 @@ extension MainContentCoordinator { var allStatements: [ParameterizedStatement] = [] let dbType = connection.type - let hasEditedCells = changeManager.hasChanges let hasPendingTableOps = !pendingTruncates.isEmpty || !pendingDeletes.isEmpty // Check if any table operation needs FK disabled (must be outside transaction) @@ -74,25 +74,16 @@ extension MainContentCoordinator { }) } - // Wrap all operations in a single transaction when we have multiple operations - let needsTransaction = hasEditedCells && hasPendingTableOps - if needsTransaction { - let beginSQL = dbType.beginTransactionSQL - allStatements.append(ParameterizedStatement(sql: beginSQL, parameters: [])) - } - - if hasEditedCells { + if changeManager.hasChanges { let editStatements = try changeManager.generateSQL() allStatements.append(contentsOf: editStatements) } if hasPendingTableOps { - // Generate table operation SQL WITHOUT FK handling (already done above) let tableOpStatements = generateTableOperationSQL( truncates: pendingTruncates, deletes: pendingDeletes, options: tableOperationOptions, - wrapInTransaction: !needsTransaction, includeFKHandling: false ) allStatements.append(contentsOf: tableOpStatements.map { @@ -100,10 +91,6 @@ extension MainContentCoordinator { }) } - if needsTransaction { - allStatements.append(ParameterizedStatement(sql: "COMMIT", parameters: [])) - } - // FK re-enable must be LAST, after transaction commits if needsDisableFK { allStatements.append(contentsOf: fkEnableStatements(for: dbType).map { diff --git a/TablePro/Views/Main/Extensions/MainContentCoordinator+TableOperations.swift b/TablePro/Views/Main/Extensions/MainContentCoordinator+TableOperations.swift index 911c8dd1..56be192f 100644 --- a/TablePro/Views/Main/Extensions/MainContentCoordinator+TableOperations.swift +++ b/TablePro/Views/Main/Extensions/MainContentCoordinator+TableOperations.swift @@ -15,14 +15,12 @@ extension MainContentCoordinator { /// - truncates: Set of table names to truncate /// - deletes: Set of table names to drop /// - options: Per-table options for FK and cascade handling - /// - wrapInTransaction: Whether to wrap statements in BEGIN/COMMIT /// - includeFKHandling: Whether to include FK disable/enable statements (set false when caller handles FK) /// - Returns: Array of SQL statements to execute func generateTableOperationSQL( truncates: Set, deletes: Set, options: [String: TableOperationOptions], - wrapInTransaction: Bool = true, includeFKHandling: Bool = true ) -> [String] { var statements: [String] = [] @@ -42,12 +40,6 @@ extension MainContentCoordinator { statements.append(contentsOf: fkDisableStatements(for: dbType)) } - // Wrap in transaction for atomicity - let needsTransaction = wrapInTransaction && (sortedTruncates.count + sortedDeletes.count) > 1 - if needsTransaction { - statements.append(dbType.beginTransactionSQL) - } - for tableName in sortedTruncates { let quotedName = dbType.quoteIdentifier(tableName) let tableOptions = options[tableName] ?? TableOperationOptions() @@ -65,10 +57,6 @@ extension MainContentCoordinator { statements.append(dropTableStatement(tableName: tableName, quotedName: quotedName, isView: viewNames.contains(tableName), options: tableOptions, dbType: dbType)) } - if needsTransaction { - statements.append("COMMIT") - } - // FK re-enable must be OUTSIDE transaction to ensure it runs even on rollback if needsDisableFK { statements.append(contentsOf: fkEnableStatements(for: dbType)) diff --git a/TablePro/Views/Main/MainContentCoordinator.swift b/TablePro/Views/Main/MainContentCoordinator.swift index 8c9e2b89..9d66f436 100644 --- a/TablePro/Views/Main/MainContentCoordinator.swift +++ b/TablePro/Views/Main/MainContentCoordinator.swift @@ -1047,26 +1047,34 @@ final class MainContentCoordinator { throw DatabaseError.notConnected } - for (i, statement) in validStatements.enumerated() { - let statementStartTime = Date() - // Execute parameterized query if has parameters, otherwise use regular execute - if statement.parameters.isEmpty { - _ = try await driver.execute(query: statement.sql) - } else { - _ = try await driver.executeParameterized(query: statement.sql, parameters: statement.parameters) - } + try await driver.beginTransaction() - let executionTime = Date().timeIntervalSince(statementStartTime) + do { + for statement in validStatements { + let statementStartTime = Date() + if statement.parameters.isEmpty { + _ = try await driver.execute(query: statement.sql) + } else { + _ = try await driver.executeParameterized(query: statement.sql, parameters: statement.parameters) + } - QueryHistoryManager.shared.recordQuery( - query: statement.sql.trimmingCharacters(in: .whitespacesAndNewlines), - connectionId: conn.id, - databaseName: conn.database, - executionTime: executionTime, - rowCount: 0, - wasSuccessful: true, - errorMessage: nil - ) + let executionTime = Date().timeIntervalSince(statementStartTime) + + QueryHistoryManager.shared.recordQuery( + query: statement.sql.trimmingCharacters(in: .whitespacesAndNewlines), + connectionId: conn.id, + databaseName: conn.database, + executionTime: executionTime, + rowCount: 0, + wasSuccessful: true, + errorMessage: nil + ) + } + + try await driver.commitTransaction() + } catch { + try? await driver.rollbackTransaction() + throw error } changeManager.clearChanges() From 41e9c1e4c69bc5490aa06970c12813e09ab0c3a6 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Mon, 9 Mar 2026 13:09:43 +0700 Subject: [PATCH 2/2] fix: add no-op transaction methods for non-transactional databases --- Plugins/ClickHouseDriverPlugin/ClickHousePlugin.swift | 3 +++ Plugins/MongoDBDriverPlugin/MongoDBPluginDriver.swift | 3 +++ Plugins/OracleDriverPlugin/OraclePlugin.swift | 8 -------- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/Plugins/ClickHouseDriverPlugin/ClickHousePlugin.swift b/Plugins/ClickHouseDriverPlugin/ClickHousePlugin.swift index 923b4017..5caab6a9 100644 --- a/Plugins/ClickHouseDriverPlugin/ClickHousePlugin.swift +++ b/Plugins/ClickHouseDriverPlugin/ClickHousePlugin.swift @@ -64,6 +64,9 @@ final class ClickHousePluginDriver: PluginDatabaseDriver, @unchecked Sendable { var serverVersion: String? { _serverVersion } var supportsSchemas: Bool { false } var supportsTransactions: Bool { false } + func beginTransaction() async throws {} + func commitTransaction() async throws {} + func rollbackTransaction() async throws {} var currentSchema: String? { nil } init(config: DriverConnectionConfig) { diff --git a/Plugins/MongoDBDriverPlugin/MongoDBPluginDriver.swift b/Plugins/MongoDBDriverPlugin/MongoDBPluginDriver.swift index 742bf26b..02ea8e5c 100644 --- a/Plugins/MongoDBDriverPlugin/MongoDBPluginDriver.swift +++ b/Plugins/MongoDBDriverPlugin/MongoDBPluginDriver.swift @@ -19,6 +19,9 @@ final class MongoDBPluginDriver: PluginDatabaseDriver { var serverVersion: String? { mongoConnection?.serverVersion() } var currentSchema: String? { nil } var supportsTransactions: Bool { false } + func beginTransaction() async throws {} + func commitTransaction() async throws {} + func rollbackTransaction() async throws {} init(config: DriverConnectionConfig) { self.config = config diff --git a/Plugins/OracleDriverPlugin/OraclePlugin.swift b/Plugins/OracleDriverPlugin/OraclePlugin.swift index d0f1e3d6..b63f4bc0 100644 --- a/Plugins/OracleDriverPlugin/OraclePlugin.swift +++ b/Plugins/OracleDriverPlugin/OraclePlugin.swift @@ -86,14 +86,6 @@ final class OraclePluginDriver: PluginDatabaseDriver, @unchecked Sendable { // Oracle uses implicit transactions — no explicit BEGIN needed } - func commitTransaction() async throws { - _ = try await execute(query: "COMMIT") - } - - func rollbackTransaction() async throws { - _ = try await execute(query: "ROLLBACK") - } - // MARK: - Query Execution func execute(query: String) async throws -> PluginQueryResult {