fix(coordinator): compose structure and create table previews on the scope save runs on (#3071)

Author: datlechin

CHANGELOG.md

@@ -364,6 +364,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Table opened in one database shown in Open Quickly's Recent in every other database, and opened there.
 - Open Quickly's Recent split between the Connections scope and the other scopes, each showing about half.
 - MySQL and MariaDB column defaults on iPhone and iPad missing for DEFAULT NULL, and string defaults shown unquoted.
+- Structure and Create Table SQL Preview disagreeing with Save on the schema, primary key name or a SQLite foreign key.
+- Row import creating its new table in another schema than its rows, and PGlite primary key changes failing to save.
 
 ### Security
 

TablePro/Core/Database/DatabaseManager+Schema.swift

@@ -13,42 +13,21 @@ import TableProPluginKit
 // MARK: - Schema Changes
 
 extension DatabaseManager {
-    /// Execute schema changes (ALTER TABLE, CREATE INDEX, etc.) in a transaction of their own,
+    /// Execute schema statements (ALTER TABLE, CREATE INDEX, etc.) in a transaction of their own,
     /// on the schema change route rather than the session driver a query tab may have left
     /// mid-transaction. The connection, database and schema all come from the editing tab's
     /// own scope, never from ambient session state that another window or tab can move.
     ///
-    /// Authorization sits between two scoped blocks rather than inside one: it awaits a
-    /// confirmation sheet and Touch ID, and holding the connection's driver gate across a
-    /// human prompt would freeze every other tab on that connection.
+    /// Authorization sits outside the scoped block: it awaits a confirmation sheet and Touch ID,
+    /// and holding the connection's driver gate across a human prompt would freeze every other
+    /// tab on that connection.
     func executeSchemaChanges(
-        tableName: String,
-        changes: [SchemaChange],
+        _ statements: [SchemaStatement],
         databaseType: DatabaseType,
         scope: DatabaseScope
     ) async throws {
         let route = schemaChangeRoute(for: scope)
 
-        let statements = try await withScopedDriver(
-            scope: scope, route: route, cancellation: .untracked
-        ) { driver in
-            let pkConstraintName = await Self.fetchPrimaryKeyConstraintName(
-                tableName: tableName,
-                databaseType: databaseType,
-                changes: changes,
-                driver: driver
-            )
-            guard let resolvedPluginDriver = (driver as? PluginDriverAdapter)?.schemaPluginDriver else {
-                throw DatabaseError.unsupportedOperation
-            }
-            let generator = SchemaStatementGenerator(
-                tableName: tableName,
-                primaryKeyConstraintName: pkConstraintName,
-                pluginDriver: resolvedPluginDriver
-            )
-            return try generator.generate(changes: changes)
-        }
-
         let combinedSQL = statements.map(\.sql).joined(separator: "\n")
         let schemaKind: OperationKind =
             QueryClassifier.classifyTier(combinedSQL, databaseType: databaseType) == .destructive
@@ -229,59 +208,4 @@ extension DatabaseManager {
             .changed(CatalogChange(connectionId: scope.connectionId, database: scope.database, kinds: .tables))
         )
     }
-
-    /// Query the actual primary key constraint name for PostgreSQL.
-    /// Returns nil if the database is not PostgreSQL, no PK modification is pending,
-    /// or the query fails (caller falls back to `{table}_pkey` convention).
-    private static func fetchPrimaryKeyConstraintName(
-        tableName: String,
-        databaseType: DatabaseType,
-        changes: [SchemaChange],
-        driver: DatabaseDriver
-    ) async -> String? {
-        // Only needed for PostgreSQL PK modifications
-        guard databaseType == .postgresql || databaseType == .redshift
-            || databaseType == .cockroachdb || databaseType == .duckdb else { return nil }
-        guard
-            changes.contains(where: {
-                if case .modifyPrimaryKey = $0 { return true }
-                return false
-            })
-        else {
-            return nil
-        }
-
-        let escapedTable = tableName.replacingOccurrences(of: "'", with: "''")
-        let schema: String
-        if let schemaDriver = driver as? SchemaSwitchable,
-           let escaped = schemaDriver.escapedSchema {
-            schema = escaped
-        } else {
-            schema = "public"
-        }
-        let query = """
-            SELECT con.conname
-            FROM pg_constraint con
-            JOIN pg_class rel ON rel.oid = con.conrelid
-            JOIN pg_namespace nsp ON nsp.oid = rel.relnamespace
-            WHERE rel.relname = '\(escapedTable)'
-              AND nsp.nspname = '\(schema)'
-              AND con.contype = 'p'
-            LIMIT 1
-            """
-
-        do {
-            let result = try await driver.execute(query: query)
-            if let row = result.rows.first, let name = row[0].asText, !name.isEmpty {
-                return name
-            }
-        } catch {
-            // Query failed - fall back to convention in SchemaStatementGenerator
-            Self.logger.warning(
-                "Failed to query PK constraint name for '\(tableName)': \(error.localizedDescription)"
-            )
-        }
-
-        return nil
-    }
 }

TablePro/Core/Database/DatabaseManager+SchemaComposition.swift

@@ -0,0 +1,68 @@
+//
+//  DatabaseManager+SchemaComposition.swift
+//  TablePro
+//
+
+import Foundation
+import TableProPluginKit
+import TableProSQLGrammar
+
+extension DatabaseManager {
+    func withSchemaComposer<T: Sendable>(
+        scope: DatabaseScope,
+        route: ScopedDriverRoute,
+        _ body: @Sendable @escaping (DatabaseDriver, any PluginDatabaseDriver) async throws -> T
+    ) async throws -> T {
+        try await withScopedDriver(scope: scope, route: route, cancellation: .untracked) { driver in
+            guard let pluginDriver = (driver as? PluginDriverAdapter)?.schemaPluginDriver else {
+                throw DatabaseError.unsupportedOperation
+            }
+            return try await body(driver, pluginDriver)
+        }
+    }
+
+    func schemaChangeStatements(
+        tableName: String,
+        changes: [SchemaChange],
+        scope: DatabaseScope
+    ) async throws -> [SchemaStatement] {
+        try await withSchemaComposer(scope: scope, route: schemaChangeRoute(for: scope)) { driver, pluginDriver in
+            let constraintName = await PrimaryKeyConstraintLookup.constraintName(
+                tableName: tableName,
+                changes: changes,
+                driver: driver
+            )
+            return try SchemaStatementGenerator(
+                tableName: tableName,
+                primaryKeyConstraintName: constraintName,
+                pluginDriver: pluginDriver
+            ).generate(changes: changes)
+        }
+    }
+
+    func createTableStatements(
+        plan: CreateTablePlan,
+        scope: DatabaseScope
+    ) async throws -> CreateTableStatements {
+        guard plan.definition != nil else {
+            return CreateTableStatements(statements: [], issues: plan.issues, tableName: nil)
+        }
+        return try await withSchemaComposer(scope: scope, route: schemaChangeRoute(for: scope)) { _, pluginDriver in
+            await MainActor.run {
+                CreateTableStatementComposer.compose(plan: plan, driver: pluginDriver)
+            }
+        }
+    }
+
+    func createTableStatements(
+        definition: PluginCreateTableDefinition,
+        scope: DatabaseScope,
+        route: ScopedDriverRoute
+    ) async throws -> [String] {
+        try await withSchemaComposer(scope: scope, route: route) { _, pluginDriver in
+            (pluginDriver.generateCreateTableStatements(definition: definition) ?? [])
+                .map { StatementBlank.trimming($0) }
+                .filter { !$0.isEmpty }
+        }
+    }
+}

TablePro/Core/SchemaTracking/PrimaryKeyConstraintLookup.swift

@@ -0,0 +1,45 @@
+//
+//  PrimaryKeyConstraintLookup.swift
+//  TablePro
+//
+
+import Foundation
+import os
+import TableProPluginKit
+
+enum PrimaryKeyConstraintLookup {
+    private static let logger = Logger(subsystem: "com.TablePro", category: "PrimaryKeyConstraintLookup")
+
+    static func constraintName(
+        tableName: String,
+        changes: [SchemaChange],
+        driver: DatabaseDriver
+    ) async -> String? {
+        guard changes.contains(where: dropsExistingPrimaryKey) else { return nil }
+        guard let schema = (driver as? SchemaSwitchable)?.escapedSchema else { return nil }
+
+        let query = """
+            SELECT CONSTRAINT_NAME
+            FROM INFORMATION_SCHEMA.TABLE_CONSTRAINTS
+            WHERE TABLE_SCHEMA = '\(schema)'
+              AND TABLE_NAME = '\(driver.escapeStringLiteral(tableName))'
+              AND CONSTRAINT_TYPE = 'PRIMARY KEY'
+            """
+
+        do {
+            let result = try await driver.execute(query: query)
+            guard let row = result.rows.first, let name = row.first?.asText, !name.isEmpty else { return nil }
+            return name
+        } catch {
+            logger.warning(
+                "Primary key constraint name lookup failed: \(error.publicLogShape, privacy: .public)"
+            )
+            return nil
+        }
+    }
+
+    private static func dropsExistingPrimaryKey(_ change: SchemaChange) -> Bool {
+        guard case .modifyPrimaryKey(let old, _) = change else { return false }
+        return !old.isEmpty
+    }
+}

TablePro/Core/Services/Export/ImportService.swift

@@ -52,6 +52,7 @@ final class ImportService: ObservableObject {
         from url: URL,
         formatId: String,
         encoding: String.Encoding,
+        scope: DatabaseScope,
         decompressedURL: URL? = nil,
         ownsDecompressedFile: Bool = false,
         knownStatementCount: Int? = nil,
@@ -62,12 +63,6 @@ final class ImportService: ObservableObject {
             throw PluginImportError.importFailed("Import format '\(formatId)' not found")
         }
 
-        /// The scope the driver is already on, not the connection's saved default: a tab may have
-        /// moved it, and on an engine that reconnects to change database, pinning somewhere else
-        /// would refuse the import outright.
-        guard let scope = DatabaseManager.shared.browseScope(for: connection.id) else {
-            throw DatabaseError.notConnected
-        }
         let route = DatabaseManager.shared.executionRoute(for: scope)
 
         state = ImportState(isImporting: true)

TablePro/Models/Schema/TableRebuildReviewRequest.swift

@@ -13,6 +13,13 @@ import TableProPluginKit
 /// first, and what the rebuild cannot carry over is named beside it.
 @MainActor
 struct TableRebuildReviewRequest: Identifiable {
+    struct Action {
+        /// What the confirming button says, which is the only thing that differs between a reorder
+        /// and a constraint change: both recreate the table, and the user is reading the same script.
+        let title: String
+        let perform: () async -> Void
+    }
+
     let id = UUID()
     let tableName: String
 
@@ -24,17 +31,17 @@ struct TableRebuildReviewRequest: Identifiable {
 
     let plan: PluginColumnReorderPlan
 
-    /// What the confirming button says, which is the only thing that differs between a reorder and
-    /// a constraint change: both recreate the table, and the user is reading the same script.
-    let actionTitle: String
-
-    let perform: () async -> Void
+    let action: Action?
 
     var warning: String? {
         plan.caveats.isEmpty ? nil : plan.caveats.joined(separator: " ")
     }
 
     var isRunnable: Bool { plan.isRunnable }
 
+    var runnableAction: Action? {
+        isRunnable ? action : nil
+    }
+
     var scriptStatements: [String] { plan.scriptStatements }
 }

TablePro/Views/Import/ImportDialog.swift

@@ -484,10 +484,17 @@ struct ImportDialog: View {
 
         importTask = Task {
             do {
+                /// The scope the driver is already on, not the connection's saved default: a tab may
+                /// have moved it, and on an engine that reconnects to change database, pinning
+                /// somewhere else would refuse the import outright.
+                guard let scope = DatabaseManager.shared.browseScope(for: connection.id) else {
+                    throw DatabaseError.notConnected
+                }
                 let result = try await service.importFile(
                     from: url,
                     formatId: selectedFormatId,
                     encoding: selectedEncoding.encoding,
+                    scope: scope,
                     decompressedURL: decompressedURL,
                     ownsDecompressedFile: ownsDecompressedFile,
                     knownStatementCount: statementCount > 0 ? statementCount : nil