fix(datagrid): key staged table operations by the object they target

CHANGELOG.md

@@ -104,6 +104,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Insert Row offered on Redis connections on iPhone and iPad, where it can only fail.
 - Missing search field in a connection's Tables tab on iPhone and iPad. (#2544)
 - A search from one connection or table still filtering another's list on iPhone and iPad.
+- A staged drop or truncate running against the database in front at Save time rather than the one it was staged in.
+- Unqualified `DROP TABLE` for every object on Oracle, Dameng, Trino, Snowflake and BigQuery, whatever its kind.
+- Dropping a table closing the tab on a same-named table in another schema or database.
 
 ### Security
 

TablePro/Core/Coordinators/RowEditingCoordinator+Discard.swift

@@ -48,8 +48,8 @@ extension RowEditingCoordinator {
     // MARK: - Discard
 
     func handleDiscard(
-        pendingTruncates: inout Set<String>,
-        pendingDeletes: inout Set<String>
+        pendingTruncates: inout Set<DatabaseTreeTableRef>,
+        pendingDeletes: inout Set<DatabaseTreeTableRef>
     ) {
         let originalValues = parent.changeManager.getOriginalValues()
         var deltas: [Delta] = []

TablePro/Core/Coordinators/RowEditingCoordinator+SaveChanges.swift

@@ -11,13 +11,14 @@ import TableProPluginKit
 private let saveChangesLogger = Logger(subsystem: "com.TablePro", category: "RowEditingCoordinator")
 
 extension RowEditingCoordinator {
-    /// The scope is read once, before the destructive-delete sheet and the authorization
-    /// prompt, so moving the selection to another database while either is open cannot
-    /// retarget the statements that were generated for the edited tab.
+    /// The plan carries the scope it was built for, so it runs where its statements were
+    /// generated however long the destructive-delete sheet and the authorization prompt stay
+    /// open. Reading the selected tab's scope again at execution time is what let a queued drop
+    /// land in whichever database the tab in front had moved to.
     func saveChanges(
-        pendingTruncates: inout Set<String>,
-        pendingDeletes: inout Set<String>,
-        tableOperationOptions: inout [String: TableOperationOptions]
+        pendingTruncates: inout Set<DatabaseTreeTableRef>,
+        pendingDeletes: inout Set<DatabaseTreeTableRef>,
+        tableOperationOptions: inout [DatabaseTreeTableRef: TableOperationOptions]
     ) {
         let hasEditedCells = parent.changeManager.hasChanges
         let hasPendingTableOps = !pendingTruncates.isEmpty || !pendingDeletes.isEmpty
@@ -43,7 +44,7 @@ extension RowEditingCoordinator {
             return
         }
 
-        guard let scope = parent.selectedTabScope else {
+        guard parent.selectedTabScope != nil else {
             failSave(message: String(localized: "Not connected to database"))
             return
         }
@@ -125,7 +126,7 @@ extension RowEditingCoordinator {
                 var opts = snapshotOptions
                 executeCommitPlan(
                     plan,
-                    scope: scope,
+                    scope: plan.scope,
                     clearTableOps: hasPendingTableOps,
                     pendingTruncates: &truncs,
                     pendingDeletes: &dels,
@@ -153,9 +154,9 @@ extension RowEditingCoordinator {
         _ plan: DataWritePlan,
         scope: DatabaseScope,
         clearTableOps: Bool,
-        pendingTruncates: inout Set<String>,
-        pendingDeletes: inout Set<String>,
-        tableOperationOptions: inout [String: TableOperationOptions]
+        pendingTruncates: inout Set<DatabaseTreeTableRef>,
+        pendingDeletes: inout Set<DatabaseTreeTableRef>,
+        tableOperationOptions: inout [DatabaseTreeTableRef: TableOperationOptions]
     ) {
         let validSteps = plan.steps.filter { !$0.statement.sql.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty }
         guard !validSteps.isEmpty else {
@@ -169,7 +170,7 @@ extension RowEditingCoordinator {
         let truncatedTables = Set(pendingTruncates)
         let conn = parent.connection
 
-        var capturedOptions: [String: TableOperationOptions] = [:]
+        var capturedOptions: [DatabaseTreeTableRef: TableOperationOptions] = [:]
         for table in deletedTables.union(truncatedTables) {
             capturedOptions[table] = tableOperationOptions[table]
         }
@@ -291,7 +292,7 @@ extension RowEditingCoordinator {
         plan: DataWritePlan,
         savingTabId: UUID?,
         clearTableOps: Bool,
-        deletedTables: Set<String>
+        deletedTables: Set<DatabaseTreeTableRef>
     ) {
         let savingTabIsSelected = savingTabId != nil && parent.tabManager.selectedTabId == savingTabId
         if savingTabIsSelected {
@@ -324,10 +325,24 @@ extension RowEditingCoordinator {
         parent.runQuery()
     }
 
-    private func closeTabsForDroppedTables(_ deletedTables: Set<String>) {
+    /// A tab is closed only when the object it is showing is one of the objects that went.
+    ///
+    /// It used to compare bare names, so dropping `analytics.users` also closed the tab on
+    /// `public.users` and threw away its row buffer, with nothing to undo it.
+    private func closeTabsForDroppedTables(_ deletedTables: Set<DatabaseTreeTableRef>) {
+        let browseDatabase = parent.browseDatabaseName
+        let dropped = Set(deletedTables.map { ref in
+            TableTabIdentity(
+                ref: ref,
+                browsing: browseDatabase,
+                resolvedSchema: DatabaseManager.shared.resolvedSchemaName(
+                    ref.qualifyingSchema, for: parent.connectionId
+                )
+            )
+        })
         let tabIdsToRemove = Set(
             parent.tabManager.tabs
-                .filter { $0.tabType == .table && deletedTables.contains($0.tableContext.tableName ?? "") }
+                .filter { tab in tab.tableIdentity(browsing: browseDatabase).map(dropped.contains) ?? false }
                 .map(\.id)
         )
         guard !tabIdsToRemove.isEmpty else { return }
@@ -493,9 +508,9 @@ extension RowEditingCoordinator {
 
     private func restorePendingTableOperations(
         connectionId: UUID,
-        truncates: Set<String>,
-        deletes: Set<String>,
-        options: [String: TableOperationOptions]
+        truncates: Set<DatabaseTreeTableRef>,
+        deletes: Set<DatabaseTreeTableRef>,
+        options: [DatabaseTreeTableRef: TableOperationOptions]
     ) {
         DatabaseManager.shared.updateSession(connectionId) { session in
             session.pendingTruncates = truncates

TablePro/Core/Database/TableOperationSQLBuilder.swift

@@ -4,61 +4,48 @@
 //
 
 import Foundation
-import os
 
 @MainActor
 struct TableOperationSQLBuilder {
-    nonisolated private static let logger = Logger(subsystem: "com.TablePro", category: "TableOperationSQLBuilder")
-
-    let connectionId: UUID
-    let databaseType: DatabaseType
-    let tableInfoProvider: () -> [String: TableInfo]
     let adapterProvider: () -> PluginDriverAdapter?
 
-    init(
-        connectionId: UUID,
-        databaseType: DatabaseType,
-        tableInfoProvider: @escaping () -> [String: TableInfo],
-        adapterProvider: @escaping () -> PluginDriverAdapter?
-    ) {
-        self.connectionId = connectionId
-        self.databaseType = databaseType
-        self.tableInfoProvider = tableInfoProvider
+    init(adapterProvider: @escaping () -> PluginDriverAdapter?) {
         self.adapterProvider = adapterProvider
     }
 
+    /// Every statement is built from the queued reference alone.
+    ///
+    /// The schema and the object's keyword used to be looked up in the connection's flat table
+    /// cache, which publishes nothing at all on an engine whose tree is per-schema: on Oracle,
+    /// Dameng, Trino, Snowflake and BigQuery every drop came out unqualified and typed `TABLE`,
+    /// so dropping a view raised ORA-00942 and dropping a table reached whatever object of that
+    /// name the login schema happened to hold.
     func generate(
-        truncates: Set<String>,
-        deletes: Set<String>,
-        options: [String: TableOperationOptions],
+        truncates: Set<DatabaseTreeTableRef>,
+        deletes: Set<DatabaseTreeTableRef>,
+        options: [DatabaseTreeTableRef: TableOperationOptions],
         includeFKHandling: Bool = true
     ) -> [String] {
         var statements: [String] = []
-        let sortedTruncates = truncates.sorted()
-        let sortedDeletes = deletes.sorted()
+        let sortedTruncates = truncates.sorted { $0.id < $1.id }
+        let sortedDeletes = deletes.sorted { $0.id < $1.id }
 
-        let needsDisableFK = includeFKHandling && truncates.union(deletes).contains { tableName in
-            options[tableName]?.ignoreForeignKeys == true
+        let needsDisableFK = includeFKHandling && truncates.union(deletes).contains { ref in
+            options[ref]?.ignoreForeignKeys == true
         }
 
         if needsDisableFK {
             statements.append(contentsOf: foreignKeyDisableStatements())
         }
 
-        let tableLookup = tableInfoProvider()
-
-        for tableName in sortedTruncates {
-            let tableOptions = options[tableName] ?? TableOperationOptions()
+        for ref in sortedTruncates {
             statements.append(contentsOf: truncateStatements(
-                tableName: tableName, schema: tableLookup[tableName]?.schema, options: tableOptions
+                ref, options: options[ref] ?? TableOperationOptions()
             ))
         }
 
-        for tableName in sortedDeletes {
-            let tableOptions = options[tableName] ?? TableOperationOptions()
-            let stmt = dropObjectStatement(
-                tableName: tableName, tableInfo: tableLookup[tableName], options: tableOptions
-            )
+        for ref in sortedDeletes {
+            let stmt = dropObjectStatement(ref, options: options[ref] ?? TableOperationOptions())
             if !stmt.isEmpty {
                 statements.append(stmt)
             }
@@ -80,38 +67,35 @@ struct TableOperationSQLBuilder {
     }
 
     private func truncateStatements(
-        tableName: String, schema: String?, options: TableOperationOptions
+        _ ref: DatabaseTreeTableRef, options: TableOperationOptions
     ) -> [String] {
         guard let adapter = adapterProvider() else { return [] }
         return adapter.truncateTableStatements(
-            table: tableName, schema: schema, cascade: options.cascade
+            table: ref.table.name, schema: ref.qualifyingSchema, cascade: options.cascade
         )
     }
 
     private func dropObjectStatement(
-        tableName: String, tableInfo: TableInfo?, options: TableOperationOptions
+        _ ref: DatabaseTreeTableRef, options: TableOperationOptions
     ) -> String {
         guard let adapter = adapterProvider() else { return "" }
-        if tableInfo == nil {
-            Self.logger.warning("No cached TableInfo for \(tableName, privacy: .public); dropping as TABLE")
-        }
         return adapter.dropObjectStatement(
-            name: tableName,
-            objectType: Self.dropKeyword(for: tableInfo?.type),
-            schema: tableInfo?.schema,
+            name: ref.table.name,
+            objectType: Self.dropKeyword(for: ref.table.type),
+            schema: ref.qualifyingSchema,
             cascade: options.cascade
         )
     }
 
-    private static func dropKeyword(for type: TableInfo.TableType?) -> String {
+    private static func dropKeyword(for type: TableInfo.TableType) -> String {
         switch type {
         case .view:
             return "VIEW"
         case .materializedView:
             return "MATERIALIZED VIEW"
         case .foreignTable:
             return "FOREIGN TABLE"
-        case .table, .systemTable, .partitionedTable, .externalTable, .none:
+        case .table, .systemTable, .partitionedTable, .externalTable:
             return "TABLE"
         }
     }

TablePro/Models/Connection/ConnectionSession.swift

@@ -22,10 +22,13 @@ struct ConnectionSession: Identifiable {
     var safeModeLevel: SafeModeLevel
 
     // Per-connection state
-    var selectedTables: Set<TableInfo> = []
-    var pendingTruncates: Set<String> = []
-    var pendingDeletes: Set<String> = []
-    var tableOperationOptions: [String: TableOperationOptions] = [:]
+    var selectedTables: Set<DatabaseTreeTableRef> = []
+    /// Queued Truncate and Drop, keyed by the object each one is aimed at rather than by its name.
+    /// The queue outlives a database switch, so a name-keyed entry was resolved at Save time
+    /// against whatever the selected tab pointed at by then.
+    var pendingTruncates: Set<DatabaseTreeTableRef> = []
+    var pendingDeletes: Set<DatabaseTreeTableRef> = []
+    var tableOperationOptions: [DatabaseTreeTableRef: TableOperationOptions] = [:]
     /// Where the user is browsing: what the sidebar lists and where a new tab opens.
     /// It is not where an open tab queries. A tab carries its own database and schema,
     /// and resolving an operation through these instead is how a tab ends up running

TablePro/Models/Database/DatabaseTreeTableRef.swift

@@ -0,0 +1,52 @@
+//
+//  DatabaseTreeTableRef.swift
+//  TablePro
+//
+
+import Foundation
+import TableProPluginKit
+
+/// One table, named by everything it takes to reach it.
+///
+/// `database` is nil when the connection browses no database, which is the normal state for
+/// an engine that has none. It stays optional all the way from `browsingDatabase` so it can be
+/// compared against the equally optional active database: an empty string here read as "some
+/// database called nothing", never equal to nil, and fired a database switch on every click.
+///
+/// It lives beside the models rather than beside the tree because it is what a queued Truncate or
+/// Drop is aimed at, and what the tab reconciliation after one matches against. Those used to hold
+/// a bare name, and a bare name cannot name a table: `orders` exists in every database on the
+/// server, the queue lives on the connection rather than on a tab, and the browse cursor moves
+/// freely while something is queued, so the queue resolved against whatever the selected tab
+/// happened to point at when Save ran.
+struct DatabaseTreeTableRef: Hashable, Identifiable, Sendable {
+    let database: String?
+    let schema: String?
+    let table: TableInfo
+
+    init(database: String?, schema: String?, table: TableInfo) {
+        self.database = database?.nilIfEmpty
+        self.schema = schema?.nilIfEmpty
+        self.table = table
+    }
+
+    /// The separator is escaped because every part of this is a user-chosen identifier and a
+    /// quoted one may contain anything. Joined raw, schema `a|b` with table `c` and schema `a`
+    /// with table `b|c` produced one id for two objects, and this id keys the outline's rows.
+    var id: String {
+        "\(Self.escaped(database))|\(Self.escaped(schema))|\(Self.escaped(table.id))"
+    }
+
+    /// The schema the statement should qualify with, which is the row's own before the table's.
+    /// A hierarchical tree hangs its tables off a schema node and the `TableInfo` under it may
+    /// carry none, while a flat list gets it the other way round.
+    var qualifyingSchema: String? {
+        schema ?? table.schema?.nilIfEmpty
+    }
+
+    private static func escaped(_ value: String?) -> String {
+        (value ?? "")
+            .replacingOccurrences(of: "\\", with: "\\\\")
+            .replacingOccurrences(of: "|", with: "\\|")
+    }
+}

TablePro/Models/Database/StagedWriteScope.swift

@@ -0,0 +1,44 @@
+//
+//  StagedWriteScope.swift
+//  TablePro
+//
+
+import Foundation
+
+/// Where one press of Save runs.
+///
+/// A queued Truncate or Drop names the database it was queued against, and the plan follows that
+/// rather than the tab in front. It used to take the selected tab's scope, so queueing a drop in
+/// one database, switching to another and pressing Save dropped the table of that name in the
+/// second one.
+///
+/// The statements are generated by the session's own driver before anything is pinned, and a
+/// driver that qualifies from its live connection writes the database it is on into the statement:
+/// Snowflake builds a three-part name from `currentDatabase`, SurrealDB opens with its current
+/// namespace. Pinning the plan afterwards cannot correct a name already written, so a queue is
+/// only saved from the database it was queued in, and is refused rather than rewritten anywhere
+/// else. Row edits carry no such queue and keep their own tab's scope.
+enum StagedWriteScope {
+    static func resolve(
+        tabScope: DatabaseScope,
+        browseDatabase: String,
+        stagedDatabases: Set<String>,
+        includesRowEdits: Bool
+    ) throws -> DatabaseScope {
+        let staged = stagedDatabases.filter { !$0.isEmpty }
+        guard !staged.isEmpty else { return tabScope }
+        guard staged.count == 1, let database = staged.first else {
+            throw DatabaseError.writeSpansDatabases
+        }
+        guard database == browseDatabase else {
+            throw DatabaseError.stagedWriteOutsideBrowsedDatabase(database)
+        }
+        guard !includesRowEdits || tabScope.database == database else {
+            throw DatabaseError.writeSpansDatabases
+        }
+        guard database != tabScope.database else { return tabScope }
+        /// The tab's schema belongs to the tab's database and means nothing in another one. Every
+        /// queued statement qualifies its own schema, so the scope does not need to carry one.
+        return DatabaseScope(connectionId: tabScope.connectionId, database: database, schema: nil)
+    }
+}