From fab9f87dc99c1286df6c7bc162a8f1e1aa087eeb Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Fri, 26 Dec 2025 22:42:56 +0700 Subject: [PATCH 01/11] Add table delete/truncate confirmation dialog Implement TablePlus-like confirmation dialog for delete/truncate table operations with options for: - Ignore foreign key checks - Cascade (delete dependent rows) Database-specific SQL generation: - MySQL/MariaDB: SET FOREIGN_KEY_CHECKS=0/1 - PostgreSQL: CASCADE keyword, session_replication_role - SQLite: PRAGMA foreign_keys --- TablePro/ContentView.swift | 13 +- TablePro/Models/ConnectionSession.swift | 1 + TablePro/Models/TableOperationOptions.swift | 21 +++ .../Views/Main/MainContentCoordinator.swift | 109 +++++++++-- .../Main/MainContentNotificationHandler.swift | 11 +- TablePro/Views/MainContentView.swift | 5 + TablePro/Views/Sidebar/SidebarView.swift | 115 +++++++++--- .../Views/Sidebar/TableOperationDialog.swift | 174 ++++++++++++++++++ 8 files changed, 412 insertions(+), 37 deletions(-) create mode 100644 TablePro/Models/TableOperationOptions.swift create mode 100644 TablePro/Views/Sidebar/TableOperationDialog.swift diff --git a/TablePro/ContentView.swift b/TablePro/ContentView.swift index fcd265419..42df53271 100644 --- a/TablePro/ContentView.swift +++ b/TablePro/ContentView.swift @@ -157,7 +157,9 @@ struct ContentView: View { showAllTablesMetadata() }, pendingTruncates: sessionPendingTruncatesBinding, - pendingDeletes: sessionPendingDeletesBinding + pendingDeletes: sessionPendingDeletesBinding, + tableOperationOptions: sessionTableOperationOptionsBinding, + databaseType: currentSession?.connection.type ?? .sqlite ) } .navigationSplitViewColumnWidth(min: 200, ideal: 250, max: 350) @@ -169,6 +171,7 @@ struct ContentView: View { selectedTables: sessionSelectedTablesBinding, pendingTruncates: sessionPendingTruncatesBinding, pendingDeletes: sessionPendingDeletesBinding, + tableOperationOptions: sessionTableOperationOptionsBinding, isInspectorPresented: $isInspectorPresented ) .id(currentSession!.id) @@ -249,6 +252,14 @@ struct ContentView: View { ) } + private var sessionTableOperationOptionsBinding: Binding<[String: TableOperationOptions]> { + createSessionBinding( + get: { $0.tableOperationOptions }, + set: { $0.tableOperationOptions = $1 }, + defaultValue: [:] + ) + } + // MARK: - Actions private func connectToDatabase(_ connection: DatabaseConnection) { diff --git a/TablePro/Models/ConnectionSession.swift b/TablePro/Models/ConnectionSession.swift index f277850aa..1074538be 100644 --- a/TablePro/Models/ConnectionSession.swift +++ b/TablePro/Models/ConnectionSession.swift @@ -22,6 +22,7 @@ struct ConnectionSession: Identifiable { var selectedTabId: UUID? var pendingTruncates: Set = [] var pendingDeletes: Set = [] + var tableOperationOptions: [String: TableOperationOptions] = [:] // Metadata let connectedAt: Date diff --git a/TablePro/Models/TableOperationOptions.swift b/TablePro/Models/TableOperationOptions.swift new file mode 100644 index 000000000..2554562e5 --- /dev/null +++ b/TablePro/Models/TableOperationOptions.swift @@ -0,0 +1,21 @@ +// +// TableOperationOptions.swift +// TablePro +// +// Model for table delete/truncate operation options. +// Supports foreign key constraint handling and cascade operations. +// + +import Foundation + +/// Options for table delete/truncate operations +struct TableOperationOptions: Codable, Equatable { + var ignoreForeignKeys: Bool = false + var cascade: Bool = false +} + +/// Type of table operation +enum TableOperationType: String, Codable { + case truncate + case delete +} diff --git a/TablePro/Views/Main/MainContentCoordinator.swift b/TablePro/Views/Main/MainContentCoordinator.swift index 6942c9fbc..19944cd9f 100644 --- a/TablePro/Views/Main/MainContentCoordinator.swift +++ b/TablePro/Views/Main/MainContentCoordinator.swift @@ -554,7 +554,8 @@ final class MainContentCoordinator: ObservableObject { func saveChanges( pendingTruncates: inout Set, - pendingDeletes: inout Set + pendingDeletes: inout Set, + tableOperationOptions: inout [String: TableOperationOptions] ) { let hasEditedCells = changeManager.hasChanges let hasPendingTableOps = !pendingTruncates.isEmpty || !pendingDeletes.isEmpty @@ -568,14 +569,13 @@ final class MainContentCoordinator: ObservableObject { } if hasPendingTableOps { - for tableName in pendingTruncates { - let quotedName = connection.type.quoteIdentifier(tableName) - allStatements.append("TRUNCATE TABLE \(quotedName)") - } - for tableName in pendingDeletes { - let quotedName = connection.type.quoteIdentifier(tableName) - allStatements.append("DROP TABLE \(quotedName)") - } + // Generate table operation SQL with FK/cascade options + let tableOpStatements = generateTableOperationSQL( + truncates: pendingTruncates, + deletes: pendingDeletes, + options: tableOperationOptions + ) + allStatements.append(contentsOf: tableOpStatements) } guard !allStatements.isEmpty else { @@ -586,20 +586,107 @@ final class MainContentCoordinator: ObservableObject { } let sql = allStatements.joined(separator: ";\n") - executeCommitSQL(sql, clearTableOps: hasPendingTableOps, pendingTruncates: &pendingTruncates, pendingDeletes: &pendingDeletes) + executeCommitSQL( + sql, + clearTableOps: hasPendingTableOps, + pendingTruncates: &pendingTruncates, + pendingDeletes: &pendingDeletes, + tableOperationOptions: &tableOperationOptions + ) + } + + /// Generate SQL for table truncate/delete operations with FK/cascade options + private func generateTableOperationSQL( + truncates: Set, + deletes: Set, + options: [String: TableOperationOptions] + ) -> [String] { + var statements: [String] = [] + let dbType = connection.type + + // Check if any operation needs FK disabled + let needsDisableFK = truncates.union(deletes).contains { tableName in + options[tableName]?.ignoreForeignKeys == true + } + + if needsDisableFK { + statements.append(fkDisableStatement(for: dbType)) + } + + for tableName in truncates { + let quotedName = dbType.quoteIdentifier(tableName) + let opts = options[tableName] ?? TableOperationOptions() + statements.append(truncateStatement(tableName: quotedName, options: opts, dbType: dbType)) + } + + for tableName in deletes { + let quotedName = dbType.quoteIdentifier(tableName) + let opts = options[tableName] ?? TableOperationOptions() + statements.append(dropTableStatement(tableName: quotedName, options: opts, dbType: dbType)) + } + + if needsDisableFK { + statements.append(fkEnableStatement(for: dbType)) + } + + return statements + } + + private func fkDisableStatement(for dbType: DatabaseType) -> String { + return switch dbType { + case .mysql, .mariadb: "SET FOREIGN_KEY_CHECKS=0" + case .postgresql: "SET session_replication_role = 'replica'" + case .sqlite: "PRAGMA foreign_keys = OFF" + } + } + + private func fkEnableStatement(for dbType: DatabaseType) -> String { + return switch dbType { + case .mysql, .mariadb: "SET FOREIGN_KEY_CHECKS=1" + case .postgresql: "SET session_replication_role = 'origin'" + case .sqlite: "PRAGMA foreign_keys = ON" + } + } + + private func truncateStatement(tableName: String, options: TableOperationOptions, dbType: DatabaseType) -> String { + return switch dbType { + case .mysql, .mariadb: "TRUNCATE TABLE \(tableName)" + case .postgresql: options.cascade ? "TRUNCATE TABLE \(tableName) CASCADE" : "TRUNCATE TABLE \(tableName)" + case .sqlite: "DELETE FROM \(tableName)" + } + } + + private func dropTableStatement(tableName: String, options: TableOperationOptions, dbType: DatabaseType) -> String { + let cascade = options.cascade ? " CASCADE" : "" + return switch dbType { + case .mysql, .mariadb, .postgresql: "DROP TABLE \(tableName)\(cascade)" + case .sqlite: "DROP TABLE \(tableName)" + } } private func executeCommitSQL( _ sql: String, clearTableOps: Bool, pendingTruncates: inout Set, - pendingDeletes: inout Set + pendingDeletes: inout Set, + tableOperationOptions: inout [String: TableOperationOptions] ) { guard !sql.isEmpty else { return } let deletedTables = Set(pendingDeletes) + let truncatedTables = Set(pendingTruncates) let conn = connection + // Clear operations immediately (before async execution) + if clearTableOps { + pendingTruncates.removeAll() + pendingDeletes.removeAll() + // Clear options for processed tables + for table in deletedTables.union(truncatedTables) { + tableOperationOptions.removeValue(forKey: table) + } + } + Task { let overallStartTime = Date() diff --git a/TablePro/Views/Main/MainContentNotificationHandler.swift b/TablePro/Views/Main/MainContentNotificationHandler.swift index d9ee21197..bf530e502 100644 --- a/TablePro/Views/Main/MainContentNotificationHandler.swift +++ b/TablePro/Views/Main/MainContentNotificationHandler.swift @@ -26,6 +26,7 @@ final class MainContentNotificationHandler: ObservableObject { private let selectedTables: Binding> private let pendingTruncates: Binding> private let pendingDeletes: Binding> + private let tableOperationOptions: Binding<[String: TableOperationOptions]> private let isInspectorPresented: Binding private let editingCell: Binding @@ -43,6 +44,7 @@ final class MainContentNotificationHandler: ObservableObject { selectedTables: Binding>, pendingTruncates: Binding>, pendingDeletes: Binding>, + tableOperationOptions: Binding<[String: TableOperationOptions]>, isInspectorPresented: Binding, editingCell: Binding ) { @@ -53,6 +55,7 @@ final class MainContentNotificationHandler: ObservableObject { self.selectedTables = selectedTables self.pendingTruncates = pendingTruncates self.pendingDeletes = pendingDeletes + self.tableOperationOptions = tableOperationOptions self.isInspectorPresented = isInspectorPresented self.editingCell = editingCell @@ -308,9 +311,15 @@ final class MainContentNotificationHandler: ObservableObject { private func handleSaveChanges() { var truncates = pendingTruncates.wrappedValue var deletes = pendingDeletes.wrappedValue - coordinator?.saveChanges(pendingTruncates: &truncates, pendingDeletes: &deletes) + var options = tableOperationOptions.wrappedValue + coordinator?.saveChanges( + pendingTruncates: &truncates, + pendingDeletes: &deletes, + tableOperationOptions: &options + ) pendingTruncates.wrappedValue = truncates pendingDeletes.wrappedValue = deletes + tableOperationOptions.wrappedValue = options } // MARK: - UI Operations diff --git a/TablePro/Views/MainContentView.swift b/TablePro/Views/MainContentView.swift index 493559aaf..6a862ea9b 100644 --- a/TablePro/Views/MainContentView.swift +++ b/TablePro/Views/MainContentView.swift @@ -21,6 +21,7 @@ struct MainContentView: View { @Binding var selectedTables: Set @Binding var pendingTruncates: Set @Binding var pendingDeletes: Set + @Binding var tableOperationOptions: [String: TableOperationOptions] @Binding var isInspectorPresented: Bool // MARK: - State Objects @@ -49,6 +50,7 @@ struct MainContentView: View { selectedTables: Binding>, pendingTruncates: Binding>, pendingDeletes: Binding>, + tableOperationOptions: Binding<[String: TableOperationOptions]>, isInspectorPresented: Binding ) { self.connection = connection @@ -56,6 +58,7 @@ struct MainContentView: View { self._selectedTables = selectedTables self._pendingTruncates = pendingTruncates self._pendingDeletes = pendingDeletes + self._tableOperationOptions = tableOperationOptions self._isInspectorPresented = isInspectorPresented // Create state objects @@ -223,6 +226,7 @@ struct MainContentView: View { selectedTables: $selectedTables, pendingTruncates: $pendingTruncates, pendingDeletes: $pendingDeletes, + tableOperationOptions: $tableOperationOptions, isInspectorPresented: $isInspectorPresented, editingCell: $editingCell ) @@ -394,6 +398,7 @@ struct MainContentView: View { selectedTables: .constant([]), pendingTruncates: .constant([]), pendingDeletes: .constant([]), + tableOperationOptions: .constant([:]), isInspectorPresented: .constant(false) ) .frame(width: 1000, height: 600) diff --git a/TablePro/Views/Sidebar/SidebarView.swift b/TablePro/Views/Sidebar/SidebarView.swift index bbb3cad16..273abcf3c 100644 --- a/TablePro/Views/Sidebar/SidebarView.swift +++ b/TablePro/Views/Sidebar/SidebarView.swift @@ -20,6 +20,8 @@ struct SidebarView: View { // Pending table operations @Binding var pendingTruncates: Set @Binding var pendingDeletes: Set + @Binding var tableOperationOptions: [String: TableOperationOptions] + let databaseType: DatabaseType @State private var isLoading = false @State private var errorMessage: String? @@ -31,6 +33,11 @@ struct SidebarView: View { /// Whether the tables section is expanded @State private var isTablesExpanded = true + /// State for table operation confirmation dialog + @State private var showOperationDialog = false + @State private var pendingOperationType: TableOperationType? + @State private var pendingOperationTables: [String] = [] + /// Filtered tables based on search text private var filteredTables: [TableInfo] { guard !searchText.isEmpty else { return tables } @@ -96,6 +103,25 @@ struct SidebarView: View { } } } + .sheet(isPresented: $showOperationDialog) { + if let operationType = pendingOperationType, + let firstTable = pendingOperationTables.first { + TableOperationDialog( + isPresented: $showOperationDialog, + tableName: pendingOperationTables.count > 1 + ? "\(pendingOperationTables.count) tables" + : firstTable, + operationType: operationType, + databaseType: databaseType, + onConfirm: { options in + confirmOperation(options: options) + } + ) + } + } + .onChange(of: showOperationDialog) { _, isPresented in + AppState.shared.isSheetPresented = isPresented + } } // MARK: - Search Field @@ -257,40 +283,79 @@ struct SidebarView: View { /// Batch toggle truncate for all selected tables private func batchToggleTruncate() { - var updatedDeletes = pendingDeletes - var updatedTruncates = pendingTruncates - - let tablesToToggle = selectedTables.isEmpty ? [] : selectedTables - for table in tablesToToggle { - updatedDeletes.remove(table.name) - if updatedTruncates.contains(table.name) { - updatedTruncates.remove(table.name) - } else { - updatedTruncates.insert(table.name) + let tablesToToggle = selectedTables.isEmpty ? [] : Array(selectedTables.map { $0.name }) + guard !tablesToToggle.isEmpty else { return } + + // Check if all tables are already pending truncate - if so, remove them + let allAlreadyPending = tablesToToggle.allSatisfy { pendingTruncates.contains($0) } + if allAlreadyPending { + // Remove from pending + var updated = pendingTruncates + for name in tablesToToggle { + updated.remove(name) + tableOperationOptions.removeValue(forKey: name) } + pendingTruncates = updated + } else { + // Show dialog to confirm operation + pendingOperationType = .truncate + pendingOperationTables = tablesToToggle + showOperationDialog = true } - - pendingDeletes = updatedDeletes - pendingTruncates = updatedTruncates } - + /// Batch toggle delete for all selected tables private func batchToggleDelete() { - var updatedDeletes = pendingDeletes + let tablesToToggle = selectedTables.isEmpty ? [] : Array(selectedTables.map { $0.name }) + guard !tablesToToggle.isEmpty else { return } + + // Check if all tables are already pending delete - if so, remove them + let allAlreadyPending = tablesToToggle.allSatisfy { pendingDeletes.contains($0) } + if allAlreadyPending { + // Remove from pending + var updated = pendingDeletes + for name in tablesToToggle { + updated.remove(name) + tableOperationOptions.removeValue(forKey: name) + } + pendingDeletes = updated + } else { + // Show dialog to confirm operation + pendingOperationType = .delete + pendingOperationTables = tablesToToggle + showOperationDialog = true + } + } + + /// Confirm the pending operation with the given options + private func confirmOperation(options: TableOperationOptions) { + guard let operationType = pendingOperationType else { return } + var updatedTruncates = pendingTruncates - - let tablesToToggle = selectedTables.isEmpty ? [] : selectedTables - for table in tablesToToggle { - updatedTruncates.remove(table.name) - if updatedDeletes.contains(table.name) { - updatedDeletes.remove(table.name) + var updatedDeletes = pendingDeletes + var updatedOptions = tableOperationOptions + + for tableName in pendingOperationTables { + // Remove from opposite set if present + if operationType == .truncate { + updatedDeletes.remove(tableName) + updatedTruncates.insert(tableName) } else { - updatedDeletes.insert(table.name) + updatedTruncates.remove(tableName) + updatedDeletes.insert(tableName) } + + // Store options for this table + updatedOptions[tableName] = options } - + pendingTruncates = updatedTruncates pendingDeletes = updatedDeletes + tableOperationOptions = updatedOptions + + // Reset dialog state + pendingOperationType = nil + pendingOperationTables = [] } // MARK: - Actions @@ -415,7 +480,9 @@ struct TableRow: View { tables: .constant([]), selectedTables: .constant([]), pendingTruncates: .constant([]), - pendingDeletes: .constant([]) + pendingDeletes: .constant([]), + tableOperationOptions: .constant([:]), + databaseType: .mysql ) .frame(width: 250, height: 400) } diff --git a/TablePro/Views/Sidebar/TableOperationDialog.swift b/TablePro/Views/Sidebar/TableOperationDialog.swift new file mode 100644 index 000000000..296d09adf --- /dev/null +++ b/TablePro/Views/Sidebar/TableOperationDialog.swift @@ -0,0 +1,174 @@ +// +// TableOperationDialog.swift +// TablePro +// +// Confirmation dialog for table delete/truncate operations. +// Provides options for foreign key constraint handling and cascade operations. +// + +import SwiftUI + +/// Confirmation dialog for table delete/truncate operations +struct TableOperationDialog: View { + + // MARK: - Properties + + @Binding var isPresented: Bool + let tableName: String + let operationType: TableOperationType + let databaseType: DatabaseType + let onConfirm: (TableOperationOptions) -> Void + + // MARK: - State + + @State private var ignoreForeignKeys = false + @State private var cascade = false + + // MARK: - Computed Properties + + private var title: String { + switch operationType { + case .delete: + return "Delete table '\(tableName)'" + case .truncate: + return "Truncate table '\(tableName)'" + } + } + + private var cascadeSupported: Bool { + // SQLite doesn't support CASCADE + databaseType != .sqlite + } + + private var cascadeDescription: String { + switch operationType { + case .delete: + return "Delete all rows linked by foreign keys" + case .truncate: + if databaseType == .mysql || databaseType == .mariadb { + return "Not supported for TRUNCATE in MySQL/MariaDB" + } + return "Truncate all tables linked by foreign keys" + } + } + + private var cascadeDisabled: Bool { + // MySQL/MariaDB don't support CASCADE for TRUNCATE + if operationType == .truncate && (databaseType == .mysql || databaseType == .mariadb) { + return true + } + return !cascadeSupported + } + + // MARK: - Body + + var body: some View { + VStack(spacing: 0) { + // Header + Text(title) + .font(.system(size: 13, weight: .semibold)) + .padding(.vertical, 16) + .padding(.horizontal, 20) + + Divider() + + // Options + VStack(alignment: .leading, spacing: 16) { + // Ignore foreign key checks + Toggle(isOn: $ignoreForeignKeys) { + Text("Ignore foreign key checks") + .font(.system(size: 13)) + } + .toggleStyle(.checkbox) + + // Cascade option + VStack(alignment: .leading, spacing: 4) { + Toggle(isOn: $cascade) { + Text("Cascade") + .font(.system(size: 13)) + } + .toggleStyle(.checkbox) + .disabled(cascadeDisabled) + + Text(cascadeDescription) + .font(.system(size: 11)) + .foregroundStyle(.secondary) + .padding(.leading, 20) + } + .opacity(cascadeDisabled ? 0.5 : 1.0) + } + .padding(.horizontal, 20) + .padding(.vertical, 20) + + Divider() + + // Footer buttons + HStack { + Button("Cancel") { + isPresented = false + } + + Spacer() + + Button("OK") { + confirmAndDismiss() + } + .buttonStyle(.borderedProminent) + .keyboardShortcut(.return, modifiers: []) + } + .padding(12) + } + .frame(width: 320) + .background(Color(nsColor: .windowBackgroundColor)) + .onExitCommand { + isPresented = false + } + } + + private func confirmAndDismiss() { + let options = TableOperationOptions( + ignoreForeignKeys: ignoreForeignKeys, + cascade: cascadeDisabled ? false : cascade + ) + onConfirm(options) + isPresented = false + } +} + +// MARK: - Preview + +#Preview("Delete Table - MySQL") { + TableOperationDialog( + isPresented: .constant(true), + tableName: "users", + operationType: .delete, + databaseType: .mysql, + onConfirm: { options in + print("Options: \(options)") + } + ) +} + +#Preview("Truncate Table - PostgreSQL") { + TableOperationDialog( + isPresented: .constant(true), + tableName: "orders", + operationType: .truncate, + databaseType: .postgresql, + onConfirm: { options in + print("Options: \(options)") + } + ) +} + +#Preview("Delete Table - SQLite") { + TableOperationDialog( + isPresented: .constant(true), + tableName: "products", + operationType: .delete, + databaseType: .sqlite, + onConfirm: { options in + print("Options: \(options)") + } + ) +} From b9be15f461ec14f60170c3f6e790b2a3b116321f Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Fri, 26 Dec 2025 22:54:08 +0700 Subject: [PATCH 02/11] Fix table operation SQL generation issues - Wrap multiple operations in BEGIN/COMMIT transaction for atomicity - Sort tables alphabetically for consistent execution order - Use SET CONSTRAINTS ALL DEFERRED for PostgreSQL (no superuser required) - PostgreSQL constraints auto-check at COMMIT --- .../Views/Main/MainContentCoordinator.swift | 51 ++++++++++++++----- 1 file changed, 37 insertions(+), 14 deletions(-) diff --git a/TablePro/Views/Main/MainContentCoordinator.swift b/TablePro/Views/Main/MainContentCoordinator.swift index 19944cd9f..8acf53fc0 100644 --- a/TablePro/Views/Main/MainContentCoordinator.swift +++ b/TablePro/Views/Main/MainContentCoordinator.swift @@ -604,47 +604,70 @@ final class MainContentCoordinator: ObservableObject { var statements: [String] = [] let dbType = connection.type + // Sort tables for consistent execution order + let sortedTruncates = truncates.sorted() + let sortedDeletes = deletes.sorted() + // Check if any operation needs FK disabled let needsDisableFK = truncates.union(deletes).contains { tableName in options[tableName]?.ignoreForeignKeys == true } + // Wrap in transaction for atomicity (except SQLite which handles differently) + let needsTransaction = (sortedTruncates.count + sortedDeletes.count) > 1 && dbType != .sqlite + if needsTransaction { + statements.append("BEGIN") + } + if needsDisableFK { - statements.append(fkDisableStatement(for: dbType)) + statements.append(contentsOf: fkDisableStatements(for: dbType)) } - for tableName in truncates { + for tableName in sortedTruncates { let quotedName = dbType.quoteIdentifier(tableName) let opts = options[tableName] ?? TableOperationOptions() statements.append(truncateStatement(tableName: quotedName, options: opts, dbType: dbType)) } - for tableName in deletes { + for tableName in sortedDeletes { let quotedName = dbType.quoteIdentifier(tableName) let opts = options[tableName] ?? TableOperationOptions() statements.append(dropTableStatement(tableName: quotedName, options: opts, dbType: dbType)) } if needsDisableFK { - statements.append(fkEnableStatement(for: dbType)) + statements.append(contentsOf: fkEnableStatements(for: dbType)) + } + + if needsTransaction { + statements.append("COMMIT") } return statements } - private func fkDisableStatement(for dbType: DatabaseType) -> String { - return switch dbType { - case .mysql, .mariadb: "SET FOREIGN_KEY_CHECKS=0" - case .postgresql: "SET session_replication_role = 'replica'" - case .sqlite: "PRAGMA foreign_keys = OFF" + private func fkDisableStatements(for dbType: DatabaseType) -> [String] { + switch dbType { + case .mysql, .mariadb: + return ["SET FOREIGN_KEY_CHECKS=0"] + case .postgresql: + // SET CONSTRAINTS works within transaction for deferrable constraints + // For non-deferrable, CASCADE is the proper approach + return ["SET CONSTRAINTS ALL DEFERRED"] + case .sqlite: + return ["PRAGMA foreign_keys = OFF"] } } - private func fkEnableStatement(for dbType: DatabaseType) -> String { - return switch dbType { - case .mysql, .mariadb: "SET FOREIGN_KEY_CHECKS=1" - case .postgresql: "SET session_replication_role = 'origin'" - case .sqlite: "PRAGMA foreign_keys = ON" + private func fkEnableStatements(for dbType: DatabaseType) -> [String] { + switch dbType { + case .mysql, .mariadb: + return ["SET FOREIGN_KEY_CHECKS=1"] + case .postgresql: + // Constraints auto-check at COMMIT + return [] + case .sqlite: + return ["PRAGMA foreign_keys = ON"] } } From 750cbcf3a6dacaad19ff70c68879fecbba1222df Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Fri, 26 Dec 2025 23:15:38 +0700 Subject: [PATCH 03/11] Address code review feedback - Rename TableOperationType.delete to .drop for clarity - Reset dialog state (ignoreForeignKeys, cascade) on open - Restore pending operations on failure for retry capability - Include SQLite in transaction wrapping - Rename 'opts' variable to 'tableOptions' for readability - Add note when multiple tables selected (same options apply to all) - Use SET CONSTRAINTS ALL DEFERRED for PostgreSQL (no superuser required) --- TablePro/Models/TableOperationOptions.swift | 2 +- .../Views/Main/MainContentCoordinator.swift | 32 ++++++++++++++----- TablePro/Views/Sidebar/SidebarView.swift | 2 +- .../Views/Sidebar/TableOperationDialog.swift | 32 ++++++++++++++----- 4 files changed, 50 insertions(+), 18 deletions(-) diff --git a/TablePro/Models/TableOperationOptions.swift b/TablePro/Models/TableOperationOptions.swift index 2554562e5..0b6d51f41 100644 --- a/TablePro/Models/TableOperationOptions.swift +++ b/TablePro/Models/TableOperationOptions.swift @@ -17,5 +17,5 @@ struct TableOperationOptions: Codable, Equatable { /// Type of table operation enum TableOperationType: String, Codable { case truncate - case delete + case drop } diff --git a/TablePro/Views/Main/MainContentCoordinator.swift b/TablePro/Views/Main/MainContentCoordinator.swift index 8acf53fc0..b27d4d34e 100644 --- a/TablePro/Views/Main/MainContentCoordinator.swift +++ b/TablePro/Views/Main/MainContentCoordinator.swift @@ -613,8 +613,8 @@ final class MainContentCoordinator: ObservableObject { options[tableName]?.ignoreForeignKeys == true } - // Wrap in transaction for atomicity (except SQLite which handles differently) - let needsTransaction = (sortedTruncates.count + sortedDeletes.count) > 1 && dbType != .sqlite + // Wrap in transaction for atomicity + let needsTransaction = (sortedTruncates.count + sortedDeletes.count) > 1 if needsTransaction { statements.append("BEGIN") } @@ -625,14 +625,14 @@ final class MainContentCoordinator: ObservableObject { for tableName in sortedTruncates { let quotedName = dbType.quoteIdentifier(tableName) - let opts = options[tableName] ?? TableOperationOptions() - statements.append(truncateStatement(tableName: quotedName, options: opts, dbType: dbType)) + let tableOptions = options[tableName] ?? TableOperationOptions() + statements.append(truncateStatement(tableName: quotedName, options: tableOptions, dbType: dbType)) } for tableName in sortedDeletes { let quotedName = dbType.quoteIdentifier(tableName) - let opts = options[tableName] ?? TableOperationOptions() - statements.append(dropTableStatement(tableName: quotedName, options: opts, dbType: dbType)) + let tableOptions = options[tableName] ?? TableOperationOptions() + statements.append(dropTableStatement(tableName: quotedName, options: tableOptions, dbType: dbType)) } if needsDisableFK { @@ -700,11 +700,16 @@ final class MainContentCoordinator: ObservableObject { let truncatedTables = Set(pendingTruncates) let conn = connection - // Clear operations immediately (before async execution) + // Capture options before clearing (for potential restore on failure) + var capturedOptions: [String: TableOperationOptions] = [:] + for table in deletedTables.union(truncatedTables) { + capturedOptions[table] = tableOperationOptions[table] + } + + // Clear operations immediately (to prevent double-execution) if clearTableOps { pendingTruncates.removeAll() pendingDeletes.removeAll() - // Clear options for processed tables for table in deletedTables.union(truncatedTables) { tableOperationOptions.removeValue(forKey: table) } @@ -791,6 +796,17 @@ final class MainContentCoordinator: ObservableObject { if let index = tabManager.selectedTabIndex { tabManager.tabs[index].errorMessage = "Save failed: \(error.localizedDescription)" } + + // Restore operations on failure so user can retry + if clearTableOps { + DatabaseManager.shared.updateSession(conn.id) { session in + session.pendingTruncates = truncatedTables + session.pendingDeletes = deletedTables + for (table, opts) in capturedOptions { + session.tableOperationOptions[table] = opts + } + } + } } } } diff --git a/TablePro/Views/Sidebar/SidebarView.swift b/TablePro/Views/Sidebar/SidebarView.swift index 273abcf3c..d36feef0b 100644 --- a/TablePro/Views/Sidebar/SidebarView.swift +++ b/TablePro/Views/Sidebar/SidebarView.swift @@ -321,7 +321,7 @@ struct SidebarView: View { pendingDeletes = updated } else { // Show dialog to confirm operation - pendingOperationType = .delete + pendingOperationType = .drop pendingOperationTables = tablesToToggle showOperationDialog = true } diff --git a/TablePro/Views/Sidebar/TableOperationDialog.swift b/TablePro/Views/Sidebar/TableOperationDialog.swift index 296d09adf..25510d3b2 100644 --- a/TablePro/Views/Sidebar/TableOperationDialog.swift +++ b/TablePro/Views/Sidebar/TableOperationDialog.swift @@ -28,8 +28,8 @@ struct TableOperationDialog: View { private var title: String { switch operationType { - case .delete: - return "Delete table '\(tableName)'" + case .drop: + return "Drop table '\(tableName)'" case .truncate: return "Truncate table '\(tableName)'" } @@ -40,10 +40,14 @@ struct TableOperationDialog: View { databaseType != .sqlite } + private var isMultipleTables: Bool { + tableName.contains("tables") + } + private var cascadeDescription: String { switch operationType { - case .delete: - return "Delete all rows linked by foreign keys" + case .drop: + return "Drop all tables that depend on this table" case .truncate: if databaseType == .mysql || databaseType == .mariadb { return "Not supported for TRUNCATE in MySQL/MariaDB" @@ -74,6 +78,13 @@ struct TableOperationDialog: View { // Options VStack(alignment: .leading, spacing: 16) { + // Note for multiple tables + if isMultipleTables { + Text("Same options will be applied to all selected tables.") + .font(.system(size: 11)) + .foregroundStyle(.secondary) + } + // Ignore foreign key checks Toggle(isOn: $ignoreForeignKeys) { Text("Ignore foreign key checks") @@ -120,6 +131,11 @@ struct TableOperationDialog: View { } .frame(width: 320) .background(Color(nsColor: .windowBackgroundColor)) + .onAppear { + // Reset state when dialog opens + ignoreForeignKeys = false + cascade = false + } .onExitCommand { isPresented = false } @@ -137,11 +153,11 @@ struct TableOperationDialog: View { // MARK: - Preview -#Preview("Delete Table - MySQL") { +#Preview("Drop Table - MySQL") { TableOperationDialog( isPresented: .constant(true), tableName: "users", - operationType: .delete, + operationType: .drop, databaseType: .mysql, onConfirm: { options in print("Options: \(options)") @@ -161,11 +177,11 @@ struct TableOperationDialog: View { ) } -#Preview("Delete Table - SQLite") { +#Preview("Drop Table - SQLite") { TableOperationDialog( isPresented: .constant(true), tableName: "products", - operationType: .delete, + operationType: .drop, databaseType: .sqlite, onConfirm: { options in print("Options: \(options)") From 93e681ede1ae2f63d9b2fc6c4ad6e53035b9706f Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Fri, 26 Dec 2025 23:31:25 +0700 Subject: [PATCH 04/11] Address Copilot review feedback (round 3) - Fix PostgreSQL FK handling: return empty array for FK statements (PostgreSQL doesn't have a simple way to disable FK checks globally, rely on CASCADE option instead) - Add FK re-enable on error in catch block for MySQL/SQLite - Add sqlite_sequence reset for true TRUNCATE semantics (reset auto-increment) - Add docstrings to helper functions for better code documentation - Fix nested transaction issue when both cell changes and table ops exist (wrap in single transaction only when multiple operation types) --- .../Views/Main/MainContentCoordinator.swift | 78 +++++++++++++++---- 1 file changed, 62 insertions(+), 16 deletions(-) diff --git a/TablePro/Views/Main/MainContentCoordinator.swift b/TablePro/Views/Main/MainContentCoordinator.swift index b27d4d34e..1b18dbd82 100644 --- a/TablePro/Views/Main/MainContentCoordinator.swift +++ b/TablePro/Views/Main/MainContentCoordinator.swift @@ -564,20 +564,32 @@ final class MainContentCoordinator: ObservableObject { var allStatements: [String] = [] + // Wrap all operations in a single transaction when we have multiple operations + let needsTransaction = hasEditedCells && hasPendingTableOps + if needsTransaction { + allStatements.append("BEGIN") + } + if hasEditedCells { allStatements.append(contentsOf: changeManager.generateSQL()) } if hasPendingTableOps { // Generate table operation SQL with FK/cascade options + // Don't wrap in transaction if we're already in one let tableOpStatements = generateTableOperationSQL( truncates: pendingTruncates, deletes: pendingDeletes, - options: tableOperationOptions + options: tableOperationOptions, + wrapInTransaction: !needsTransaction ) allStatements.append(contentsOf: tableOpStatements) } + if needsTransaction { + allStatements.append("COMMIT") + } + guard !allStatements.isEmpty else { if let index = tabManager.selectedTabIndex { tabManager.tabs[index].errorMessage = "Could not generate SQL for changes." @@ -595,11 +607,18 @@ final class MainContentCoordinator: ObservableObject { ) } - /// Generate SQL for table truncate/delete operations with FK/cascade options + /// Generates SQL statements for table truncate/drop operations with FK handling. + /// - Parameters: + /// - 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 + /// - Returns: Array of SQL statements to execute private func generateTableOperationSQL( truncates: Set, deletes: Set, - options: [String: TableOperationOptions] + options: [String: TableOperationOptions], + wrapInTransaction: Bool = true ) -> [String] { var statements: [String] = [] let dbType = connection.type @@ -608,13 +627,13 @@ final class MainContentCoordinator: ObservableObject { let sortedTruncates = truncates.sorted() let sortedDeletes = deletes.sorted() - // Check if any operation needs FK disabled - let needsDisableFK = truncates.union(deletes).contains { tableName in + // Check if any operation needs FK disabled (not applicable to PostgreSQL) + let needsDisableFK = dbType != .postgresql && truncates.union(deletes).contains { tableName in options[tableName]?.ignoreForeignKeys == true } // Wrap in transaction for atomicity - let needsTransaction = (sortedTruncates.count + sortedDeletes.count) > 1 + let needsTransaction = wrapInTransaction && (sortedTruncates.count + sortedDeletes.count) > 1 if needsTransaction { statements.append("BEGIN") } @@ -626,7 +645,7 @@ final class MainContentCoordinator: ObservableObject { for tableName in sortedTruncates { let quotedName = dbType.quoteIdentifier(tableName) let tableOptions = options[tableName] ?? TableOperationOptions() - statements.append(truncateStatement(tableName: quotedName, options: tableOptions, dbType: dbType)) + statements.append(contentsOf: truncateStatements(tableName: tableName, quotedName: quotedName, options: tableOptions, dbType: dbType)) } for tableName in sortedDeletes { @@ -646,39 +665,53 @@ final class MainContentCoordinator: ObservableObject { return statements } + /// Returns SQL statements to disable foreign key checks for the database type. + /// - Note: PostgreSQL doesn't support globally disabling FK checks; use CASCADE instead. private func fkDisableStatements(for dbType: DatabaseType) -> [String] { switch dbType { case .mysql, .mariadb: return ["SET FOREIGN_KEY_CHECKS=0"] case .postgresql: - // SET CONSTRAINTS works within transaction for deferrable constraints - // For non-deferrable, CASCADE is the proper approach - return ["SET CONSTRAINTS ALL DEFERRED"] + // PostgreSQL doesn't support globally disabling non-deferrable FKs. + // Use CASCADE option for reliable FK handling. + return [] case .sqlite: return ["PRAGMA foreign_keys = OFF"] } } + /// Returns SQL statements to re-enable foreign key checks for the database type. private func fkEnableStatements(for dbType: DatabaseType) -> [String] { switch dbType { case .mysql, .mariadb: return ["SET FOREIGN_KEY_CHECKS=1"] case .postgresql: - // Constraints auto-check at COMMIT return [] case .sqlite: return ["PRAGMA foreign_keys = ON"] } } - private func truncateStatement(tableName: String, options: TableOperationOptions, dbType: DatabaseType) -> String { - return switch dbType { - case .mysql, .mariadb: "TRUNCATE TABLE \(tableName)" - case .postgresql: options.cascade ? "TRUNCATE TABLE \(tableName) CASCADE" : "TRUNCATE TABLE \(tableName)" - case .sqlite: "DELETE FROM \(tableName)" + /// Generates TRUNCATE/DELETE statements for a table. + /// - Note: SQLite uses DELETE and resets auto-increment via sqlite_sequence. + private func truncateStatements(tableName: String, quotedName: String, options: TableOperationOptions, dbType: DatabaseType) -> [String] { + switch dbType { + case .mysql, .mariadb: + return ["TRUNCATE TABLE \(quotedName)"] + case .postgresql: + let cascade = options.cascade ? " CASCADE" : "" + return ["TRUNCATE TABLE \(quotedName)\(cascade)"] + case .sqlite: + // DELETE FROM + reset auto-increment counter for true TRUNCATE semantics + let escapedName = tableName.replacingOccurrences(of: "'", with: "''") + return [ + "DELETE FROM \(quotedName)", + "DELETE FROM sqlite_sequence WHERE name = '\(escapedName)'" + ] } } + /// Generates DROP TABLE statement with optional CASCADE. private func dropTableStatement(tableName: String, options: TableOperationOptions, dbType: DatabaseType) -> String { let cascade = options.cascade ? " CASCADE" : "" return switch dbType { @@ -699,6 +732,12 @@ final class MainContentCoordinator: ObservableObject { let deletedTables = Set(pendingDeletes) let truncatedTables = Set(pendingTruncates) let conn = connection + let dbType = connection.type + + // Track if FK checks were disabled (need to re-enable on failure) + let fkWasDisabled = dbType != .postgresql && deletedTables.union(truncatedTables).contains { tableName in + tableOperationOptions[tableName]?.ignoreForeignKeys == true + } // Capture options before clearing (for potential restore on failure) var capturedOptions: [String: TableOperationOptions] = [:] @@ -782,6 +821,13 @@ final class MainContentCoordinator: ObservableObject { } catch { let executionTime = Date().timeIntervalSince(overallStartTime) + // Try to re-enable FK checks if they were disabled + if fkWasDisabled, let driver = DatabaseManager.shared.activeDriver { + for statement in self.fkEnableStatements(for: dbType) { + try? await driver.execute(query: statement) + } + } + await MainActor.run { QueryHistoryManager.shared.recordQuery( query: sql, From 764a4a062798c6282ddb1e16cde276d9a762ded4 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Fri, 26 Dec 2025 23:45:07 +0700 Subject: [PATCH 05/11] Address Copilot review feedback (round 4) - Fix SQL injection: escape embedded quotes in quoteIdentifier by doubling them (SQL standard escaping) - Move FK disable/enable statements outside transaction boundary to ensure they take effect even on rollback - Rename dropTableStatement parameter from tableName to quotedName for consistency with truncateStatements - Disable "Ignore foreign key checks" option for PostgreSQL with explanation text (use CASCADE instead) - Add onChange handlers to reset cascade/ignoreForeignKeys when their toggles become disabled, preventing silent override of user selection - Simplify confirmAndDismiss since values are now reset when disabled --- TablePro/Models/DatabaseConnection.swift | 7 ++- .../Views/Main/MainContentCoordinator.swift | 26 +++++----- .../Views/Sidebar/TableOperationDialog.swift | 49 ++++++++++++++++--- 3 files changed, 62 insertions(+), 20 deletions(-) diff --git a/TablePro/Models/DatabaseConnection.swift b/TablePro/Models/DatabaseConnection.swift index d084eff30..8e52ca86c 100644 --- a/TablePro/Models/DatabaseConnection.swift +++ b/TablePro/Models/DatabaseConnection.swift @@ -92,10 +92,13 @@ enum DatabaseType: String, CaseIterable, Identifiable, Codable { } } - /// Quote an identifier (table or column name) for this database type + /// Quote an identifier (table or column name) for this database type. + /// Escapes embedded quote characters to prevent SQL injection. func quoteIdentifier(_ name: String) -> String { let q = identifierQuote - return "\(q)\(name)\(q)" + // Escape embedded quotes by doubling them (SQL standard) + let escaped = name.replacingOccurrences(of: q, with: q + q) + return "\(q)\(escaped)\(q)" } } diff --git a/TablePro/Views/Main/MainContentCoordinator.swift b/TablePro/Views/Main/MainContentCoordinator.swift index 1b18dbd82..5ec4c92ae 100644 --- a/TablePro/Views/Main/MainContentCoordinator.swift +++ b/TablePro/Views/Main/MainContentCoordinator.swift @@ -632,16 +632,17 @@ final class MainContentCoordinator: ObservableObject { options[tableName]?.ignoreForeignKeys == true } + // FK disable must be OUTSIDE transaction to ensure it takes effect even on rollback + if needsDisableFK { + statements.append(contentsOf: fkDisableStatements(for: dbType)) + } + // Wrap in transaction for atomicity let needsTransaction = wrapInTransaction && (sortedTruncates.count + sortedDeletes.count) > 1 if needsTransaction { statements.append("BEGIN") } - if needsDisableFK { - statements.append(contentsOf: fkDisableStatements(for: dbType)) - } - for tableName in sortedTruncates { let quotedName = dbType.quoteIdentifier(tableName) let tableOptions = options[tableName] ?? TableOperationOptions() @@ -651,17 +652,18 @@ final class MainContentCoordinator: ObservableObject { for tableName in sortedDeletes { let quotedName = dbType.quoteIdentifier(tableName) let tableOptions = options[tableName] ?? TableOperationOptions() - statements.append(dropTableStatement(tableName: quotedName, options: tableOptions, dbType: dbType)) - } - - if needsDisableFK { - statements.append(contentsOf: fkEnableStatements(for: dbType)) + statements.append(dropTableStatement(quotedName: quotedName, 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)) + } + return statements } @@ -712,11 +714,11 @@ final class MainContentCoordinator: ObservableObject { } /// Generates DROP TABLE statement with optional CASCADE. - private func dropTableStatement(tableName: String, options: TableOperationOptions, dbType: DatabaseType) -> String { + private func dropTableStatement(quotedName: String, options: TableOperationOptions, dbType: DatabaseType) -> String { let cascade = options.cascade ? " CASCADE" : "" return switch dbType { - case .mysql, .mariadb, .postgresql: "DROP TABLE \(tableName)\(cascade)" - case .sqlite: "DROP TABLE \(tableName)" + case .mysql, .mariadb, .postgresql: "DROP TABLE \(quotedName)\(cascade)" + case .sqlite: "DROP TABLE \(quotedName)" } } diff --git a/TablePro/Views/Sidebar/TableOperationDialog.swift b/TablePro/Views/Sidebar/TableOperationDialog.swift index 25510d3b2..bd992b1bf 100644 --- a/TablePro/Views/Sidebar/TableOperationDialog.swift +++ b/TablePro/Views/Sidebar/TableOperationDialog.swift @@ -64,6 +64,18 @@ struct TableOperationDialog: View { return !cascadeSupported } + /// PostgreSQL doesn't support globally disabling FK checks; use CASCADE instead + private var ignoreFKDisabled: Bool { + databaseType == .postgresql + } + + private var ignoreFKDescription: String? { + if databaseType == .postgresql { + return "Not supported for PostgreSQL. Use CASCADE instead." + } + return nil + } + // MARK: - Body var body: some View { @@ -86,11 +98,22 @@ struct TableOperationDialog: View { } // Ignore foreign key checks - Toggle(isOn: $ignoreForeignKeys) { - Text("Ignore foreign key checks") - .font(.system(size: 13)) + VStack(alignment: .leading, spacing: 4) { + Toggle(isOn: $ignoreForeignKeys) { + Text("Ignore foreign key checks") + .font(.system(size: 13)) + } + .toggleStyle(.checkbox) + .disabled(ignoreFKDisabled) + + if let description = ignoreFKDescription { + Text(description) + .font(.system(size: 11)) + .foregroundStyle(.secondary) + .padding(.leading, 20) + } } - .toggleStyle(.checkbox) + .opacity(ignoreFKDisabled ? 0.5 : 1.0) // Cascade option VStack(alignment: .leading, spacing: 4) { @@ -132,19 +155,33 @@ struct TableOperationDialog: View { .frame(width: 320) .background(Color(nsColor: .windowBackgroundColor)) .onAppear { - // Reset state when dialog opens + // Reset state when dialog opens, respecting disabled states ignoreForeignKeys = false cascade = false } + .onChange(of: cascadeDisabled) { _, isDisabled in + // Reset cascade when it becomes disabled to avoid silent override + if isDisabled { + cascade = false + } + } + .onChange(of: ignoreFKDisabled) { _, isDisabled in + // Reset ignoreForeignKeys when it becomes disabled + if isDisabled { + ignoreForeignKeys = false + } + } .onExitCommand { isPresented = false } } private func confirmAndDismiss() { + // Values are already reset when their toggles become disabled, + // so we can pass them directly without override checks let options = TableOperationOptions( ignoreForeignKeys: ignoreForeignKeys, - cascade: cascadeDisabled ? false : cascade + cascade: cascade ) onConfirm(options) isPresented = false From 3b2cce7b465a495af6d2444452516c0d20267c28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ng=C3=B4=20Qu=E1=BB=91c=20=C4=90=E1=BA=A1t?= Date: Sat, 27 Dec 2025 10:41:52 +0700 Subject: [PATCH 06/11] Update TablePro/Views/Sidebar/TableOperationDialog.swift Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- TablePro/Views/Sidebar/TableOperationDialog.swift | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/TablePro/Views/Sidebar/TableOperationDialog.swift b/TablePro/Views/Sidebar/TableOperationDialog.swift index bd992b1bf..b68a4ae82 100644 --- a/TablePro/Views/Sidebar/TableOperationDialog.swift +++ b/TablePro/Views/Sidebar/TableOperationDialog.swift @@ -155,22 +155,10 @@ struct TableOperationDialog: View { .frame(width: 320) .background(Color(nsColor: .windowBackgroundColor)) .onAppear { - // Reset state when dialog opens, respecting disabled states + // Reset state when dialog opens ignoreForeignKeys = false cascade = false } - .onChange(of: cascadeDisabled) { _, isDisabled in - // Reset cascade when it becomes disabled to avoid silent override - if isDisabled { - cascade = false - } - } - .onChange(of: ignoreFKDisabled) { _, isDisabled in - // Reset ignoreForeignKeys when it becomes disabled - if isDisabled { - ignoreForeignKeys = false - } - } .onExitCommand { isPresented = false } From 197e54ce871875c453a3d2fb17e78f5e1621e10f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ng=C3=B4=20Qu=E1=BB=91c=20=C4=90=E1=BA=A1t?= Date: Sat, 27 Dec 2025 10:42:01 +0700 Subject: [PATCH 07/11] Update TablePro/Views/Main/MainContentCoordinator.swift Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- TablePro/Views/Main/MainContentCoordinator.swift | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/TablePro/Views/Main/MainContentCoordinator.swift b/TablePro/Views/Main/MainContentCoordinator.swift index 5ec4c92ae..8e1b0553b 100644 --- a/TablePro/Views/Main/MainContentCoordinator.swift +++ b/TablePro/Views/Main/MainContentCoordinator.swift @@ -826,7 +826,12 @@ final class MainContentCoordinator: ObservableObject { // Try to re-enable FK checks if they were disabled if fkWasDisabled, let driver = DatabaseManager.shared.activeDriver { for statement in self.fkEnableStatements(for: dbType) { - try? await driver.execute(query: statement) + do { + try await driver.execute(query: statement) + } catch { + // Log failure to re-enable FK checks so it is not silently ignored + print("Warning: Failed to re-enable foreign key checks with statement '\(statement)': \(error)") + } } } From fa7ab2b02b7783ae7467e39bb0607d3e9d174601 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ng=C3=B4=20Qu=E1=BB=91c=20=C4=90=E1=BA=A1t?= Date: Sat, 27 Dec 2025 10:42:15 +0700 Subject: [PATCH 08/11] Update TablePro/Views/Main/MainContentCoordinator.swift Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- TablePro/Views/Main/MainContentCoordinator.swift | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/TablePro/Views/Main/MainContentCoordinator.swift b/TablePro/Views/Main/MainContentCoordinator.swift index 8e1b0553b..42f256f9f 100644 --- a/TablePro/Views/Main/MainContentCoordinator.swift +++ b/TablePro/Views/Main/MainContentCoordinator.swift @@ -715,10 +715,11 @@ final class MainContentCoordinator: ObservableObject { /// Generates DROP TABLE statement with optional CASCADE. private func dropTableStatement(quotedName: String, options: TableOperationOptions, dbType: DatabaseType) -> String { - let cascade = options.cascade ? " CASCADE" : "" return switch dbType { - case .mysql, .mariadb, .postgresql: "DROP TABLE \(quotedName)\(cascade)" - case .sqlite: "DROP TABLE \(quotedName)" + case .postgresql: + "DROP TABLE \(quotedName)\(options.cascade ? " CASCADE" : "")" + case .mysql, .mariadb, .sqlite: + "DROP TABLE \(quotedName)" } } From 6d4155b5fd55effdeae009a12ef5f163407362fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ng=C3=B4=20Qu=E1=BB=91c=20=C4=90=E1=BA=A1t?= Date: Sat, 27 Dec 2025 10:42:28 +0700 Subject: [PATCH 09/11] Update TablePro/Views/Sidebar/TableOperationDialog.swift Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- TablePro/Views/Sidebar/TableOperationDialog.swift | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/TablePro/Views/Sidebar/TableOperationDialog.swift b/TablePro/Views/Sidebar/TableOperationDialog.swift index b68a4ae82..6a25c6007 100644 --- a/TablePro/Views/Sidebar/TableOperationDialog.swift +++ b/TablePro/Views/Sidebar/TableOperationDialog.swift @@ -36,8 +36,14 @@ struct TableOperationDialog: View { } private var cascadeSupported: Bool { - // SQLite doesn't support CASCADE - databaseType != .sqlite + // PostgreSQL supports CASCADE for both DROP and TRUNCATE. + // MySQL, MariaDB, and SQLite do not support CASCADE for these operations. + switch databaseType { + case .postgresql: + return true + default: + return false + } } private var isMultipleTables: Bool { From c20e7f42a5219f7327df3bae33c2173235be5c77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ng=C3=B4=20Qu=E1=BB=91c=20=C4=90=E1=BA=A1t?= Date: Sat, 27 Dec 2025 10:43:09 +0700 Subject: [PATCH 10/11] Update TablePro/Views/Sidebar/SidebarView.swift Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- TablePro/Views/Sidebar/SidebarView.swift | 29 +++++++++++++----------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/TablePro/Views/Sidebar/SidebarView.swift b/TablePro/Views/Sidebar/SidebarView.swift index d36feef0b..52c85111f 100644 --- a/TablePro/Views/Sidebar/SidebarView.swift +++ b/TablePro/Views/Sidebar/SidebarView.swift @@ -104,19 +104,22 @@ struct SidebarView: View { } } .sheet(isPresented: $showOperationDialog) { - if let operationType = pendingOperationType, - let firstTable = pendingOperationTables.first { - TableOperationDialog( - isPresented: $showOperationDialog, - tableName: pendingOperationTables.count > 1 - ? "\(pendingOperationTables.count) tables" - : firstTable, - operationType: operationType, - databaseType: databaseType, - onConfirm: { options in - confirmOperation(options: options) - } - ) + if let operationType = pendingOperationType { + let tables = pendingOperationTables + if let firstTable = tables.first { + let tableName = tables.count > 1 + ? "\(tables.count) tables" + : firstTable + TableOperationDialog( + isPresented: $showOperationDialog, + tableName: tableName, + operationType: operationType, + databaseType: databaseType, + onConfirm: { options in + confirmOperation(options: options) + } + ) + } } } .onChange(of: showOperationDialog) { _, isPresented in From 1bd106cf6730e7ce82ddda775315a0c5bd05f530 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Sat, 27 Dec 2025 10:52:37 +0700 Subject: [PATCH 11/11] wip --- .../Views/Main/MainContentCoordinator.swift | 89 ++++++++++++++----- TablePro/Views/Sidebar/SidebarView.swift | 10 ++- .../Views/Sidebar/TableOperationDialog.swift | 5 +- 3 files changed, 77 insertions(+), 27 deletions(-) diff --git a/TablePro/Views/Main/MainContentCoordinator.swift b/TablePro/Views/Main/MainContentCoordinator.swift index 42f256f9f..1cc35a321 100644 --- a/TablePro/Views/Main/MainContentCoordinator.swift +++ b/TablePro/Views/Main/MainContentCoordinator.swift @@ -563,6 +563,17 @@ final class MainContentCoordinator: ObservableObject { guard hasEditedCells || hasPendingTableOps else { return } var allStatements: [String] = [] + let dbType = connection.type + + // Check if any table operation needs FK disabled (must be outside transaction) + let needsDisableFK = dbType != .postgresql && pendingTruncates.union(pendingDeletes).contains { tableName in + tableOperationOptions[tableName]?.ignoreForeignKeys == true + } + + // FK disable must be FIRST, before any transaction begins + if needsDisableFK { + allStatements.append(contentsOf: fkDisableStatements(for: dbType)) + } // Wrap all operations in a single transaction when we have multiple operations let needsTransaction = hasEditedCells && hasPendingTableOps @@ -571,17 +582,18 @@ final class MainContentCoordinator: ObservableObject { } if hasEditedCells { + // changeManager.generateSQL() does NOT include transaction statements allStatements.append(contentsOf: changeManager.generateSQL()) } if hasPendingTableOps { - // Generate table operation SQL with FK/cascade options - // Don't wrap in transaction if we're already in one + // Generate table operation SQL WITHOUT FK handling (already done above) let tableOpStatements = generateTableOperationSQL( truncates: pendingTruncates, deletes: pendingDeletes, options: tableOperationOptions, - wrapInTransaction: !needsTransaction + wrapInTransaction: !needsTransaction, + includeFKHandling: false // FK handling done at this level ) allStatements.append(contentsOf: tableOpStatements) } @@ -590,6 +602,11 @@ final class MainContentCoordinator: ObservableObject { allStatements.append("COMMIT") } + // FK re-enable must be LAST, after transaction commits + if needsDisableFK { + allStatements.append(contentsOf: fkEnableStatements(for: dbType)) + } + guard !allStatements.isEmpty else { if let index = tabManager.selectedTabIndex { tabManager.tabs[index].errorMessage = "Could not generate SQL for changes." @@ -597,9 +614,9 @@ final class MainContentCoordinator: ObservableObject { return } - let sql = allStatements.joined(separator: ";\n") - executeCommitSQL( - sql, + // Pass statements as array to avoid SQL injection via semicolon splitting + executeCommitStatements( + allStatements, clearTableOps: hasPendingTableOps, pendingTruncates: &pendingTruncates, pendingDeletes: &pendingDeletes, @@ -607,18 +624,20 @@ final class MainContentCoordinator: ObservableObject { ) } - /// Generates SQL statements for table truncate/drop operations with FK handling. + /// Generates SQL statements for table truncate/drop operations. /// - Parameters: /// - 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 private func generateTableOperationSQL( truncates: Set, deletes: Set, options: [String: TableOperationOptions], - wrapInTransaction: Bool = true + wrapInTransaction: Bool = true, + includeFKHandling: Bool = true ) -> [String] { var statements: [String] = [] let dbType = connection.type @@ -628,7 +647,7 @@ final class MainContentCoordinator: ObservableObject { let sortedDeletes = deletes.sorted() // Check if any operation needs FK disabled (not applicable to PostgreSQL) - let needsDisableFK = dbType != .postgresql && truncates.union(deletes).contains { tableName in + let needsDisableFK = includeFKHandling && dbType != .postgresql && truncates.union(deletes).contains { tableName in options[tableName]?.ignoreForeignKeys == true } @@ -704,10 +723,16 @@ final class MainContentCoordinator: ObservableObject { let cascade = options.cascade ? " CASCADE" : "" return ["TRUNCATE TABLE \(quotedName)\(cascade)"] case .sqlite: - // DELETE FROM + reset auto-increment counter for true TRUNCATE semantics + // DELETE FROM + reset auto-increment counter for true TRUNCATE semantics. + // Note: quotedName uses backticks (via quoteIdentifier) for SQL identifiers, + // while escapedName uses single-quote escaping for string literals in the + // sqlite_sequence query. These are different SQL quoting mechanisms for + // different purposes (identifier vs string literal). let escapedName = tableName.replacingOccurrences(of: "'", with: "''") return [ "DELETE FROM \(quotedName)", + // sqlite_sequence may not exist if no table has AUTOINCREMENT. + // This DELETE will succeed silently if the table isn't in sqlite_sequence. "DELETE FROM sqlite_sequence WHERE name = '\(escapedName)'" ] } @@ -723,14 +748,23 @@ final class MainContentCoordinator: ObservableObject { } } - private func executeCommitSQL( - _ sql: String, + /// Executes an array of SQL statements sequentially. + /// This approach prevents SQL injection by avoiding semicolon-based string splitting. + /// - Parameters: + /// - statements: Pre-segmented array of SQL statements to execute + /// - clearTableOps: Whether to clear pending table operations on success + /// - pendingTruncates: Inout binding to pending truncate operations (restored on failure) + /// - pendingDeletes: Inout binding to pending delete operations (restored on failure) + /// - tableOperationOptions: Inout binding to operation options (restored on failure) + private func executeCommitStatements( + _ statements: [String], clearTableOps: Bool, pendingTruncates: inout Set, pendingDeletes: inout Set, tableOperationOptions: inout [String: TableOperationOptions] ) { - guard !sql.isEmpty else { return } + let validStatements = statements.filter { !$0.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty } + guard !validStatements.isEmpty else { return } let deletedTables = Set(pendingDeletes) let truncatedTables = Set(pendingTruncates) @@ -749,6 +783,7 @@ final class MainContentCoordinator: ObservableObject { } // Clear operations immediately (to prevent double-execution) + // Store references to restore synchronously on failure if clearTableOps { pendingTruncates.removeAll() pendingDeletes.removeAll() @@ -757,6 +792,10 @@ final class MainContentCoordinator: ObservableObject { } } + // Capture inout references for async restoration via notification + // This avoids the race condition of async updateSession + let restoreNotificationName = Notification.Name("RestorePendingTableOperations_\(conn.id)") + Task { let overallStartTime = Date() @@ -770,11 +809,7 @@ final class MainContentCoordinator: ObservableObject { throw DatabaseError.notConnected } - let statements = sql.components(separatedBy: ";").filter { - !$0.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty - } - - for statement in statements { + for statement in validStatements { let statementStartTime = Date() _ = try await driver.execute(query: statement) let executionTime = Date().timeIntervalSince(statementStartTime) @@ -830,15 +865,15 @@ final class MainContentCoordinator: ObservableObject { do { try await driver.execute(query: statement) } catch { - // Log failure to re-enable FK checks so it is not silently ignored print("Warning: Failed to re-enable foreign key checks with statement '\(statement)': \(error)") } } } await MainActor.run { + let allSQL = validStatements.joined(separator: "; ") QueryHistoryManager.shared.recordQuery( - query: sql, + query: allSQL, connectionId: conn.id, databaseName: conn.database ?? "", executionTime: executionTime, @@ -851,8 +886,20 @@ final class MainContentCoordinator: ObservableObject { tabManager.tabs[index].errorMessage = "Save failed: \(error.localizedDescription)" } - // Restore operations on failure so user can retry + // Restore operations on failure so user can retry. + // Use notification to restore via MainContentView's bindings for synchronous update. if clearTableOps { + NotificationCenter.default.post( + name: restoreNotificationName, + object: nil, + userInfo: [ + "truncates": truncatedTables, + "deletes": deletedTables, + "options": capturedOptions + ] + ) + + // Also update session for persistence DatabaseManager.shared.updateSession(conn.id) { session in session.pendingTruncates = truncatedTables session.pendingDeletes = deletedTables diff --git a/TablePro/Views/Sidebar/SidebarView.swift b/TablePro/Views/Sidebar/SidebarView.swift index 52c85111f..72040f787 100644 --- a/TablePro/Views/Sidebar/SidebarView.swift +++ b/TablePro/Views/Sidebar/SidebarView.swift @@ -290,9 +290,10 @@ struct SidebarView: View { guard !tablesToToggle.isEmpty else { return } // Check if all tables are already pending truncate - if so, remove them + // Cancellation doesn't require confirmation since it's a safe operation that + // simply removes the pending state. The stored options are intentionally discarded. let allAlreadyPending = tablesToToggle.allSatisfy { pendingTruncates.contains($0) } if allAlreadyPending { - // Remove from pending var updated = pendingTruncates for name in tablesToToggle { updated.remove(name) @@ -313,9 +314,10 @@ struct SidebarView: View { guard !tablesToToggle.isEmpty else { return } // Check if all tables are already pending delete - if so, remove them + // Cancellation doesn't require confirmation since it's a safe operation that + // simply removes the pending state. The stored options are intentionally discarded. let allAlreadyPending = tablesToToggle.allSatisfy { pendingDeletes.contains($0) } if allAlreadyPending { - // Remove from pending var updated = pendingDeletes for name in tablesToToggle { updated.remove(name) @@ -439,7 +441,7 @@ struct TableRow: View { ZStack(alignment: .bottomTrailing) { Image(systemName: table.type == .view ? "eye" : "tablecells") .foregroundStyle(iconColor) - .frame(width: 20) + .frame(width: 14) // Pending operation indicator if isPendingDelete { @@ -456,7 +458,7 @@ struct TableRow: View { } Text(table.name) - .font(.system(.body, design: .monospaced)) + .font(.system(size: 12, design: .monospaced)) .lineLimit(1) .foregroundStyle(textColor) } diff --git a/TablePro/Views/Sidebar/TableOperationDialog.swift b/TablePro/Views/Sidebar/TableOperationDialog.swift index 6a25c6007..f0d25458e 100644 --- a/TablePro/Views/Sidebar/TableOperationDialog.swift +++ b/TablePro/Views/Sidebar/TableOperationDialog.swift @@ -119,7 +119,7 @@ struct TableOperationDialog: View { .padding(.leading, 20) } } - .opacity(ignoreFKDisabled ? 0.5 : 1.0) + .opacity(ignoreFKDisabled ? 0.6 : 1.0) // Cascade option VStack(alignment: .leading, spacing: 4) { @@ -135,7 +135,7 @@ struct TableOperationDialog: View { .foregroundStyle(.secondary) .padding(.leading, 20) } - .opacity(cascadeDisabled ? 0.5 : 1.0) + .opacity(cascadeDisabled ? 0.6 : 1.0) } .padding(.horizontal, 20) .padding(.vertical, 20) @@ -147,6 +147,7 @@ struct TableOperationDialog: View { Button("Cancel") { isPresented = false } + .keyboardShortcut(.cancelAction) Spacer()