diff --git a/OpenTable.xcodeproj/project.pbxproj b/OpenTable.xcodeproj/project.pbxproj index 309618c1b..2aab6ae1b 100644 --- a/OpenTable.xcodeproj/project.pbxproj +++ b/OpenTable.xcodeproj/project.pbxproj @@ -253,7 +253,7 @@ AUTOMATION_APPLE_EVENTS = NO; "CODE_SIGN_IDENTITY[sdk=macosx*]" = "Apple Development"; CODE_SIGN_STYLE = Automatic; - CURRENT_PROJECT_VERSION = 3; + CURRENT_PROJECT_VERSION = 4; DEVELOPMENT_TEAM = D7HJ5TFYCU; ENABLE_APP_SANDBOX = NO; ENABLE_HARDENED_RUNTIME = YES; @@ -286,7 +286,7 @@ "LD_RUNPATH_SEARCH_PATHS[sdk=macosx*]" = "@executable_path/../Frameworks"; LIBRARY_SEARCH_PATHS = "$(PROJECT_DIR)/Libs"; MACOSX_DEPLOYMENT_TARGET = 14.6; - MARKETING_VERSION = 0.1.3; + MARKETING_VERSION = 0.1.4; OTHER_LDFLAGS = ( "-force_load", "$(PROJECT_DIR)/Libs/libmariadb.a", @@ -331,7 +331,7 @@ AUTOMATION_APPLE_EVENTS = NO; "CODE_SIGN_IDENTITY[sdk=macosx*]" = "Apple Development"; CODE_SIGN_STYLE = Automatic; - CURRENT_PROJECT_VERSION = 3; + CURRENT_PROJECT_VERSION = 4; DEVELOPMENT_TEAM = D7HJ5TFYCU; ENABLE_APP_SANDBOX = NO; ENABLE_HARDENED_RUNTIME = YES; @@ -364,7 +364,7 @@ "LD_RUNPATH_SEARCH_PATHS[sdk=macosx*]" = "@executable_path/../Frameworks"; LIBRARY_SEARCH_PATHS = "$(PROJECT_DIR)/Libs"; MACOSX_DEPLOYMENT_TARGET = 14.6; - MARKETING_VERSION = 0.1.3; + MARKETING_VERSION = 0.1.4; OTHER_LDFLAGS = ( "-force_load", "$(PROJECT_DIR)/Libs/libmariadb.a", diff --git a/OpenTable.xcodeproj/project.xcworkspace/xcuserdata/ngoquocdat.xcuserdatad/UserInterfaceState.xcuserstate b/OpenTable.xcodeproj/project.xcworkspace/xcuserdata/ngoquocdat.xcuserdatad/UserInterfaceState.xcuserstate index e096cf486..547ce69b8 100644 Binary files a/OpenTable.xcodeproj/project.xcworkspace/xcuserdata/ngoquocdat.xcuserdatad/UserInterfaceState.xcuserstate and b/OpenTable.xcodeproj/project.xcworkspace/xcuserdata/ngoquocdat.xcuserdatad/UserInterfaceState.xcuserstate differ diff --git a/OpenTable/Models/DataChange.swift b/OpenTable/Models/DataChange.swift index 64357179e..7235ef9d8 100644 --- a/OpenTable/Models/DataChange.swift +++ b/OpenTable/Models/DataChange.swift @@ -66,6 +66,8 @@ enum UndoAction { case rowDeletion(rowIndex: Int, originalRow: [String?]) /// Batch deletion of multiple rows (for undo as a single action) case batchRowDeletion(rows: [(rowIndex: Int, originalRow: [String?])]) + /// Batch insertion undo - when user deletes multiple inserted rows at once + case batchRowInsertion(rowIndices: [Int], rowValues: [[String?]]) } @MainActor @@ -101,6 +103,11 @@ final class DataChangeManager: ObservableObject { /// Set of "rowIndex-colIndex" strings for modified cells - O(1) lookup private var modifiedCells: Set = [] + /// Lazy storage for inserted row values - avoids creating CellChange objects until needed + /// Maps rowIndex -> column values array for newly inserted rows + /// This dramatically improves add row performance for tables with many columns + private var insertedRowData: [Int: [String?]] = [:] + /// Undo stack for reversing changes (LIFO) private var undoStack: [UndoAction] = [] @@ -326,6 +333,12 @@ final class DataChangeManager: ObservableObject { /// Undo a pending row deletion func undoRowDeletion(rowIndex: Int) { + // SAFETY: Only process if this row is actually marked as deleted + guard deletedRowIndices.contains(rowIndex) else { + print("⚠️ undoRowDeletion called for row \(rowIndex) but it's not in deletedRowIndices") + return + } + changes.removeAll { $0.rowIndex == rowIndex && $0.type == .delete } deletedRowIndices.remove(rowIndex) hasChanges = !changes.isEmpty @@ -334,11 +347,17 @@ final class DataChangeManager: ObservableObject { /// Undo a pending row insertion func undoRowInsertion(rowIndex: Int) { + // SAFETY: Only process if this row is actually marked as inserted + guard insertedRowIndices.contains(rowIndex) else { + print("⚠️ undoRowInsertion: row \(rowIndex) not in insertedRowIndices") + return + } + + // Remove the INSERT change from the changes array changes.removeAll { $0.rowIndex == rowIndex && $0.type == .insert } insertedRowIndices.remove(rowIndex) // Shift down indices for rows after the removed row - // This is necessary because when a row is removed, all subsequent rows shift down var shiftedInsertedIndices = Set() for idx in insertedRowIndices { if idx > rowIndex { @@ -349,7 +368,7 @@ final class DataChangeManager: ObservableObject { } insertedRowIndices = shiftedInsertedIndices - // Also update row indices in changes array + // Also update row indices in changes array for all changes after this row for i in 0.. rowIndex { changes[i].rowIndex -= 1 @@ -358,6 +377,65 @@ final class DataChangeManager: ObservableObject { hasChanges = !changes.isEmpty } + + /// Undo multiple row insertions at once (for batch deletion) + /// This is more efficient than calling undoRowInsertion multiple times + /// - Parameter rowIndices: Array of row indices to undo, MUST be sorted in descending order + func undoBatchRowInsertion(rowIndices: [Int]) { + guard !rowIndices.isEmpty else { return } + + // Verify all rows are inserted + let validRows = rowIndices.filter { insertedRowIndices.contains($0) } + + if validRows.count != rowIndices.count { + let invalidRows = Set(rowIndices).subtracting(validRows) + print("⚠️ undoBatchRowInsertion: rows \(invalidRows) not in insertedRowIndices") + } + + guard !validRows.isEmpty else { return } + + // Collect row values BEFORE removing changes (for undo/redo) + var rowValues: [[String?]] = [] + for rowIndex in validRows { + if let insertChange = changes.first(where: { $0.rowIndex == rowIndex && $0.type == .insert }) { + let values = insertChange.cellChanges.sorted { $0.columnIndex < $1.columnIndex } + .map { $0.newValue } + rowValues.append(values) + } else { + rowValues.append(Array(repeating: nil, count: columns.count)) + } + } + + // Remove all INSERT changes for these rows + for rowIndex in validRows { + changes.removeAll { $0.rowIndex == rowIndex && $0.type == .insert } + insertedRowIndices.remove(rowIndex) + } + + // Push undo action so user can undo this deletion + pushUndo(.batchRowInsertion(rowIndices: validRows, rowValues: rowValues)) + + // Shift indices for all remaining rows + for deletedIndex in validRows.reversed() { + var shiftedIndices = Set() + for idx in insertedRowIndices { + if idx > deletedIndex { + shiftedIndices.insert(idx - 1) + } else { + shiftedIndices.insert(idx) + } + } + insertedRowIndices = shiftedIndices + + for i in 0.. deletedIndex { + changes[i].rowIndex -= 1 + } + } + } + + hasChanges = !changes.isEmpty + } // MARK: - Undo Stack Management @@ -462,6 +540,33 @@ final class DataChangeManager: ObservableObject { undoRowDeletion(rowIndex: rowIndex) } return (action, false, true, nil) + + case .batchRowInsertion(let rowIndices, let rowValues): + // Undo the deletion of inserted rows - restore them as INSERT changes + // Process in reverse order (ascending) to maintain correct indices when re-inserting + for (index, rowIndex) in rowIndices.enumerated().reversed() { + guard index < rowValues.count else { continue } + let values = rowValues[index] + + // Re-create INSERT change + let cellChanges = values.enumerated().map { colIndex, value in + CellChange( + rowIndex: rowIndex, + columnIndex: colIndex, + columnName: columns[safe: colIndex] ?? "", + oldValue: nil, + newValue: value + ) + } + let rowChange = RowChange(rowIndex: rowIndex, type: .insert, cellChanges: cellChanges) + changes.append(rowChange) + insertedRowIndices.insert(rowIndex) + } + + hasChanges = !changes.isEmpty + reloadVersion += 1 + // Return true for needsRowInsert so MainContentView knows to restore to resultRows + return (action, true, false, nil) } } @@ -484,8 +589,26 @@ final class DataChangeManager: ObservableObject { return (action, false, false) case .rowInsertion(let rowIndex): - // Re-apply the row insertion - mark as inserted + // Re-apply the row insertion - we need to restore the full INSERT change + // Note: We don't have the original cell values in the UndoAction, + // so we need the caller (MainContentView) to provide them when re-inserting the row + // For now, just mark as inserted and let the caller handle cell values insertedRowIndices.insert(rowIndex) + + // Create empty INSERT change - caller should update with actual values + // The row should already exist in resultRows from the redo handler in MainContentView + let cellChanges = columns.enumerated().map { index, columnName in + CellChange( + rowIndex: rowIndex, + columnIndex: index, + columnName: columnName, + oldValue: nil, + newValue: nil // Will be updated by caller + ) + } + let rowChange = RowChange(rowIndex: rowIndex, type: .insert, cellChanges: cellChanges) + changes.append(rowChange) + hasChanges = true reloadVersion += 1 return (action, true, false) @@ -505,6 +628,20 @@ final class DataChangeManager: ObservableObject { _ = undoStack.popLast() } return (action, false, true) + + case .batchRowInsertion(let rowIndices, _): + // Redo the deletion of inserted rows - remove them again + // This is called when user: delete inserted rows -> undo -> redo + // We need to remove the rows from changes and insertedRowIndices again + for rowIndex in rowIndices { + changes.removeAll { $0.rowIndex == rowIndex && $0.type == .insert } + insertedRowIndices.remove(rowIndex) + } + hasChanges = !changes.isEmpty + reloadVersion += 1 + // Return true for needsRowInsert to signal MainContentView to remove from resultRows + // (We repurpose this flag since the logic is similar - rows need to be removed) + return (action, true, false) } } @@ -520,10 +657,20 @@ final class DataChangeManager: ObservableObject { statements.append(sql) } case .insert: + // SAFETY: Verify the row is still marked as inserted + guard insertedRowIndices.contains(change.rowIndex) else { + print("⚠️ Skipping INSERT for row \(change.rowIndex) - not in insertedRowIndices") + continue + } if let sql = generateInsertSQL(for: change) { statements.append(sql) } case .delete: + // SAFETY: Verify the row is still marked as deleted + guard deletedRowIndices.contains(change.rowIndex) else { + print("⚠️ Skipping DELETE for row \(change.rowIndex) - not in deletedRowIndices") + continue + } if let sql = generateDeleteSQL(for: change) { statements.append(sql) } diff --git a/OpenTable/Views/MainContentView.swift b/OpenTable/Views/MainContentView.swift index 91ef963e3..aa82647f4 100644 --- a/OpenTable/Views/MainContentView.swift +++ b/OpenTable/Views/MainContentView.swift @@ -344,10 +344,16 @@ struct MainContentView: View { if hasEditedCells || hasPendingTableOps { pendingDiscardAction = .refresh } else { - // No changes - refresh table browser and run current query - DispatchQueue.main.async { - NotificationCenter.default.post(name: .refreshAll, object: nil) + // Cancel any running query to prevent race conditions + currentQueryTask?.cancel() + + // Rebuild query for table tabs to ensure fresh data + if let tabIndex = tabManager.selectedTabIndex, + tabManager.tabs[tabIndex].tabType == .table { + rebuildTableQuery(at: tabIndex) } + + // Fetch fresh data from database runQuery() } } @@ -811,6 +817,7 @@ struct MainContentView: View { guard !Task.isCancelled else { return } if let idx = tabManager.tabs.firstIndex(where: { $0.id == tabId }) { + // CRITICAL: Update tab atomically to prevent objc_retain crashes // with large result sets (25+ columns). Working with a copy first // prevents partial updates that can crash during deallocation. @@ -983,59 +990,84 @@ struct MainContentView: View { /// Delete selected rows (Delete key or menu) private func deleteSelectedRows() { - guard let index = tabManager.selectedTabIndex, + guard let tabIndex = tabManager.selectedTabIndex, !selectedRowIndices.isEmpty else { return } - // Collect rows to delete for batch undo (sorted descending to handle removals correctly) - var rowsToDelete: [(rowIndex: Int, originalRow: [String?])] = [] + // Separate inserted rows from existing rows + var insertedRowsToDelete: [Int] = [] + var existingRowsToDelete: [(rowIndex: Int, originalRow: [String?])] = [] // Find the lowest selected row index for selection movement let minSelectedRow = selectedRowIndices.min() ?? 0 let maxSelectedRow = selectedRowIndices.max() ?? 0 - // Delete each selected row (sorted descending to handle removals correctly) + // Categorize rows (process in descending order to maintain correct indices) for rowIndex in selectedRowIndices.sorted(by: >) { if changeManager.isRowInserted(rowIndex) { - // For inserted rows, remove them completely - undoInsertRow(at: rowIndex) + // Collect inserted rows to delete + insertedRowsToDelete.append(rowIndex) } else if !changeManager.isRowDeleted(rowIndex) { - // For existing rows, collect for batch deletion - if rowIndex < tabManager.tabs[index].resultRows.count { - let originalRow = tabManager.tabs[index].resultRows[rowIndex].values - rowsToDelete.append((rowIndex: rowIndex, originalRow: originalRow)) + // Collect existing rows for batch deletion + if rowIndex < tabManager.tabs[tabIndex].resultRows.count { + let originalRow = tabManager.tabs[tabIndex].resultRows[rowIndex].values + existingRowsToDelete.append((rowIndex: rowIndex, originalRow: originalRow)) } } } - // Record batch deletion (single undo action for all rows) - if !rowsToDelete.isEmpty { - changeManager.recordBatchRowDeletion(rows: rowsToDelete) + // Process inserted rows deletion + if !insertedRowsToDelete.isEmpty { + // Sort descending so removing higher indices first doesn't affect lower indices + let sortedInsertedRows = insertedRowsToDelete.sorted(by: >) + + // Remove from resultRows first (descending order) + for rowIndex in sortedInsertedRows { + guard rowIndex < tabManager.tabs[tabIndex].resultRows.count else { continue } + tabManager.tabs[tabIndex].resultRows.remove(at: rowIndex) + } + + // Update changeManager for ALL deleted inserted rows at once + // This prevents index shifting issues from calling undoRowInsertion multiple times + changeManager.undoBatchRowInsertion(rowIndices: sortedInsertedRows) + } + + // Record batch deletion for existing rows (single undo action for all rows) + if !existingRowsToDelete.isEmpty { + changeManager.recordBatchRowDeletion(rows: existingRowsToDelete) } // Move selection to next available row after deletion - let totalRows = tabManager.tabs[index].resultRows.count - let nextRow: Int + let totalRows = tabManager.tabs[tabIndex].resultRows.count - if maxSelectedRow + 1 < totalRows { + // Calculate next row selection, accounting for deleted inserted rows + let rowsDeleted = insertedRowsToDelete.count + let adjustedMaxRow = maxSelectedRow - rowsDeleted + let adjustedMinRow = minSelectedRow - insertedRowsToDelete.filter { $0 < minSelectedRow }.count + + let nextRow: Int + if adjustedMaxRow + 1 < totalRows { // Select row after the deleted range - nextRow = maxSelectedRow + 1 - } else if minSelectedRow > 0 { + nextRow = min(adjustedMaxRow + 1, totalRows - 1) + } else if adjustedMinRow > 0 { // Deleted rows at end, select previous row - nextRow = minSelectedRow - 1 + nextRow = adjustedMinRow - 1 + } else if totalRows > 0 { + // Select first row if available + nextRow = 0 } else { - // All rows deleted or only first row deleted + // All rows deleted nextRow = -1 } - if nextRow >= 0 { + if nextRow >= 0 && nextRow < totalRows { selectedRowIndices = [nextRow] } else { selectedRowIndices.removeAll() } // Mark tab as having user interaction (prevents auto-replacement) - tabManager.tabs[index].hasUserInteraction = true + tabManager.tabs[tabIndex].hasUserInteraction = true } /// Toggle table deletion state (for sidebar table selection) @@ -1183,6 +1215,38 @@ struct MainContentView: View { tabManager.tabs[tabIndex].query = newQuery runQuery() } + + /// Rebuild query for a table tab based on current filters and sort state + /// Used when refreshing to ensure query reflects current state + private func rebuildTableQuery(at tabIndex: Int) { + guard tabIndex < tabManager.tabs.count else { return } + let tab = tabManager.tabs[tabIndex] + guard let tableName = tab.tableName else { return } + + let quotedTable = connection.type.quoteIdentifier(tableName) + var newQuery = "SELECT * FROM \(quotedTable)" + + // Apply filters if any + if filterStateManager.hasAppliedFilters { + let generator = FilterSQLGenerator(databaseType: connection.type) + let whereClause = generator.generateWhereClause(from: filterStateManager.appliedFilters) + if !whereClause.isEmpty { + newQuery += " \(whereClause)" + } + } + + // Preserve ORDER BY + if let columnIndex = tab.sortState.columnIndex, + columnIndex < tab.resultColumns.count { + let columnName = tab.resultColumns[columnIndex] + let direction = tab.sortState.direction == .ascending ? "ASC" : "DESC" + newQuery += " ORDER BY \(connection.type.quoteIdentifier(columnName)) \(direction)" + } + + newQuery += " LIMIT 200" + + tabManager.tabs[tabIndex].query = newQuery + } // MARK: - Column Sorting @@ -1510,6 +1574,18 @@ struct MainContentView: View { // All rows are restored in changeManager - visual indicators will be removed // No need to modify resultRows since deletions were just visual indicators break + + case .batchRowInsertion(let rowIndices, let rowValues): + // Restore deleted inserted rows - add them back to resultRows + // Process in reverse order (ascending) to maintain correct indices + for (index, rowIndex) in rowIndices.enumerated().reversed() { + guard index < rowValues.count else { continue } + guard rowIndex <= tabManager.tabs[tabIndex].resultRows.count else { continue } + + let values = rowValues[index] + let newRow = QueryResultRow(values: values) + tabManager.tabs[tabIndex].resultRows.insert(newRow, at: rowIndex) + } } // Mark tab as having user interaction @@ -1549,6 +1625,14 @@ struct MainContentView: View { // Rows are re-marked as deleted in changeManager // No need to modify resultRows since deletions are just visual indicators break + + case .batchRowInsertion(let rowIndices, _): + // Redo the deletion - remove the rows from resultRows again + // Remove in descending order to avoid index shifting issues + for rowIndex in rowIndices.sorted(by: >) { + guard rowIndex < tabManager.tabs[tabIndex].resultRows.count else { continue } + tabManager.tabs[tabIndex].resultRows.remove(at: rowIndex) + } } // Mark tab as having user interaction @@ -1682,6 +1766,11 @@ struct MainContentView: View { // Execute the specific action switch action { case .refresh, .refreshAll: + // Rebuild query for table tabs before refreshing + if let tabIndex = tabManager.selectedTabIndex, + tabManager.tabs[tabIndex].tabType == .table { + rebuildTableQuery(at: tabIndex) + } runQuery() case .closeTab: closeCurrentTab() @@ -1719,16 +1808,10 @@ struct MainContentView: View { /// Save pending changes (Cmd+S) private func saveChanges() { - print("DEBUG: saveChanges() called") - let hasEditedCells = changeManager.hasChanges let hasPendingTableOps = !pendingTruncates.isEmpty || !pendingDeletes.isEmpty - print("DEBUG: hasEditedCells = \(hasEditedCells)") - print("DEBUG: hasPendingTableOps = \(hasPendingTableOps)") - guard hasEditedCells || hasPendingTableOps else { - print("DEBUG: No changes to save") return } @@ -1737,9 +1820,7 @@ struct MainContentView: View { // 1. Generate SQL for cell edits if hasEditedCells { let cellStatements = changeManager.generateSQL() - print("DEBUG: Generated \(cellStatements.count) cell edit SQL statements") for (index, stmt) in cellStatements.enumerated() { - print("DEBUG: Cell statement \(index + 1): \(stmt)") } allStatements.append(contentsOf: cellStatements) } @@ -1750,7 +1831,6 @@ struct MainContentView: View { for tableName in pendingTruncates { let quotedName = connection.type.quoteIdentifier(tableName) let stmt = "TRUNCATE TABLE \(quotedName)" - print("DEBUG: Table operation: \(stmt)") allStatements.append(stmt) } @@ -1758,13 +1838,11 @@ struct MainContentView: View { for tableName in pendingDeletes { let quotedName = connection.type.quoteIdentifier(tableName) let stmt = "DROP TABLE \(quotedName)" - print("DEBUG: Table operation: \(stmt)") allStatements.append(stmt) } } guard !allStatements.isEmpty else { - print("DEBUG: No SQL statements generated") if let index = tabManager.selectedTabIndex { tabManager.tabs[index].errorMessage = "Could not generate SQL for changes." } @@ -1779,7 +1857,6 @@ struct MainContentView: View { private func executeCommitSQL(_ sql: String, clearTableOps: Bool = false) { guard !sql.isEmpty else { return } - print("DEBUG: Executing SQL:\n\(sql)") Task { do { @@ -1799,11 +1876,9 @@ struct MainContentView: View { } for statement in statements { - print("DEBUG: Executing: \(statement)") _ = try await driver.execute(query: statement) } - print("DEBUG: All statements executed successfully") // Clear pending changes since they're now saved await MainActor.run { @@ -1848,7 +1923,6 @@ struct MainContentView: View { } } - print("DEBUG: Changes cleared, refreshing query") } // Refresh the current query to show updated data (if tab still exists) @@ -1857,7 +1931,6 @@ struct MainContentView: View { } } catch { - print("DEBUG: Error during save: \(error)") await MainActor.run { if let index = tabManager.selectedTabIndex { tabManager.tabs[index].errorMessage = diff --git a/OpenTable/Views/Sidebar/SidebarView.swift b/OpenTable/Views/Sidebar/SidebarView.swift index 55e7b8ef0..967d049bb 100644 --- a/OpenTable/Views/Sidebar/SidebarView.swift +++ b/OpenTable/Views/Sidebar/SidebarView.swift @@ -350,10 +350,8 @@ struct SidebarView: View { pendingTruncates.remove(tableName) if pendingDeletes.contains(tableName) { pendingDeletes.remove(tableName) - print("DEBUG: Removed \(tableName) from pendingDeletes") } else { pendingDeletes.insert(tableName) - print("DEBUG: Inserted \(tableName) into pendingDeletes, count: \(pendingDeletes.count)") } } }