diff --git a/CHANGELOG.md b/CHANGELOG.md index 3cb585c45..047407f20 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Tab rows in Settings > General > Tabs, wrapping the strip instead of scrolling it. (#2438) - Autoscrolling while dragging a tab, so a tab can be moved past the run currently on screen. (#2438) - Move Tab to New Window on a tab's right-click menu, and by dragging a tab out of the strip. (#2438) +- Column reorder by dragging on ClickHouse and Oracle. (#2479) +- Column reorder on PostgreSQL, SQLite, libSQL, Turso and Cloudflare D1, through a table rebuild shown before anything runs. (#2479) - Recognition of SQLite and DuckDB databases by their contents, whatever they are named. (#2476) - `.parquet` files in Finder's Open With, read through DuckDB. (#2476) - Prompt to install the driver a file needs, before the file opens. (#2476) @@ -27,6 +29,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - MongoDB editor diagnostics report JavaScript syntax errors rather than unsupported method names. - Editor tab presses handled by AppKit rather than SwiftUI gestures. (#2438) - Connection-first labels with the database or schema on a second line in the connections strip. (#2550) +- Column reorder withheld, with the reason on the row number, where the engine cannot change column order. (#2479) - File > Open File… as an app command over every file TablePro reads. (#2476) ### Fixed diff --git a/Plugins/ClickHouseDriverPlugin/ClickHousePlugin.swift b/Plugins/ClickHouseDriverPlugin/ClickHousePlugin.swift index 23d0e280b..c26f9a1d2 100644 --- a/Plugins/ClickHouseDriverPlugin/ClickHousePlugin.swift +++ b/Plugins/ClickHouseDriverPlugin/ClickHousePlugin.swift @@ -827,6 +827,52 @@ final class ClickHousePluginDriver: PluginDatabaseDriver, @unchecked Sendable { "ALTER TABLE \(quoteIdentifier(table)) DROP COLUMN \(quoteIdentifier(columnName))" } + /// `ALTER TABLE … MODIFY COLUMN name type FIRST | AFTER other`. + /// + /// Measured against 26.8: the type is mandatory (`MODIFY COLUMN c AFTER b` is a syntax error), + /// and naming it alone is enough. `MODIFY COLUMN` changes only the properties the statement + /// spells out, so the default, comment, codec, TTL and the MATERIALIZED, ALIAS and EPHEMERAL + /// kinds all survive a move, and the statement rewrites metadata without starting a mutation. + /// Restating the full definition instead would rewrite a MATERIALIZED column as a DEFAULT one + /// and requote every expression default. + /// + /// The type comes from the server rather than from the caller's definition, because it has to + /// be the stored type down to the `Nullable(…)` wrapper and a round trip is cheaper than a + /// column that comes back with a different type than it went in with. + func generateColumnReorderPlan( + table: String, + schema: String?, + columns: [PluginColumnDefinition], + desiredOrder: [String] + ) async throws -> PluginColumnReorderPlan? { + let storedTypes = try await fetchStoredColumnTypes(table: table) + let currentOrder = storedTypes.map(\.name) + let statements = PluginColumnReorderPlanner + .moves(from: currentOrder, to: desiredOrder) + .compactMap { move -> String? in + guard let type = storedTypes.first(where: { $0.name == move.column })?.type else { return nil } + let position = move.afterColumn.map { "AFTER \(quoteIdentifier($0))" } ?? "FIRST" + return "ALTER TABLE \(quoteIdentifier(table)) " + + "MODIFY COLUMN \(quoteIdentifier(move.column)) \(type) \(position)" + } + guard !statements.isEmpty else { return nil } + return PluginColumnReorderPlan(statements: statements, cost: .metadataOnly) + } + + private func fetchStoredColumnTypes(table: String) async throws -> [(name: String, type: String)] { + let escapedTable = table.replacingOccurrences(of: "'", with: "''") + let result = try await execute(query: """ + SELECT name, type + FROM system.columns + WHERE database = currentDatabase() AND table = '\(escapedTable)' + ORDER BY position + """) + return result.rows.compactMap { row in + guard let name = row[safe: 0]?.asText, let type = row[safe: 1]?.asText else { return nil } + return (name, type) + } + } + func generateAddIndexSQL(table: String, index: PluginIndexDefinition) -> String? { let cols = index.columns.map { quoteIdentifier($0) }.joined(separator: ", ") let indexType = index.indexType ?? "minmax" diff --git a/Plugins/CloudflareD1DriverPlugin/CloudflareD1PluginDriver.swift b/Plugins/CloudflareD1DriverPlugin/CloudflareD1PluginDriver.swift index 2feb3c450..25ae5eeeb 100644 --- a/Plugins/CloudflareD1DriverPlugin/CloudflareD1PluginDriver.swift +++ b/Plugins/CloudflareD1DriverPlugin/CloudflareD1PluginDriver.swift @@ -714,6 +714,32 @@ final class CloudflareD1PluginDriver: PluginDatabaseDriver, @unchecked Sendable "ALTER TABLE \(quoteIdentifier(table)) DROP COLUMN \(quoteIdentifier(columnName))" } + /// SQLite has no positional `ALTER`, so the order changes by rebuilding the table, using the + /// shared recipe every SQLite-derived driver follows. + /// + /// Never run by TablePro. D1 answers each statement over its own HTTP request, so nothing can + /// hold the rebuild's transaction open across them, and a half-applied rebuild is data loss. + func generateColumnReorderPlan( + table: String, + schema: String?, + columns: [PluginColumnDefinition], + desiredOrder: [String] + ) async throws -> PluginColumnReorderPlan? { + try await SQLiteColumnReorderPlanner.plan( + tableName: table, + desiredOrder: desiredOrder, + isRunnable: false, + execute: { try await self.execute(query: $0) } + ) + } + + func columnReorderSchemaFingerprint(table: String, schema: String?) async throws -> String? { + try await SQLiteColumnReorderPlanner.schemaFingerprint( + tableName: table, + execute: { try await self.execute(query: $0) } + ) + } + func generateAddIndexSQL(table: String, index: PluginIndexDefinition) -> String? { let uniqueStr = index.isUnique ? "UNIQUE " : "" let cols = index.columns.map { quoteIdentifier($0) }.joined(separator: ", ") diff --git a/Plugins/LibSQLDriverPlugin/LibSQLPluginDriver.swift b/Plugins/LibSQLDriverPlugin/LibSQLPluginDriver.swift index 01496b8d9..1e68dbe9f 100644 --- a/Plugins/LibSQLDriverPlugin/LibSQLPluginDriver.swift +++ b/Plugins/LibSQLDriverPlugin/LibSQLPluginDriver.swift @@ -730,6 +730,33 @@ final class LibSQLPluginDriver: PluginDatabaseDriver, @unchecked Sendable { "ALTER TABLE \(quoteIdentifier(table)) DROP COLUMN \(quoteIdentifier(columnName))" } + /// SQLite has no positional `ALTER`, so the order changes by rebuilding the table, using the + /// shared recipe every SQLite-derived driver follows. + /// + /// Runnable only in local mode. A local database is a real SQLite handle that holds a + /// transaction across statements, which is what makes the rebuild atomic; over HTTP each + /// statement is its own request and the script has to be handed to the user instead. + func generateColumnReorderPlan( + table: String, + schema: String?, + columns: [PluginColumnDefinition], + desiredOrder: [String] + ) async throws -> PluginColumnReorderPlan? { + try await SQLiteColumnReorderPlanner.plan( + tableName: table, + desiredOrder: desiredOrder, + isRunnable: isLocalMode, + execute: { try await self.execute(query: $0) } + ) + } + + func columnReorderSchemaFingerprint(table: String, schema: String?) async throws -> String? { + try await SQLiteColumnReorderPlanner.schemaFingerprint( + tableName: table, + execute: { try await self.execute(query: $0) } + ) + } + func generateAddIndexSQL(table: String, index: PluginIndexDefinition) -> String? { let uniqueStr = index.isUnique ? "UNIQUE " : "" let cols = index.columns.map { quoteIdentifier($0) }.joined(separator: ", ") diff --git a/Plugins/MySQLDriverPlugin/MySQLColumnDefinitionSQL.swift b/Plugins/MySQLDriverPlugin/MySQLColumnDefinitionSQL.swift index 12e9c017f..c5eea2809 100644 --- a/Plugins/MySQLDriverPlugin/MySQLColumnDefinitionSQL.swift +++ b/Plugins/MySQLDriverPlugin/MySQLColumnDefinitionSQL.swift @@ -88,8 +88,18 @@ internal func mysqlColumnDefinitionSQL(_ column: PluginColumnDefinition) -> Stri // A generated column takes the expression in place of the ordinary default and auto-increment // attributes, and MySQL rejects most of them alongside it. The keyword is spelled out because // both MySQL and MariaDB default to VIRTUAL. + // Charset and collation belong to the type, so they come before the expression. Leaving them + // out reset a generated string column to the table defaults, because MODIFY COLUMN replaces + // the whole definition and a reorder restates it. let kind = (column.generationKind ?? .virtual).rawValue - var definition = "\(name) \(column.dataType) GENERATED ALWAYS AS (\(expression)) \(kind)" + var definition = "\(name) \(column.dataType)" + if let charset = column.charset, !charset.isEmpty { + definition += " CHARACTER SET \(charset)" + } + if let collation = column.collation, !collation.isEmpty { + definition += " COLLATE \(collation)" + } + definition += " GENERATED ALWAYS AS (\(expression)) \(kind)" if !column.isNullable { definition += " NOT NULL" } if let comment = column.comment, !comment.isEmpty { definition += " COMMENT '\(mysqlEscapeStringLiteral(comment))'" diff --git a/Plugins/MySQLDriverPlugin/MySQLPluginDriver.swift b/Plugins/MySQLDriverPlugin/MySQLPluginDriver.swift index 297f2a505..0767634d6 100644 --- a/Plugins/MySQLDriverPlugin/MySQLPluginDriver.swift +++ b/Plugins/MySQLDriverPlugin/MySQLPluginDriver.swift @@ -944,18 +944,31 @@ final class MySQLPluginDriver: PluginDatabaseDriver, @unchecked Sendable { func generateMoveColumnSQL(table: String, column: PluginColumnDefinition, afterColumn: String?) -> String? { let tableName = quoteIdentifier(table) - let colName = quoteIdentifier(column.name) - - let def = "\(column.dataType)" + mysqlColumnAttributesSQL(column) - - let position: String - if let afterCol = afterColumn { - position = "AFTER \(quoteIdentifier(afterCol))" - } else { - position = "FIRST" - } - - return "ALTER TABLE \(tableName) MODIFY COLUMN \(colName) \(def) \(position)" + let position = afterColumn.map { "AFTER \(quoteIdentifier($0))" } ?? "FIRST" + /// The same builder `ADD COLUMN` uses, rather than the attribute list alone. `MODIFY` + /// replaces the whole definition, and the attribute list does not carry + /// `GENERATED ALWAYS AS`, so moving a generated column with it dropped the expression and + /// left a plain column of stored defaults behind. + return "ALTER TABLE \(tableName) MODIFY COLUMN \(buildColumnDefinitionSQL(column)) \(position)" + } + + /// `MODIFY COLUMN` replaces the whole definition, so every move restates the column in full. + /// Restating only the type is what drops charset, collation and `ON UPDATE`. + func generateColumnReorderPlan( + table: String, + schema: String?, + columns: [PluginColumnDefinition], + desiredOrder: [String] + ) async throws -> PluginColumnReorderPlan? { + let byName = Dictionary(columns.map { ($0.name, $0) }, uniquingKeysWith: { first, _ in first }) + let statements = PluginColumnReorderPlanner + .moves(from: columns.map(\.name), to: desiredOrder) + .compactMap { move -> String? in + guard let column = byName[move.column] else { return nil } + return generateMoveColumnSQL(table: table, column: column, afterColumn: move.afterColumn) + } + guard !statements.isEmpty else { return nil } + return PluginColumnReorderPlan(statements: statements, cost: .metadataOnly) } // MARK: - View Templates diff --git a/Plugins/OracleDriverPlugin/OraclePlugin.swift b/Plugins/OracleDriverPlugin/OraclePlugin.swift index 0f698b023..8de9e16cd 100644 --- a/Plugins/OracleDriverPlugin/OraclePlugin.swift +++ b/Plugins/OracleDriverPlugin/OraclePlugin.swift @@ -1034,6 +1034,48 @@ final class OraclePluginDriver: PluginDatabaseDriver, @unchecked Sendable { "ALTER TABLE \(oracleQualifiedTable(table)) DROP COLUMN \(quoteIdentifier(columnName))" } + /// Oracle has no positional clause, but making a column invisible and visible again moves it to + /// the end of the visible order, so any order is reachable by appending the right suffix. + /// + /// Measured against Oracle Free 23: the cycle works on the primary key, on an identity column + /// and on a virtual column; the rows, the default, the NOT NULL, the comment, the identity + /// sequence, the constraints, the indexes and the foreign keys pointing at the table all + /// survive it, and no data is read or written. Needs 12.1, where invisible columns arrived; an + /// older server rejects the statement and the error is reported as it is. + /// + /// The two halves of a cycle are separate statements because Oracle commits each DDL on its + /// own, so a column is invisible for the width of one statement. Cycling one column at a time + /// keeps that window as small as it can be. + func generateColumnReorderPlan( + table: String, + schema: String?, + columns: [PluginColumnDefinition], + desiredOrder: [String] + ) async throws -> PluginColumnReorderPlan? { + let qt = oracleQualifiedTable(table) + let currentOrder = try await fetchColumns(table: table, schema: schema).map(\.name) + let cycled = PluginColumnReorderPlanner.appendCycle(from: currentOrder, to: desiredOrder) + let statements = cycled.flatMap { column -> [String] in + let quoted = quoteIdentifier(column) + return [ + "ALTER TABLE \(qt) MODIFY (\(quoted) INVISIBLE)", + "ALTER TABLE \(qt) MODIFY (\(quoted) VISIBLE)" + ] + } + guard !statements.isEmpty else { return nil } + + /// Oracle commits each DDL statement on its own, so there is no transaction to roll back: + /// a cycle whose `VISIBLE` half fails, on a dropped connection or a server error, leaves + /// that column hidden for good. Every cycled column gets a compensating `VISIBLE` that the + /// executor runs on any mid-plan failure, which is idempotent on a column that is already + /// visible and puts back the one that is not. + return PluginColumnReorderPlan( + statements: statements, + compensation: cycled.map { "ALTER TABLE \(qt) MODIFY (\(quoteIdentifier($0)) VISIBLE)" }, + cost: .metadataOnly + ) + } + func generateAddIndexSQL(table: String, index: PluginIndexDefinition) -> String? { oracleIndexDefinition(index, qualifiedTable: oracleQualifiedTable(table)) } diff --git a/Plugins/PostgreSQLDriverPlugin/PostgreSQLPluginDriver+ColumnReorder.swift b/Plugins/PostgreSQLDriverPlugin/PostgreSQLPluginDriver+ColumnReorder.swift new file mode 100644 index 000000000..399a4b5b4 --- /dev/null +++ b/Plugins/PostgreSQLDriverPlugin/PostgreSQLPluginDriver+ColumnReorder.swift @@ -0,0 +1,355 @@ +// +// PostgreSQLPluginDriver+ColumnReorder.swift +// TablePro +// + +import Foundation +import TableProPluginKit + +extension PostgreSQLPluginDriver { + /// PostgreSQL stores column order as `pg_attribute.attnum` and offers nothing that changes it, + /// so the order changes by recreating the table and copying its rows. + /// + /// TablePro writes the script and does not run it. The catalog describes the table's columns, + /// constraints, indexes, triggers and comments, all through server functions that produce + /// canonical text, but it does not hand back everything a table can carry: the caveats name + /// what a rebuild leaves behind. Running that behind a button would report success over a lost + /// grant or a policy that no longer applies, so the script goes to the user instead. + func generateColumnReorderPlan( + table: String, + schema: String?, + columns: [PluginColumnDefinition], + desiredOrder: [String] + ) async throws -> PluginColumnReorderPlan? { + let resolvedSchema = schema ?? core.currentSchema + let parts = try await fetchRebuildParts(table: table, schema: resolvedSchema) + guard !parts.columnDefinitions.isEmpty else { return nil } + guard parts.columnNames != desiredOrder, + Set(parts.columnNames) == Set(desiredOrder), + parts.columnNames.count == desiredOrder.count else { return nil } + + let qualified = "\(quoteIdentifier(resolvedSchema)).\(quoteIdentifier(table))" + let staging = "\(quoteIdentifier(resolvedSchema)).\(quoteIdentifier("\(table)_tablepro_reorder"))" + let copyList = parts.copyableColumns.map { quoteIdentifier($0) }.joined(separator: ", ") + + let body = desiredOrder.compactMap { parts.columnDefinitions[$0] } + + /// The order here is the whole difficulty, and every step of it was measured against + /// PostgreSQL 17. The old table is renamed rather than dropped, so a foreign key in another + /// table keeps pointing at real rows while the copy runs. But a rename moves nothing else: + /// the staging table still owns every index name and every constraint name the original + /// had, and both live in the schema rather than on the table. Declaring the constraints + /// inside the `CREATE TABLE` therefore silently renames them, which shipped as `x_pkey1`, + /// `x_a_b_key1` and `x_c_check1`; creating an index before the staging table goes fails + /// outright with "relation already exists". So nothing that carries a name is created until + /// the staging table is dropped, and the staging table cannot be dropped until every + /// inbound foreign key has let go of it. + var statements: [String] = [] + statements.append("ALTER TABLE \(qualified) RENAME TO \(quoteIdentifier("\(table)_tablepro_reorder"))") + statements.append("CREATE TABLE \(qualified) (\n " + body.joined(separator: ",\n ") + "\n)") + /// `OVERRIDING SYSTEM VALUE` unconditionally. A `GENERATED ALWAYS AS IDENTITY` column + /// refuses a written value without it and takes the whole rebuild down; measured, the + /// clause is accepted and does nothing on a `BY DEFAULT` identity and on a table that has + /// no identity column at all. + statements.append(""" + INSERT INTO \(qualified) (\(copyList)) OVERRIDING SYSTEM VALUE SELECT \(copyList) FROM \(staging) + """) + statements.append(contentsOf: parts.identityResets(qualified: qualified, quote: quoteIdentifier)) + statements.append(contentsOf: parts.inboundForeignKeyDrops) + /// A `serial` column's default still calls the sequence the staging table owns, so `DROP + /// TABLE` tries to take that sequence with it and PostgreSQL refuses, rolling the whole + /// script back. Measured: handing ownership to the rebuilt table first lets the drop + /// through, and the sequence keeps its original name. + statements.append(contentsOf: parts.serialSequenceHandovers) + statements.append("DROP TABLE \(staging)") + statements.append(contentsOf: parts.tableConstraints.map { "ALTER TABLE \(qualified) ADD \($0)" }) + statements.append(contentsOf: parts.outboundForeignKeys.map { "ALTER TABLE \(qualified) ADD \($0)" }) + statements.append(contentsOf: parts.inboundForeignKeyAdds) + statements.append(contentsOf: parts.indexes) + statements.append(contentsOf: parts.triggers) + statements.append(contentsOf: parts.triggerModes) + statements.append(contentsOf: parts.comments) + + return PluginColumnReorderPlan( + statements: statements, + isTransactional: true, + cost: .tableRebuild, + caveats: parts.dependentViewCaveat + [ + String(localized: "Grants, row-level security policies, publications, extended statistics, partitioning and table inheritance are not carried over."), + String(localized: "A column collation that differs from its type default is not reproduced."), + String(localized: "An identity column keeps its value, but its sequence is recreated under a new name because the old table still holds the original name when the new one is created.") + ], + isRunnable: false + ) + } + + private struct RebuildParts { + var columnNames: [String] = [] + var columnDefinitions: [String: String] = [:] + var copyableColumns: [String] = [] + var identityColumns: [String] = [] + var tableConstraints: [String] = [] + var outboundForeignKeys: [String] = [] + var inboundForeignKeyDrops: [String] = [] + var inboundForeignKeyAdds: [String] = [] + var indexes: [String] = [] + var triggers: [String] = [] + var triggerModes: [String] = [] + var comments: [String] = [] + var dependentViews: [String] = [] + var serialSequenceHandovers: [String] = [] + + /// PostgreSQL binds a view to the table's OID, not its name, so a view follows the rename + /// onto the staging table and then refuses to let it be dropped. Measured: the rebuild + /// stops at `DROP TABLE` with "cannot drop table … because other objects depend on it" and + /// the whole transaction rolls back. Naming them here is what stops that being discovered + /// three quarters of the way through the script. + var dependentViewCaveat: [String] { + guard !dependentViews.isEmpty else { return [] } + return [ + String( + format: String( + localized: "Drop and recreate these views first, or the script stops when it drops the old table: %@." + ), + dependentViews.joined(separator: ", ") + ) + ] + } + + /// A new identity column starts its sequence at one, so it is wound forward to the rows the + /// copy just wrote. Without this the next insert collides with an existing key. + /// + /// Both arguments are escaped. A schema or table name may legally contain an apostrophe, + /// and it lands inside a single-quoted literal here. + func identityResets(qualified: String, quote: (String) -> String) -> [String] { + identityColumns.map { column in + """ + SELECT setval( + pg_get_serial_sequence('\(literal(qualified))', '\(literal(column))'), + GREATEST(COALESCE((SELECT MAX(\(quote(column))) FROM \(qualified)), 0), 1), + true + ) + """ + } + } + + private func literal(_ value: String) -> String { + value.replacingOccurrences(of: "'", with: "''") + } + } + + private func fetchRebuildParts(table: String, schema: String) async throws -> RebuildParts { + let safeTable = escapeLiteral(table) + let safeSchema = escapeLiteral(schema) + let caps = versionedCapabilities + var parts = RebuildParts() + + let identityClause = caps.hasIdentityColumns ? """ + CASE + WHEN a.attidentity = 'a' THEN ' GENERATED ALWAYS AS IDENTITY' + WHEN a.attidentity = 'd' THEN ' GENERATED BY DEFAULT AS IDENTITY' + ELSE '' + END || + """ : "" + let generatedClause = caps.hasGeneratedColumns ? """ + CASE + WHEN a.attgenerated = 's' THEN ' GENERATED ALWAYS AS (' || pg_get_expr(d.adbin, d.adrelid) || ') STORED' + WHEN a.attgenerated = 'v' THEN ' GENERATED ALWAYS AS (' || pg_get_expr(d.adbin, d.adrelid) || ') VIRTUAL' + ELSE '' + END || + """ : "" + let defaultGuard = [ + caps.hasIdentityColumns ? "AND a.attidentity = ''" : "", + caps.hasGeneratedColumns ? "AND a.attgenerated = ''" : "" + ].filter { !$0.isEmpty }.joined(separator: " ") + let identityFlag = caps.hasIdentityColumns ? "a.attidentity <> ''" : "false" + let generatedFlag = caps.hasGeneratedColumns ? "a.attgenerated <> ''" : "false" + + let columnRows = try await execute(query: """ + SELECT + a.attname, + quote_ident(a.attname) || ' ' || format_type(a.atttypid, a.atttypmod) || + \(identityClause) + \(generatedClause) + CASE WHEN a.attnotnull THEN ' NOT NULL' ELSE '' END || + CASE + WHEN a.atthasdef \(defaultGuard) + THEN ' DEFAULT ' || pg_get_expr(d.adbin, d.adrelid) + ELSE '' + END, + \(identityFlag), + \(generatedFlag) + FROM pg_attribute a + JOIN pg_class c ON c.oid = a.attrelid + JOIN pg_namespace n ON n.oid = c.relnamespace + LEFT JOIN pg_attrdef d ON d.adrelid = c.oid AND d.adnum = a.attnum + WHERE c.relname = '\(safeTable)' AND n.nspname = '\(safeSchema)' + AND a.attnum > 0 AND NOT a.attisdropped + ORDER BY a.attnum + """).rows + + for row in columnRows { + guard let name = row[safe: 0]?.asText, let definition = row[safe: 1]?.asText else { continue } + parts.columnNames.append(name) + parts.columnDefinitions[name] = definition + if isTrue(row[safe: 2]?.asText) { parts.identityColumns.append(name) } + /// A generated column is computed, never written, so `INSERT` refuses it by name. + if !isTrue(row[safe: 3]?.asText) { parts.copyableColumns.append(name) } + } + + /// Named, and added after the staging table is gone. Declared inline instead, PostgreSQL + /// finds the name already taken and quietly picks another. + parts.tableConstraints = try await textRows(""" + SELECT 'CONSTRAINT ' || quote_ident(con.conname) || ' ' || pg_get_constraintdef(con.oid, true) + FROM pg_constraint con + JOIN pg_class c ON c.oid = con.conrelid + JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE c.relname = '\(safeTable)' AND n.nspname = '\(safeSchema)' + AND con.contype IN ('p', 'u', 'c', 'x') + ORDER BY CASE con.contype WHEN 'p' THEN 0 WHEN 'u' THEN 1 ELSE 2 END, con.conname + """) + + parts.outboundForeignKeys = try await textRows(""" + SELECT 'CONSTRAINT ' || quote_ident(con.conname) || ' ' || pg_get_constraintdef(con.oid, true) + FROM pg_constraint con + JOIN pg_class c ON c.oid = con.conrelid + JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE c.relname = '\(safeTable)' AND n.nspname = '\(safeSchema)' AND con.contype = 'f' + ORDER BY con.conname + """) + + /// A key in another table follows the rename, so it now points at the staging table and is + /// the only thing keeping it alive. Dropping every one is what lets the staging table go; + /// re-adding them against the rebuilt table happens once its primary key is back. + let inboundClause = """ + FROM pg_constraint con + JOIN pg_class c ON c.oid = con.confrelid + JOIN pg_namespace n ON n.oid = c.relnamespace + JOIN pg_class c2 ON c2.oid = con.conrelid + JOIN pg_namespace n2 ON n2.oid = c2.relnamespace + WHERE c.relname = '\(safeTable)' AND n.nspname = '\(safeSchema)' AND con.contype = 'f' + AND con.conrelid <> con.confrelid + ORDER BY con.conname + """ + parts.inboundForeignKeyDrops = try await textRows(""" + SELECT 'ALTER TABLE ' || quote_ident(n2.nspname) || '.' || quote_ident(c2.relname) + || ' DROP CONSTRAINT ' || quote_ident(con.conname) + \(inboundClause) + """) + parts.inboundForeignKeyAdds = try await textRows(""" + SELECT 'ALTER TABLE ' || quote_ident(n2.nspname) || '.' || quote_ident(c2.relname) + || ' ADD CONSTRAINT ' || quote_ident(con.conname) || ' ' || pg_get_constraintdef(con.oid, true) + \(inboundClause) + """) + + /// The indexes a constraint owns come back with the constraint, so listing them again would + /// fail on a duplicate name. + parts.indexes = try await textRows(""" + SELECT indexdef FROM pg_indexes + WHERE tablename = '\(safeTable)' AND schemaname = '\(safeSchema)' + AND indexname NOT IN ( + SELECT con.conname FROM pg_constraint con + JOIN pg_class c ON c.oid = con.conrelid + JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE c.relname = '\(safeTable)' AND n.nspname = '\(safeSchema)' + ) + ORDER BY indexname + """) + + parts.triggers = try await textRows(""" + SELECT pg_get_triggerdef(t.oid, true) + FROM pg_trigger t + JOIN pg_class c ON c.oid = t.tgrelid + JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE c.relname = '\(safeTable)' AND n.nspname = '\(safeSchema)' AND NOT t.tgisinternal + ORDER BY t.tgname + """) + + /// `pg_get_triggerdef` writes the definition and nothing about whether the trigger is + /// firing, so a recreated one comes back ordinarily enabled however it was left. A trigger + /// the user disabled, or set to fire only on a replica or always, silently starts firing on + /// writes it was excluded from. + parts.triggerModes = try await textRows(""" + SELECT 'ALTER TABLE ' || quote_ident(n.nspname) || '.' || quote_ident(c.relname) + || CASE t.tgenabled + WHEN 'D' THEN ' DISABLE TRIGGER ' + WHEN 'R' THEN ' ENABLE REPLICA TRIGGER ' + WHEN 'A' THEN ' ENABLE ALWAYS TRIGGER ' + END + || quote_ident(t.tgname) + FROM pg_trigger t + JOIN pg_class c ON c.oid = t.tgrelid + JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE c.relname = '\(safeTable)' AND n.nspname = '\(safeSchema)' AND NOT t.tgisinternal + AND t.tgenabled <> 'O' + ORDER BY t.tgname + """) + + /// A `serial` column, unlike an identity one, owns a sequence the rebuilt table's default + /// still calls. The server writes the whole statement so no identifier has to be quoted or + /// escaped here. + parts.serialSequenceHandovers = try await textRows(""" + SELECT 'ALTER SEQUENCE ' || pg_get_serial_sequence( + quote_ident(n.nspname) || '.' || quote_ident(c.relname), a.attname + ) + || ' OWNED BY ' || quote_ident(n.nspname) || '.' || quote_ident(c.relname) + || '.' || quote_ident(a.attname) + FROM pg_attribute a + JOIN pg_class c ON c.oid = a.attrelid + JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE c.relname = '\(safeTable)' AND n.nspname = '\(safeSchema)' + AND a.attnum > 0 AND NOT a.attisdropped + AND \(caps.hasIdentityColumns ? "a.attidentity = ''" : "true") + AND pg_get_serial_sequence( + quote_ident(n.nspname) || '.' || quote_ident(c.relname), a.attname + ) IS NOT NULL + ORDER BY a.attnum + """) + + parts.comments = try await textRows(""" + SELECT 'COMMENT ON TABLE ' || quote_ident(n.nspname) || '.' || quote_ident(c.relname) + || ' IS ' || quote_literal(obj_description(c.oid, 'pg_class')) + FROM pg_class c + JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE c.relname = '\(safeTable)' AND n.nspname = '\(safeSchema)' + AND obj_description(c.oid, 'pg_class') IS NOT NULL + UNION ALL + SELECT 'COMMENT ON COLUMN ' || quote_ident(n.nspname) || '.' || quote_ident(c.relname) + || '.' || quote_ident(a.attname) + || ' IS ' || quote_literal(col_description(c.oid, a.attnum)) + FROM pg_attribute a + JOIN pg_class c ON c.oid = a.attrelid + JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE c.relname = '\(safeTable)' AND n.nspname = '\(safeSchema)' + AND a.attnum > 0 AND NOT a.attisdropped + AND col_description(c.oid, a.attnum) IS NOT NULL + """) + + parts.dependentViews = try await textRows(""" + SELECT DISTINCT quote_ident(dn.nspname) || '.' || quote_ident(dc.relname) + FROM pg_depend d + JOIN pg_rewrite r ON r.oid = d.objid + JOIN pg_class dc ON dc.oid = r.ev_class + JOIN pg_namespace dn ON dn.oid = dc.relnamespace + JOIN pg_class c ON c.oid = d.refobjid + JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE c.relname = '\(safeTable)' AND n.nspname = '\(safeSchema)' + AND dc.relkind IN ('v', 'm') + AND dc.oid <> c.oid + ORDER BY 1 + """) + + return parts + } + + private func textRows(_ query: String) async throws -> [String] { + try await execute(query: query).rows.compactMap { $0[safe: 0]?.asText } + } + + /// libpq reports a boolean as `t` on the text protocol and the driver may hand it back either + /// way, so both spellings are accepted rather than one being assumed. + private func isTrue(_ value: String?) -> Bool { + guard let value else { return false } + return value == "t" || value.lowercased() == "true" + } +} diff --git a/Plugins/SQLiteDriverPlugin/SQLitePlugin.swift b/Plugins/SQLiteDriverPlugin/SQLitePlugin.swift index 30f4e26e0..3a4cb15f6 100644 --- a/Plugins/SQLiteDriverPlugin/SQLitePlugin.swift +++ b/Plugins/SQLiteDriverPlugin/SQLitePlugin.swift @@ -1217,6 +1217,33 @@ final class SQLitePluginDriver: PluginDatabaseDriver, @unchecked Sendable { "ALTER TABLE \(quoteIdentifier(table)) DROP COLUMN \(quoteIdentifier(columnName))" } + /// SQLite has no positional `ALTER`, so the order changes by rebuilding the table. + /// + /// The new table is written by moving the original column definitions as text inside the + /// statement SQLite stored, so a `CHECK`, a `COLLATE`, a `GENERATED ALWAYS AS` and a `DEFAULT` + /// with a comma in it all come through untouched. Re-rendering them from `PRAGMA table_info` + /// would lose every one, because the pragma does not report them. + func generateColumnReorderPlan( + table: String, + schema: String?, + columns: [PluginColumnDefinition], + desiredOrder: [String] + ) async throws -> PluginColumnReorderPlan? { + try await SQLiteColumnReorderPlanner.plan( + tableName: table, + desiredOrder: desiredOrder, + isRunnable: true, + execute: { try await self.execute(query: $0) } + ) + } + + func columnReorderSchemaFingerprint(table: String, schema: String?) async throws -> String? { + try await SQLiteColumnReorderPlanner.schemaFingerprint( + tableName: table, + execute: { try await self.execute(query: $0) } + ) + } + /// ADD/DROP CONSTRAINT arrived in SQLite 3.53.0. Returning nil below that version makes /// `SchemaStatementGenerator` refuse the change with "Unsupported schema operation" rather than /// sending a statement the linked library cannot parse. diff --git a/Plugins/TableProPluginKit/ColumnReorderTypes.swift b/Plugins/TableProPluginKit/ColumnReorderTypes.swift new file mode 100644 index 000000000..8936cdc68 --- /dev/null +++ b/Plugins/TableProPluginKit/ColumnReorderTypes.swift @@ -0,0 +1,170 @@ +// +// ColumnReorderTypes.swift +// TableProPluginKit +// + +import Foundation + +/// What running a column reorder costs, which is what decides whether the user is asked first. +/// +/// The line is not how many statements there are. A positional `ALTER` and Oracle's +/// invisible/visible cycle both touch catalog rows alone, so they run on the drop the way every +/// other direct manipulation does. A rebuild copies every row into a new table and drops the +/// original, so it is presented for review and confirmed before anything runs. +public enum PluginColumnReorderCost: Sendable, Equatable { + case metadataOnly + case tableRebuild +} + +/// The statements that put a table's columns into a wanted order. +/// +/// A plan carries DDL and nothing else: no `BEGIN`, no `COMMIT`, no `ROLLBACK`. Whoever runs it +/// owns the transaction, because both places that can are already opening one. TablePro's own +/// execution path does it through `DatabaseDriver.beginTransaction`, and the query editor's Run All +/// wraps a multi-statement script the same way, so a plan that spelled the transaction out in SQL +/// would nest inside theirs and fail. +public struct PluginColumnReorderPlan: Sendable, Equatable { + /// The DDL, in order, to run inside that transaction. + public let statements: [String] + + /// Run before the transaction opens, and after it closes. SQLite's `foreign_keys` pragma is the + /// case: it is silently ignored inside a transaction, so it cannot travel with the rest. + public let prologue: [String] + public let epilogue: [String] + + /// Undoes what has already run, for an engine that commits each DDL statement on its own and so + /// has no transaction to roll back. Oracle's invisible/visible cycle is the case: a cycle whose + /// second half fails leaves a column hidden, and only a compensating statement brings it back. + public let compensation: [String] + + /// Whether the statements need a transaction around them at all. False for a plan whose + /// statements each stand alone, where opening one would only widen the window. + public let isTransactional: Bool + + public let cost: PluginColumnReorderCost + + /// What the plan does not carry over, phrased for the user and shown before a rebuild runs. + /// A rebuild reproduces the table from what the server will describe, so anything the server + /// does not describe is named here rather than lost quietly. + public let caveats: [String] + + /// Whether TablePro may run this itself. + /// + /// False where the engine's catalog cannot describe enough of a table to reproduce it, or where + /// the transport cannot hold a transaction across the statements, so the script is handed over + /// for the user to read and run instead of sitting behind a button that would report success + /// over a lost grant or a half-applied rebuild. + public let isRunnable: Bool + + public init( + statements: [String], + prologue: [String] = [], + epilogue: [String] = [], + compensation: [String] = [], + isTransactional: Bool = false, + cost: PluginColumnReorderCost, + caveats: [String] = [], + isRunnable: Bool = true + ) { + self.statements = statements + self.prologue = prologue + self.epilogue = epilogue + self.compensation = compensation + self.isTransactional = isTransactional + self.cost = cost + self.caveats = caveats + self.isRunnable = isRunnable + } + + /// Everything the plan runs, in order, for showing the user and for handing to an editor. The + /// transaction is deliberately absent: the reader's Run All supplies it. + public var scriptStatements: [String] { prologue + statements + epilogue } +} + +/// Turns a wanted column order into the moves an engine's positional primitive can actually make. +/// +/// Kept here rather than in each driver because the arithmetic is the same everywhere and getting +/// it wrong is silent: a plan that produces the wrong order still runs and still reports success. +public enum PluginColumnReorderPlanner { + public struct Move: Sendable, Equatable { + public let column: String + /// The column this one follows once the move has run. Nil places it first. + public let afterColumn: String? + + public init(column: String, afterColumn: String?) { + self.column = column + self.afterColumn = afterColumn + } + } + + /// The fewest `FIRST` / `AFTER` moves that turn `currentOrder` into `desiredOrder`. + /// + /// A column that is not moved keeps its position relative to the others that are not moved, so + /// the largest set worth leaving alone is the longest subsequence common to both orders, and + /// everything outside it has to move exactly once. Walking the wanted order and fixing each + /// position that disagrees looks equivalent and is not: dragging a column down one place makes + /// every column it passed disagree, so it emits one statement per column passed instead of one + /// for the column the user actually dragged. + /// + /// Each move is anchored on the column that precedes it in the wanted order, which by then is + /// already in its final position, whether it was moved or left alone. + /// + /// Empty when the two orders are not permutations of each other, which is the only shape this + /// can be asked for that has no answer. + public static func moves(from currentOrder: [String], to desiredOrder: [String]) -> [Move] { + guard isPermutation(currentOrder, desiredOrder) else { return [] } + let stationary = longestCommonSubsequence(currentOrder, desiredOrder) + return desiredOrder.enumerated() + .filter { !stationary.contains($0.element) } + .map { Move(column: $0.element, afterColumn: $0.offset == 0 ? nil : desiredOrder[$0.offset - 1]) } + } + + private static func longestCommonSubsequence(_ lhs: [String], _ rhs: [String]) -> Set { + var lengths = Array(repeating: Array(repeating: 0, count: rhs.count + 1), count: lhs.count + 1) + for i in stride(from: lhs.count - 1, through: 0, by: -1) { + for j in stride(from: rhs.count - 1, through: 0, by: -1) { + lengths[i][j] = lhs[i] == rhs[j] + ? lengths[i + 1][j + 1] + 1 + : max(lengths[i + 1][j], lengths[i][j + 1]) + } + } + + var common: Set = [] + var i = 0 + var j = 0 + while i < lhs.count, j < rhs.count { + if lhs[i] == rhs[j] { + common.insert(lhs[i]) + i += 1 + j += 1 + } else if lengths[i + 1][j] >= lengths[i][j + 1] { + i += 1 + } else { + j += 1 + } + } + return common + } + + /// The columns to send to the end, in order, for an engine whose only positional primitive + /// appends. Oracle's invisible/visible cycle is the case: it moves a column to the end and + /// nothing else, so any order is reachable by appending the right suffix in the right order. + /// + /// The columns left alone have to be a prefix of the wanted order and keep their current + /// relative order, so the longest such prefix is exactly the set worth not touching. + public static func appendCycle(from currentOrder: [String], to desiredOrder: [String]) -> [String] { + guard isPermutation(currentOrder, desiredOrder) else { return [] } + var kept = 0 + var cursor = currentOrder.startIndex + for name in desiredOrder { + guard let found = currentOrder[cursor...].firstIndex(of: name) else { break } + cursor = currentOrder.index(after: found) + kept += 1 + } + return Array(desiredOrder.dropFirst(kept)) + } + + private static func isPermutation(_ lhs: [String], _ rhs: [String]) -> Bool { + lhs.count == rhs.count && lhs.sorted() == rhs.sorted() + } +} diff --git a/Plugins/TableProPluginKit/PluginDatabaseDriver.swift b/Plugins/TableProPluginKit/PluginDatabaseDriver.swift index 762816040..b1d1caeeb 100644 --- a/Plugins/TableProPluginKit/PluginDatabaseDriver.swift +++ b/Plugins/TableProPluginKit/PluginDatabaseDriver.swift @@ -207,6 +207,34 @@ public protocol PluginDatabaseDriver: AnyObject, Sendable { func generateRenameCheckConstraintSQL(table: String, from oldName: String, to newName: String) -> String? func generateModifyPrimaryKeySQL(table: String, oldColumns: [String], newColumns: [String], constraintName: String?) -> [String]? func generateMoveColumnSQL(table: String, column: PluginColumnDefinition, afterColumn: String?) -> String? + + /// The statements that put `table`'s columns into `desiredOrder`, or nil where the engine + /// cannot reorder them. + /// + /// Supersedes `generateMoveColumnSQL`, which can only say "one `ALTER`, one column" and so + /// cannot express Oracle's invisible/visible cycle or the create-copy-swap a rebuild engine + /// needs. The old requirement stays published and defaulted: removing one breaks every plugin + /// whose witness table hard-references its default. + /// + /// `columns` is the table's current definitions in current order, so a driver that has to + /// restate a column keeps the charset and collation the app already resolved. Anything else a + /// rebuild needs, the driver queries for itself. + func generateColumnReorderPlan( + table: String, + schema: String?, + columns: [PluginColumnDefinition], + desiredOrder: [String] + ) async throws -> PluginColumnReorderPlan? + + /// A fingerprint of everything a reorder plan reproduces, cheap enough to take twice. + /// + /// A rebuild plan is built before its review sheet opens and run after it closes, and it ends + /// in a `DROP`. Anything another connection added in between is inside the table the plan is + /// about to drop and outside the plan that is about to replace it. Comparing this before and + /// after is what turns that into a refusal instead of silent loss. Nil where the driver cannot + /// answer, which stands the check down for an engine TablePro never runs a rebuild on anyway. + func columnReorderSchemaFingerprint(table: String, schema: String?) async throws -> String? + func generateCreateTableSQL(definition: PluginCreateTableDefinition) -> String? // Definition SQL for clipboard copy (optional — return nil if not supported) @@ -533,6 +561,16 @@ public extension PluginDatabaseDriver { func generateRenameCheckConstraintSQL(table: String, from oldName: String, to newName: String) -> String? { nil } func generateModifyPrimaryKeySQL(table: String, oldColumns: [String], newColumns: [String], constraintName: String?) -> [String]? { nil } func generateMoveColumnSQL(table: String, column: PluginColumnDefinition, afterColumn: String?) -> String? { nil } + + func generateColumnReorderPlan( + table: String, + schema: String?, + columns: [PluginColumnDefinition], + desiredOrder: [String] + ) async throws -> PluginColumnReorderPlan? { nil } + + func columnReorderSchemaFingerprint(table: String, schema: String?) async throws -> String? { nil } + func generateCreateTableSQL(definition: PluginCreateTableDefinition) -> String? { nil } func generateColumnDefinitionSQL(column: PluginColumnDefinition) -> String? { nil } diff --git a/Plugins/TableProPluginKit/SQLiteColumnReorderPlanner.swift b/Plugins/TableProPluginKit/SQLiteColumnReorderPlanner.swift new file mode 100644 index 000000000..c60f7e701 --- /dev/null +++ b/Plugins/TableProPluginKit/SQLiteColumnReorderPlanner.swift @@ -0,0 +1,168 @@ +// +// SQLiteColumnReorderPlanner.swift +// TableProPluginKit +// + +import Foundation + +/// Builds SQLite's documented table-rebuild script for a column reorder. +/// +/// SQLite has no positional `ALTER`, so the order changes by creating the table again in the wanted +/// order, copying the rows into it, dropping the original and renaming. This is the procedure +/// SQLite's own `ALTER TABLE` documentation prescribes, in its order, and it is shared by every +/// SQLite-derived driver. +/// +/// Measured against 3.54 with a table carrying a generated column, a table `CHECK`, a `COLLATE`, a +/// `DEFAULT` containing a comma, a `DECIMAL(10,2)`, an index, a trigger, an outbound foreign key +/// and two dependent views: every one of them survives, and no `PRAGMA legacy_alter_table` is +/// needed for the rename to pass the views. +public enum SQLiteColumnReorderPlanner { + /// - Parameters: + /// - createTableSQL: the statement SQLite stored for this table, from `sqlite_master.sql`. + /// - copyableColumns: the columns to carry over, in the table's current order, with the + /// generated ones left out. `INSERT` refuses a generated column, so listing one fails the + /// whole rebuild. + /// - dependentObjectSQL: the `CREATE INDEX` and `CREATE TRIGGER` statements `DROP TABLE` + /// takes with it, replayed after the rename. + /// - autoincrementHighWaterMark: the table's `sqlite_sequence` value, for an `AUTOINCREMENT` + /// table. Nil for every other table. + /// - foreignKeysWereOn: what `PRAGMA foreign_keys` read before the rebuild, so the epilogue + /// puts it back rather than forcing it on. + /// - isRunnable: false for a driver whose connection cannot hold a transaction across + /// statements, which is every HTTP-backed one. + public static func plan( + tableName: String, + createTableSQL: String, + desiredOrder: [String], + copyableColumns: [String], + dependentObjectSQL: [String], + autoincrementHighWaterMark: Int64?, + foreignKeysWereOn: Bool, + isRunnable: Bool + ) -> PluginColumnReorderPlan? { + guard let parsed = SQLiteTableDDL.parse(createTableSQL: createTableSQL) else { return nil } + guard parsed.columnNames != desiredOrder else { return nil } + + let temporaryName = "\(tableName)_tablepro_reorder" + guard let createNew = SQLiteTableDDL.reordered(parsed, to: desiredOrder, tableName: temporaryName) else { + return nil + } + + let quotedOriginal = SQLiteTableDDL.quote(tableName) + let quotedTemporary = SQLiteTableDDL.quote(temporaryName) + let columnList = copyableColumns.map(SQLiteTableDDL.quote).joined(separator: ", ") + + var statements = [ + createNew, + "INSERT INTO \(quotedTemporary) (\(columnList)) SELECT \(columnList) FROM \(quotedOriginal)", + "DROP TABLE \(quotedOriginal)", + "ALTER TABLE \(quotedTemporary) RENAME TO \(quotedOriginal)" + ] + + /// `DROP TABLE` takes the table's `sqlite_sequence` row with it, so the rebuilt table is + /// seeded from the rows that were copied rather than from the highest id ever issued. + /// Measured: a table whose last row was deleted comes back one lower and the next insert + /// reuses an id that was already handed out, which is the one thing `AUTOINCREMENT` + /// promises will not happen. + if let highWaterMark = autoincrementHighWaterMark { + statements.append(""" + UPDATE sqlite_sequence SET seq = \(highWaterMark) WHERE name = '\(escapeLiteral(tableName))' + """) + } + + statements.append(contentsOf: dependentObjectSQL) + + /// The pragma is restored to what it was rather than forced on. This driver opens + /// connections with foreign keys off, and a user can turn them off deliberately; leaving + /// them on afterwards turns later writes on the same connection from allowed into + /// constraint failures. + return PluginColumnReorderPlan( + statements: statements, + prologue: ["PRAGMA foreign_keys = off"], + epilogue: ["PRAGMA foreign_keys = \(foreignKeysWereOn ? "on" : "off")"], + isTransactional: true, + cost: .tableRebuild, + caveats: [ + String(localized: "A view that selects * from this table will return its columns in the new order.") + ], + isRunnable: isRunnable + ) + } + + internal static func escapeLiteral(_ value: String) -> String { + value.replacingOccurrences(of: "'", with: "''") + } +} + +public extension SQLiteColumnReorderPlanner { + /// Gathers what the rebuild needs from `sqlite_master` and the table's pragmas, then builds the + /// plan. Every SQLite-derived driver answers these queries identically, so they share one + /// implementation rather than carrying four copies that drift. + static func plan( + tableName: String, + desiredOrder: [String], + isRunnable: Bool, + execute: (String) async throws -> PluginQueryResult + ) async throws -> PluginColumnReorderPlan? { + let literal = escapeLiteral(tableName) + let quoted = SQLiteTableDDL.quote(tableName) + + let createSQL = try await execute(""" + SELECT sql FROM sqlite_master WHERE type = 'table' AND name = '\(literal)' + """).rows.first?[safe: 0]?.asText + guard let createSQL else { return nil } + + /// `table_xinfo` rather than `table_info`, which omits a generated column entirely. + let columns = try await execute("PRAGMA table_xinfo(\(quoted))").rows + let copyable = columns.compactMap { row -> String? in + guard let name = row[safe: 1]?.asText else { return nil } + let hidden = row[safe: 6]?.asText.flatMap { Int($0) } ?? 0 + return hidden == 0 ? name : nil + } + + /// The indexes and triggers `DROP TABLE` takes with it. An auto-index backing a `UNIQUE` or + /// `PRIMARY KEY` has no `sql` of its own and comes back with the table. + let dependents = try await execute(""" + SELECT sql FROM sqlite_master + WHERE tbl_name = '\(literal)' AND type IN ('index', 'trigger') AND sql IS NOT NULL + ORDER BY type, name + """).rows.compactMap { $0[safe: 0]?.asText } + + var highWaterMark: Int64? + if createSQL.uppercased().contains("AUTOINCREMENT") { + highWaterMark = try await execute(""" + SELECT seq FROM sqlite_sequence WHERE name = '\(literal)' + """).rows.first?[safe: 0]?.asText.flatMap { Int64($0) } + } + + let foreignKeysWereOn = (try await execute("PRAGMA foreign_keys") + .rows.first?[safe: 0]?.asText).map { $0 == "1" || $0.lowercased() == "true" } ?? false + + return plan( + tableName: tableName, + createTableSQL: createSQL, + desiredOrder: desiredOrder, + copyableColumns: copyable, + dependentObjectSQL: dependents, + autoincrementHighWaterMark: highWaterMark, + foreignKeysWereOn: foreignKeysWereOn, + isRunnable: isRunnable + ) + } + + /// A fingerprint of everything the rebuild reproduces, so a plan built before a review sheet + /// opened can be checked against the database before it drops anything. + static func schemaFingerprint( + tableName: String, + execute: (String) async throws -> PluginQueryResult + ) async throws -> String { + let literal = escapeLiteral(tableName) + return try await execute(""" + SELECT group_concat(type || ':' || name || ':' || coalesce(sql, ''), '\u{1}') + FROM ( + SELECT type, name, sql FROM sqlite_master + WHERE tbl_name = '\(literal)' ORDER BY type, name + ) + """).rows.first?[safe: 0]?.asText ?? "" + } +} diff --git a/Plugins/TableProPluginKit/SQLiteTableDDL.swift b/Plugins/TableProPluginKit/SQLiteTableDDL.swift new file mode 100644 index 000000000..9f8af29ca --- /dev/null +++ b/Plugins/TableProPluginKit/SQLiteTableDDL.swift @@ -0,0 +1,289 @@ +// +// SQLiteTableDDL.swift +// TableProPluginKit +// + +import Foundation + +/// Reads and rewrites the `CREATE TABLE` text SQLite stores for a table. +/// +/// SQLite keeps the statement the user wrote, verbatim, in `sqlite_master.sql`. Reordering a +/// table's columns by moving those definitions as text is the only way to keep everything they +/// carry: a `CHECK`, a `COLLATE`, a `GENERATED ALWAYS AS`, a `DEFAULT 'a, b'` with a comma inside +/// it. Re-rendering a column from `PRAGMA table_info` instead loses all four, because the pragma +/// does not report them. +/// +/// Shared by every SQLite-derived driver. libSQL, Turso and Cloudflare D1 all answer the same +/// `sqlite_master` query, so they get the same rewrite rather than three copies of it. +public enum SQLiteTableDDL { + /// One top-level entry inside the parentheses of a `CREATE TABLE`. + public struct Entry: Sendable, Equatable { + /// The entry's source text, exactly as SQLite stored it. + public let text: String + /// The column this entry defines, or nil for a table constraint. + public let columnName: String? + + public init(text: String, columnName: String?) { + self.text = text + self.columnName = columnName + } + } + + /// A parsed `CREATE TABLE`, kept as the three pieces a reorder needs to reassemble it. + public struct Parsed: Sendable, Equatable { + public let prefix: String + public let entries: [Entry] + public let suffix: String + + public init(prefix: String, entries: [Entry], suffix: String) { + self.prefix = prefix + self.entries = entries + self.suffix = suffix + } + + public var columnNames: [String] { entries.compactMap(\.columnName) } + } + + private static let tableConstraintKeywords: Set = [ + "CONSTRAINT", "PRIMARY", "UNIQUE", "CHECK", "FOREIGN" + ] + + /// Splits `CREATE TABLE x (…) WITHOUT ROWID` into its prefix, its top-level entries and its + /// trailing options. + /// + /// Nil for anything that is not an ordinary table, because a rebuild of one of those destroys + /// it. `sqlite_master` stores an FTS5 table as `CREATE VIRTUAL TABLE docs USING fts5(title, + /// body)`, whose parentheses parse exactly like a column list, so accepting it would recreate + /// the table as a plain one and take the index and its shadow tables down with the `DROP`. + /// `CREATE TABLE … AS SELECT` has no column list to reorder either. + public static func parse(createTableSQL sql: String) -> Parsed? { + guard let open = topLevelBodyStart(in: sql) else { return nil } + guard isOrdinaryTable(prefix: sql[sql.startIndex.. Entry in + Entry(text: text, columnName: leadingIdentifier(of: text)) + } + guard !entries.isEmpty, entries.contains(where: { $0.columnName != nil }) else { return nil } + + return Parsed( + prefix: String(sql[sql.startIndex...open]), + entries: entries, + suffix: String(sql[close...]) + ) + } + + /// The same statement with its column definitions in `desiredOrder` and everything else where + /// it was. Nil when the wanted order is not a permutation of the columns the statement defines. + public static func reordered(_ parsed: Parsed, to desiredOrder: [String], tableName: String) -> String? { + let byName = Dictionary( + parsed.entries.compactMap { entry in entry.columnName.map { ($0, entry) } }, + uniquingKeysWith: { first, _ in first } + ) + guard byName.count == desiredOrder.count, + desiredOrder.allSatisfy({ byName[$0] != nil }) else { return nil } + + var reordered: [Entry] = [] + var columnCursor = desiredOrder.makeIterator() + for entry in parsed.entries { + guard entry.columnName != nil else { + reordered.append(entry) + continue + } + guard let next = columnCursor.next(), let replacement = byName[next] else { return nil } + reordered.append(replacement) + } + + let indent = "\n " + let body = reordered.map { $0.text.trimmingCharacters(in: .whitespacesAndNewlines) } + .joined(separator: ",\(indent)") + return "CREATE TABLE \(quote(tableName)) (\(indent)\(body)\n)\(trailingOptions(of: parsed))" + } + + public static func quote(_ identifier: String) -> String { + "\"\(identifier.replacingOccurrences(of: "\"", with: "\"\""))\"" + } + + // MARK: - Parsing + + private static func trailingOptions(of parsed: Parsed) -> String { + let trailing = parsed.suffix.dropFirst().trimmingCharacters(in: .whitespacesAndNewlines) + return trailing.isEmpty ? "" : " \(trailing)" + } + + /// Whether what stands before the column list is a plain `CREATE TABLE`. Only the keywords + /// SQLite allows there are accepted, so an unrecognised form is refused rather than rebuilt. + private static func isOrdinaryTable(prefix: Substring) -> Bool { + let words = prefix + .split(whereSeparator: { $0.isWhitespace }) + .map { $0.uppercased() } + guard let tableIndex = words.firstIndex(of: "TABLE") else { return false } + /// Only these may stand before TABLE. VIRTUAL does not, which is what rules out an FTS or + /// R-tree table whose module arguments would otherwise read as a column list. + guard words[.. String.Index? { + var scanner = Scanner(sql) + while let index = scanner.next() { + if scanner.isInsideLiteral { continue } + if sql[index] == "(" { return index } + } + return nil + } + + private static func matchingCloseParen(in sql: String, from open: String.Index) -> String.Index? { + var scanner = Scanner(sql, from: sql.index(after: open)) + var depth = 1 + while let index = scanner.next() { + if scanner.isInsideLiteral { continue } + switch sql[index] { + case "(": depth += 1 + case ")": + depth -= 1 + if depth == 0 { return index } + default: break + } + } + return nil + } + + /// Splits on the commas that separate entries, ignoring the ones inside `DECIMAL(10,2)`, a + /// `CHECK (a IN (1,2))`, a string literal or a quoted identifier. + private static func splitTopLevel(_ body: String) -> [String] { + var parts: [String] = [] + var current = body.startIndex + var depth = 0 + var scanner = Scanner(body) + while let index = scanner.next() { + if scanner.isInsideLiteral { continue } + switch body[index] { + case "(": depth += 1 + case ")": depth -= 1 + case "," where depth == 0: + parts.append(String(body[current.. String? { + let trimmed = entry.trimmingCharacters(in: .whitespacesAndNewlines) + guard let first = trimmed.first else { return nil } + + if let closing = closingQuote(for: first) { + var name = "" + var index = trimmed.index(after: trimmed.startIndex) + while index < trimmed.endIndex { + let character = trimmed[index] + if character == closing { + let next = trimmed.index(after: index) + /// A doubled quote is an escaped one, not the end of the identifier. + if next < trimmed.endIndex, trimmed[next] == closing, closing != "]" { + name.append(character) + index = trimmed.index(after: next) + continue + } + return name + } + name.append(character) + index = trimmed.index(after: index) + } + return nil + } + + let word = trimmed.prefix { $0.isLetter || $0.isNumber || $0 == "_" || $0 == "$" } + guard !word.isEmpty else { return nil } + return tableConstraintKeywords.contains(word.uppercased()) ? nil : String(word) + } + + private static func closingQuote(for opening: Character) -> Character? { + switch opening { + case "\"": "\"" + case "`": "`" + case "[": "]" + default: nil + } + } + + /// Walks a statement one character at a time, reporting whether each one sits inside a string + /// literal, a quoted identifier or a comment. Every scan here needs the same answer, so they + /// share one implementation rather than three that drift. + private struct Scanner { + private let text: String + private var index: String.Index + private var quote: Character? + private var comment: Comment? + + private enum Comment { case line, block } + + var isInsideLiteral: Bool { quote != nil || comment != nil } + + init(_ text: String, from start: String.Index? = nil) { + self.text = text + self.index = start ?? text.startIndex + } + + mutating func next() -> String.Index? { + guard index < text.endIndex else { return nil } + let current = index + let character = text[current] + index = text.index(after: current) + + switch comment { + case .line: + if character == "\n" { comment = nil } + return current + case .block: + if character == "*", index < text.endIndex, text[index] == "/" { + comment = nil + index = text.index(after: index) + } + return current + case nil: + break + } + + if let open = quote { + if character == open { + /// A doubled quote escapes itself, so it closes nothing. + if index < text.endIndex, text[index] == open, open != "]" { + index = text.index(after: index) + } else { + quote = nil + /// The closing character is part of the literal, not the text around it. + return current + } + } + return current + } + + switch character { + case "'", "\"", "`": + quote = character + case "[": + quote = "]" + case "-" where index < text.endIndex && text[index] == "-": + comment = .line + case "/" where index < text.endIndex && text[index] == "*": + comment = .block + default: + break + } + return current + } + } +} diff --git a/TablePro/Core/Plugins/PluginDriverAdapter.swift b/TablePro/Core/Plugins/PluginDriverAdapter.swift index df823305d..e1289f501 100644 --- a/TablePro/Core/Plugins/PluginDriverAdapter.swift +++ b/TablePro/Core/Plugins/PluginDriverAdapter.swift @@ -624,6 +624,27 @@ final class PluginDriverAdapter: DatabaseDriver, SchemaSwitchable, DatabaseRepor pluginDriver.generateMoveColumnSQL(table: table, column: column, afterColumn: afterColumn) } + /// Routed to the session driver rather than through `withMetadataDriver`. A rebuild plan reads + /// the catalog of the database this session is on, and a pooled driver is a second connection + /// that an embedded engine answers from a different database entirely. + func generateColumnReorderPlan( + table: String, + schema: String?, + columns: [PluginColumnDefinition], + desiredOrder: [String] + ) async throws -> PluginColumnReorderPlan? { + try await pluginDriver.generateColumnReorderPlan( + table: table, + schema: schema, + columns: columns, + desiredOrder: desiredOrder + ) + } + + func columnReorderSchemaFingerprint(table: String, schema: String?) async throws -> String? { + try await pluginDriver.columnReorderSchemaFingerprint(table: table, schema: schema) + } + func generateCreateTableSQL(definition: PluginCreateTableDefinition) -> String? { pluginDriver.generateCreateTableSQL(definition: definition) } diff --git a/TablePro/Core/Plugins/PluginManager+Registration.swift b/TablePro/Core/Plugins/PluginManager+Registration.swift index 04081972a..a131defa5 100644 --- a/TablePro/Core/Plugins/PluginManager+Registration.swift +++ b/TablePro/Core/Plugins/PluginManager+Registration.swift @@ -531,9 +531,9 @@ extension PluginManager { .capabilities.supportsSOCKSProxy ?? true } - func supportsColumnReorder(for databaseType: DatabaseType) -> Bool { + func columnReorderSupport(for databaseType: DatabaseType) -> ColumnReorderSupport { PluginMetadataRegistry.shared.snapshot(for: databaseType)? - .supportsColumnReorder ?? false + .columnReorder ?? .unsupported } func supportsDropDatabase(for databaseType: DatabaseType) -> Bool { diff --git a/TablePro/Core/Plugins/PluginMetadataRegistry+CloudDefaults.swift b/TablePro/Core/Plugins/PluginMetadataRegistry+CloudDefaults.swift index b3ffd1bd4..ad0da9f99 100644 --- a/TablePro/Core/Plugins/PluginMetadataRegistry+CloudDefaults.swift +++ b/TablePro/Core/Plugins/PluginMetadataRegistry+CloudDefaults.swift @@ -20,7 +20,6 @@ extension PluginMetadataRegistry { brandColorHex: "#4053D6", queryLanguageName: "PartiQL", editorLanguage: .sql, connectionMode: .apiOnly, supportsDatabaseSwitching: false, - supportsColumnReorder: false, capabilities: PluginMetadataSnapshot.CapabilityFlags( supportsSchemaSwitching: false, supportsImport: false, @@ -167,7 +166,6 @@ extension PluginMetadataRegistry { brandColorHex: "#4285F4", queryLanguageName: "SQL", editorLanguage: .sql, connectionMode: .apiOnly, supportsDatabaseSwitching: false, - supportsColumnReorder: false, capabilities: PluginMetadataSnapshot.CapabilityFlags( supportsSchemaSwitching: true, supportsImport: false, @@ -359,7 +357,6 @@ extension PluginMetadataRegistry { brandColorHex: "#29B5E8", queryLanguageName: "SQL", editorLanguage: .sql, connectionMode: .apiOnly, supportsDatabaseSwitching: true, - supportsColumnReorder: false, capabilities: PluginMetadataSnapshot.CapabilityFlags( supportsSchemaSwitching: true, supportsImport: true, diff --git a/TablePro/Core/Plugins/PluginMetadataRegistry+CuratedDefaults.swift b/TablePro/Core/Plugins/PluginMetadataRegistry+CuratedDefaults.swift index 00ae86da3..41dc49104 100644 --- a/TablePro/Core/Plugins/PluginMetadataRegistry+CuratedDefaults.swift +++ b/TablePro/Core/Plugins/PluginMetadataRegistry+CuratedDefaults.swift @@ -223,7 +223,7 @@ extension PluginMetadataRegistry { brandColorHex: "#FF9500", queryLanguageName: "SQL", editorLanguage: .sql, connectionMode: .network, supportsDatabaseSwitching: true, - supportsColumnReorder: true, + columnReorder: .alter, capabilities: PluginMetadataSnapshot.CapabilityFlags( supportsSchemaSwitching: false, supportsImport: true, @@ -287,7 +287,7 @@ extension PluginMetadataRegistry { brandColorHex: "#00B4D8", queryLanguageName: "SQL", editorLanguage: .sql, connectionMode: .network, supportsDatabaseSwitching: true, - supportsColumnReorder: true, + columnReorder: .alter, capabilities: PluginMetadataSnapshot.CapabilityFlags( supportsSchemaSwitching: false, supportsImport: true, @@ -352,7 +352,7 @@ extension PluginMetadataRegistry { brandColorHex: "#336791", queryLanguageName: "SQL", editorLanguage: .sql, connectionMode: .network, supportsDatabaseSwitching: true, - supportsColumnReorder: false, + columnReorder: .rebuild, capabilities: PluginMetadataSnapshot.CapabilityFlags( supportsSchemaSwitching: true, supportsImport: true, @@ -418,7 +418,6 @@ extension PluginMetadataRegistry { brandColorHex: "#205B8E", queryLanguageName: "SQL", editorLanguage: .sql, connectionMode: .network, supportsDatabaseSwitching: true, - supportsColumnReorder: false, capabilities: PluginMetadataSnapshot.CapabilityFlags( supportsSchemaSwitching: true, supportsImport: true, @@ -484,7 +483,6 @@ extension PluginMetadataRegistry { brandColorHex: "#6933FF", queryLanguageName: "SQL", editorLanguage: .sql, connectionMode: .network, supportsDatabaseSwitching: true, - supportsColumnReorder: false, capabilities: PluginMetadataSnapshot.CapabilityFlags( supportsSchemaSwitching: true, supportsImport: true, @@ -551,7 +549,6 @@ extension PluginMetadataRegistry { brandColorHex: "#F4B942", queryLanguageName: "SQL", editorLanguage: .sql, connectionMode: .network, supportsDatabaseSwitching: true, - supportsColumnReorder: false, capabilities: PluginMetadataSnapshot.CapabilityFlags( supportsSchemaSwitching: true, supportsImport: true, @@ -617,7 +614,7 @@ extension PluginMetadataRegistry { brandColorHex: "#003B57", queryLanguageName: "SQL", editorLanguage: .sql, connectionMode: .fileBased, supportsDatabaseSwitching: false, - supportsColumnReorder: false, + columnReorder: .rebuild, capabilities: PluginMetadataSnapshot.CapabilityFlags( supportsSchemaSwitching: false, supportsImport: true, diff --git a/TablePro/Core/Plugins/PluginMetadataRegistry+DuckDBDefaults.swift b/TablePro/Core/Plugins/PluginMetadataRegistry+DuckDBDefaults.swift index 9cb0347c6..a037e07ff 100644 --- a/TablePro/Core/Plugins/PluginMetadataRegistry+DuckDBDefaults.swift +++ b/TablePro/Core/Plugins/PluginMetadataRegistry+DuckDBDefaults.swift @@ -27,7 +27,6 @@ extension PluginMetadataRegistry { brandColorHex: "#FFD900", queryLanguageName: "SQL", editorLanguage: .sql, connectionMode: .apiOnly, supportsDatabaseSwitching: true, - supportsColumnReorder: false, capabilities: PluginMetadataSnapshot.CapabilityFlags( supportsSchemaSwitching: true, supportsImport: true, diff --git a/TablePro/Core/Plugins/PluginMetadataRegistry+ElasticsearchDefaults.swift b/TablePro/Core/Plugins/PluginMetadataRegistry+ElasticsearchDefaults.swift index 7ea8baf3a..027033e09 100644 --- a/TablePro/Core/Plugins/PluginMetadataRegistry+ElasticsearchDefaults.swift +++ b/TablePro/Core/Plugins/PluginMetadataRegistry+ElasticsearchDefaults.swift @@ -18,7 +18,6 @@ extension PluginMetadataRegistry { brandColorHex: "#FEC514", queryLanguageName: "Query DSL", editorLanguage: .javascript, connectionMode: .network, supportsDatabaseSwitching: false, - supportsColumnReorder: false, capabilities: PluginMetadataSnapshot.CapabilityFlags( supportsSchemaSwitching: false, supportsImport: false, diff --git a/TablePro/Core/Plugins/PluginMetadataRegistry+KafkaDefaults.swift b/TablePro/Core/Plugins/PluginMetadataRegistry+KafkaDefaults.swift index 935fd0071..13dc0b7e0 100644 --- a/TablePro/Core/Plugins/PluginMetadataRegistry+KafkaDefaults.swift +++ b/TablePro/Core/Plugins/PluginMetadataRegistry+KafkaDefaults.swift @@ -27,7 +27,6 @@ extension PluginMetadataRegistry { brandColorHex: "#231F20", queryLanguageName: "KafkaQL", editorLanguage: .custom("kafkaql"), connectionMode: .network, supportsDatabaseSwitching: false, - supportsColumnReorder: false, capabilities: PluginMetadataSnapshot.CapabilityFlags( supportsSchemaSwitching: false, supportsImport: false, diff --git a/TablePro/Core/Plugins/PluginMetadataRegistry+RegistryDefaults.swift b/TablePro/Core/Plugins/PluginMetadataRegistry+RegistryDefaults.swift index 5790cf88f..9fa0f0340 100644 --- a/TablePro/Core/Plugins/PluginMetadataRegistry+RegistryDefaults.swift +++ b/TablePro/Core/Plugins/PluginMetadataRegistry+RegistryDefaults.swift @@ -27,7 +27,6 @@ extension PluginMetadataRegistry { brandColorHex: "#00ED63", queryLanguageName: "MQL", editorLanguage: .javascript, connectionMode: .network, supportsDatabaseSwitching: true, - supportsColumnReorder: false, capabilities: PluginMetadataSnapshot.CapabilityFlags( supportsSchemaSwitching: false, supportsImport: true, @@ -122,7 +121,6 @@ extension PluginMetadataRegistry { brandColorHex: "#DC382D", queryLanguageName: "Redis CLI", editorLanguage: .bash, connectionMode: .network, supportsDatabaseSwitching: false, - supportsColumnReorder: false, capabilities: PluginMetadataSnapshot.CapabilityFlags( supportsSchemaSwitching: false, supportsImport: false, @@ -242,7 +240,6 @@ extension PluginMetadataRegistry { brandColorHex: "#E34517", queryLanguageName: "SQL", editorLanguage: .sql, connectionMode: .network, supportsDatabaseSwitching: true, - supportsColumnReorder: false, capabilities: PluginMetadataSnapshot.CapabilityFlags( supportsSchemaSwitching: true, supportsImport: true, @@ -323,7 +320,6 @@ extension PluginMetadataRegistry { brandColorHex: "#F37440", queryLanguageName: "SQL", editorLanguage: .sql, connectionMode: .network, supportsDatabaseSwitching: true, - supportsColumnReorder: false, capabilities: PluginMetadataSnapshot.CapabilityFlags( supportsSchemaSwitching: false, supportsImport: true, @@ -413,7 +409,6 @@ extension PluginMetadataRegistry { brandColorHex: "#DD5F3B", queryLanguageName: "SQL", editorLanguage: .sql, connectionMode: .network, supportsDatabaseSwitching: true, - supportsColumnReorder: false, capabilities: PluginMetadataSnapshot.CapabilityFlags( supportsSchemaSwitching: true, supportsImport: false, @@ -532,7 +527,7 @@ extension PluginMetadataRegistry { brandColorHex: "#C3160B", queryLanguageName: "SQL", editorLanguage: .sql, connectionMode: .network, supportsDatabaseSwitching: false, - supportsColumnReorder: false, + columnReorder: .alter, capabilities: PluginMetadataSnapshot.CapabilityFlags( supportsSchemaSwitching: true, supportsImport: true, @@ -591,7 +586,6 @@ extension PluginMetadataRegistry { brandColorHex: "#C60018", queryLanguageName: "SQL", editorLanguage: .sql, connectionMode: .network, supportsDatabaseSwitching: false, - supportsColumnReorder: false, capabilities: PluginMetadataSnapshot.CapabilityFlags( supportsSchemaSwitching: true, supportsImport: true, @@ -654,7 +648,7 @@ extension PluginMetadataRegistry { brandColorHex: "#FFD100", queryLanguageName: "SQL", editorLanguage: .sql, connectionMode: .network, supportsDatabaseSwitching: true, - supportsColumnReorder: false, + columnReorder: .alter, capabilities: PluginMetadataSnapshot.CapabilityFlags( supportsSchemaSwitching: false, supportsImport: true, @@ -702,7 +696,6 @@ extension PluginMetadataRegistry { brandColorHex: "#3F7D20", queryLanguageName: "SQL", editorLanguage: .sql, connectionMode: .fileBased, supportsDatabaseSwitching: false, - supportsColumnReorder: false, capabilities: PluginMetadataSnapshot.CapabilityFlags( supportsSchemaSwitching: false, supportsImport: false, @@ -792,7 +785,6 @@ extension PluginMetadataRegistry { brandColorHex: "#26A0D8", queryLanguageName: "CQL", editorLanguage: .sql, connectionMode: .network, supportsDatabaseSwitching: true, - supportsColumnReorder: false, capabilities: PluginMetadataSnapshot.CapabilityFlags( supportsSchemaSwitching: false, supportsImport: false, @@ -856,7 +848,6 @@ extension PluginMetadataRegistry { brandColorHex: "#6B2EE3", queryLanguageName: "CQL", editorLanguage: .sql, connectionMode: .network, supportsDatabaseSwitching: true, - supportsColumnReorder: false, capabilities: PluginMetadataSnapshot.CapabilityFlags( supportsSchemaSwitching: false, supportsImport: false, @@ -919,7 +910,6 @@ extension PluginMetadataRegistry { brandColorHex: "#419EDA", queryLanguageName: "etcdctl", editorLanguage: .bash, connectionMode: .network, supportsDatabaseSwitching: false, - supportsColumnReorder: false, capabilities: PluginMetadataSnapshot.CapabilityFlags( supportsSchemaSwitching: false, supportsImport: false, @@ -1009,7 +999,7 @@ extension PluginMetadataRegistry { brandColorHex: "#F6821F", queryLanguageName: "SQL", editorLanguage: .sql, connectionMode: .apiOnly, supportsDatabaseSwitching: true, - supportsColumnReorder: false, + columnReorder: .rebuild, capabilities: PluginMetadataSnapshot.CapabilityFlags( supportsSchemaSwitching: false, supportsImport: false, @@ -1069,7 +1059,7 @@ extension PluginMetadataRegistry { brandColorHex: "#4FF8D2", queryLanguageName: "SQL", editorLanguage: .sql, connectionMode: .apiOnly, supportsDatabaseSwitching: false, - supportsColumnReorder: false, + columnReorder: .rebuild, capabilities: PluginMetadataSnapshot.CapabilityFlags( supportsSchemaSwitching: false, supportsImport: false, diff --git a/TablePro/Core/Plugins/PluginMetadataRegistry+SurrealDBDefaults.swift b/TablePro/Core/Plugins/PluginMetadataRegistry+SurrealDBDefaults.swift index 1f48dbc2d..c0907de2e 100644 --- a/TablePro/Core/Plugins/PluginMetadataRegistry+SurrealDBDefaults.swift +++ b/TablePro/Core/Plugins/PluginMetadataRegistry+SurrealDBDefaults.swift @@ -20,7 +20,6 @@ extension PluginMetadataRegistry { brandColorHex: "#FF00A0", queryLanguageName: "SurrealQL", editorLanguage: .custom("surrealql"), connectionMode: .network, supportsDatabaseSwitching: true, - supportsColumnReorder: false, capabilities: PluginMetadataSnapshot.CapabilityFlags( supportsSchemaSwitching: true, supportsImport: false, diff --git a/TablePro/Core/Plugins/PluginMetadataRegistry+TursoDefaults.swift b/TablePro/Core/Plugins/PluginMetadataRegistry+TursoDefaults.swift index a477b8a34..c57521fc4 100644 --- a/TablePro/Core/Plugins/PluginMetadataRegistry+TursoDefaults.swift +++ b/TablePro/Core/Plugins/PluginMetadataRegistry+TursoDefaults.swift @@ -45,7 +45,7 @@ extension PluginMetadataRegistry { brandColorHex: "#4FF8D2", queryLanguageName: "SQL", editorLanguage: .sql, connectionMode: .apiOnly, supportsDatabaseSwitching: false, - supportsColumnReorder: false, + columnReorder: .rebuild, capabilities: PluginMetadataSnapshot.CapabilityFlags( supportsSchemaSwitching: false, supportsImport: false, diff --git a/TablePro/Core/Plugins/PluginMetadataRegistry.swift b/TablePro/Core/Plugins/PluginMetadataRegistry.swift index dddf4fbfc..bbf763c00 100644 --- a/TablePro/Core/Plugins/PluginMetadataRegistry.swift +++ b/TablePro/Core/Plugins/PluginMetadataRegistry.swift @@ -28,7 +28,7 @@ struct PluginMetadataSnapshot: Sendable { let editorLanguage: EditorLanguage let connectionMode: ConnectionMode let supportsDatabaseSwitching: Bool - let supportsColumnReorder: Bool + var columnReorder: ColumnReorderSupport = .unsupported let capabilities: CapabilityFlags let schema: SchemaInfo @@ -241,7 +241,7 @@ struct PluginMetadataSnapshot: Sendable { brandColorHex: brandColorHex, queryLanguageName: queryLanguageName, editorLanguage: editorLanguage, connectionMode: connectionMode, supportsDatabaseSwitching: supportsDatabaseSwitching, - supportsColumnReorder: supportsColumnReorder, + columnReorder: columnReorder, capabilities: capabilities, schema: schema, editor: editor, connection: connection ) } @@ -258,7 +258,7 @@ struct PluginMetadataSnapshot: Sendable { brandColorHex: brandColorHex, queryLanguageName: queryLanguageName, editorLanguage: editorLanguage, connectionMode: connectionMode, supportsDatabaseSwitching: supportsDatabaseSwitching, - supportsColumnReorder: supportsColumnReorder, + columnReorder: columnReorder, capabilities: capabilities, schema: schema, editor: editor, connection: connection ) } @@ -275,7 +275,7 @@ struct PluginMetadataSnapshot: Sendable { brandColorHex: source.brandColorHex, queryLanguageName: queryLanguageName, editorLanguage: editorLanguage, connectionMode: connectionMode, supportsDatabaseSwitching: supportsDatabaseSwitching, - supportsColumnReorder: supportsColumnReorder, + columnReorder: columnReorder, capabilities: capabilities, schema: schema, editor: editor, connection: connection ) } @@ -292,7 +292,7 @@ struct PluginMetadataSnapshot: Sendable { brandColorHex: brandColorHex, queryLanguageName: queryLanguageName, editorLanguage: editorLanguage, connectionMode: connectionMode, supportsDatabaseSwitching: supportsDatabaseSwitching, - supportsColumnReorder: supportsColumnReorder, + columnReorder: columnReorder, capabilities: capabilities, schema: schema, editor: editor, connection: connection ) } @@ -309,7 +309,7 @@ struct PluginMetadataSnapshot: Sendable { brandColorHex: brandColorHex, queryLanguageName: queryLanguageName, editorLanguage: editorLanguage, connectionMode: connectionMode, supportsDatabaseSwitching: source.supportsDatabaseSwitching, - supportsColumnReorder: supportsColumnReorder, + columnReorder: columnReorder, capabilities: capabilities, schema: SchemaInfo( defaultSchemaName: source.schema.defaultSchemaName, @@ -562,7 +562,7 @@ final class PluginMetadataRegistry: @unchecked Sendable { editorLanguage: driverType.editorLanguage, connectionMode: driverType.connectionMode, supportsDatabaseSwitching: driverType.supportsDatabaseSwitching, - supportsColumnReorder: existingSnapshot?.supportsColumnReorder ?? false, + columnReorder: existingSnapshot?.columnReorder ?? .unsupported, capabilities: PluginMetadataSnapshot.CapabilityFlags( supportsSchemaSwitching: driverType.supportsSchemaSwitching, supportsImport: driverType.supportsImport, diff --git a/TablePro/Models/Schema/ColumnReorderReviewRequest.swift b/TablePro/Models/Schema/ColumnReorderReviewRequest.swift new file mode 100644 index 000000000..a0e6eca05 --- /dev/null +++ b/TablePro/Models/Schema/ColumnReorderReviewRequest.swift @@ -0,0 +1,35 @@ +// +// ColumnReorderReviewRequest.swift +// TablePro +// + +import Foundation +import TableProPluginKit + +/// A reorder whose plan recreates the table, held while the user reads it. +/// +/// A positional `ALTER` runs on the drop the way any other direct manipulation does. A rebuild +/// copies every row into a new table and drops the original, so it is shown in full and confirmed +/// first, and what the rebuild cannot carry over is named beside it. +@MainActor +struct ColumnReorderReviewRequest: Identifiable { + let id = UUID() + let tableName: String + + /// The database and schema the plan was built against, carried so the script opens where it + /// belongs. The connection's browse database can be somewhere else by the time the sheet is + /// answered, and PostgreSQL cannot qualify a table with a database, so an unqualified script + /// opened on the wrong one would rebuild a same-named table there. + let scope: DatabaseScope + + let plan: PluginColumnReorderPlan + let perform: () async -> Void + + var warning: String? { + plan.caveats.isEmpty ? nil : plan.caveats.joined(separator: " ") + } + + var isRunnable: Bool { plan.isRunnable } + + var scriptStatements: [String] { plan.scriptStatements } +} diff --git a/TablePro/Models/Schema/ColumnReorderSupport.swift b/TablePro/Models/Schema/ColumnReorderSupport.swift new file mode 100644 index 000000000..569228524 --- /dev/null +++ b/TablePro/Models/Schema/ColumnReorderSupport.swift @@ -0,0 +1,106 @@ +// +// ColumnReorderSupport.swift +// TablePro +// + +import Foundation + +/// How an engine changes the order of a table's columns, if it can at all. +/// +/// Curated per database type rather than asked of a driver, because the drag has to be offered or +/// withheld before the gesture starts and a capability the app only learns from a live connection +/// is too late for that. +enum ColumnReorderSupport: Sendable, Equatable { + /// Positional DDL: the catalog is rewritten and no row is read or written. MySQL, MariaDB and + /// ClickHouse have `MODIFY COLUMN … FIRST | AFTER`; Oracle has no positional clause but its + /// invisible/visible cycle moves a column to the end, which composes into any order. + case alter + + /// No positional DDL at all, so the order changes by recreating the table and copying its rows. + /// The script is reviewed and confirmed before anything runs. + case rebuild + + case unsupported +} + +/// Whether a reorder can be started right now, and what to tell the user when it cannot. +enum ColumnReorderAvailability: Sendable, Equatable { + case available(ColumnReorderSupport) + + /// Reordering is not a gesture this list offers at all, so its absence needs no explaining. + /// An index or foreign key list has no order to change. + case notApplicable + + /// Withheld where the user would reasonably expect to be able to drag, so the reason is shown. + case unavailable(reason: String) + + var support: ColumnReorderSupport? { + if case .available(let support) = self { return support } + return nil + } + + var isAvailable: Bool { support != nil } + + var unavailableReason: String? { + if case .unavailable(let reason) = self { return reason } + return nil + } +} + +/// The single answer to "may this column be dragged, and if not why not". +/// +/// Pure and exhaustive so both the affordance and its explanation come from one place. Splitting +/// them is what shipped the reported bug: the engine gate decided whether anything would happen on +/// the drop, while the drag itself was offered unconditionally, so 30 engines lifted the row, +/// opened the insertion gap, took the drop and did nothing. +enum ColumnReorderPolicy { + static func resolve( + support: ColumnReorderSupport, + engineName: String, + isColumnsTab: Bool, + isTable: Bool, + canEditSchema: Bool, + hasStagedChanges: Bool, + isRearranged: Bool + ) -> ColumnReorderAvailability { + guard isColumnsTab else { return .notApplicable } + /// Every mechanism emits table DDL, and the SQLite one looks the table up by + /// `sqlite_master.type = 'table'`, so a view drag would end in an error rather than an + /// explanation. A view's column order comes from its own `SELECT`. + guard isTable else { + return .unavailable( + reason: String(localized: "A view's column order comes from its query. Edit the view to change it.") + ) + } + guard canEditSchema else { + return .unavailable( + reason: String(format: String(localized: "%@ cannot edit a table's structure."), engineName) + ) + } + switch support { + case .unsupported: + return .unavailable( + reason: String( + format: String(localized: "%@ cannot change the order of a table's columns."), + engineName + ) + ) + case .alter, .rebuild: + guard !hasStagedChanges else { + return .unavailable( + reason: String(localized: "Save or discard the pending structure changes before reordering columns.") + ) + } + /// A drop reports the row's position in what is on screen, and a filtered or sorted + /// list is not the table's order, so "third from the top" names a different column in + /// each. There is nothing to map it back to either: the wanted order is a statement + /// about every column, and a filtered list is not showing every column. + guard !isRearranged else { + return .unavailable( + reason: String(localized: "Clear the filter and the sort to reorder columns.") + ) + } + return .available(support) + } + } +} diff --git a/TablePro/Views/Main/Child/MainEditorContentView.swift b/TablePro/Views/Main/Child/MainEditorContentView.swift index 62bad06d3..8c7b9014a 100644 --- a/TablePro/Views/Main/Child/MainEditorContentView.swift +++ b/TablePro/Views/Main/Child/MainEditorContentView.swift @@ -651,6 +651,7 @@ struct MainEditorContentView: View { connection: connection, databaseName: scope?.database ?? "", schemaName: scope?.schema, + isViewObject: tab.tableContext.isView, toolbarState: coordinator.toolbarState, coordinator: coordinator, selectionState: selectionState, diff --git a/TablePro/Views/Main/Extensions/MainContentCoordinator+SQLPreview.swift b/TablePro/Views/Main/Extensions/MainContentCoordinator+SQLPreview.swift index 2248739e6..fe7958e22 100644 --- a/TablePro/Views/Main/Extensions/MainContentCoordinator+SQLPreview.swift +++ b/TablePro/Views/Main/Extensions/MainContentCoordinator+SQLPreview.swift @@ -48,6 +48,33 @@ extension MainContentCoordinator { activeSheet = .sqlPreview } + /// Hands the rebuild script to a query tab so the user can read, edit and run it themselves. + /// + /// The rebuild reproduces the table from what the server will describe, so a table using + /// something the catalog queries do not reach is better rebuilt by hand from a script the user + /// owns than by a button that reports success. + /// Opened on the scope the plan was built against, not on whatever the connection is browsing. + /// The script names its table without a database, and PostgreSQL has no way to qualify one, so + /// running it against another database would rebuild the same-named table there. + func openColumnReorderScriptInEditor(_ request: ColumnReorderReviewRequest) { + let script = request.scriptStatements + .map { $0.hasSuffix(";") ? $0 : $0 + ";" } + .joined(separator: "\n\n") + WindowManager.shared.openTab( + payload: EditorTabPayload( + connectionId: request.scope.connectionId, + tabType: .query, + databaseName: request.scope.database, + schemaName: request.scope.schema, + initialQuery: script, + skipAutoExecute: true, + tabTitle: String(format: String(localized: "Reorder %@"), request.tableName) + ) + ) + columnReorderRequest = nil + activeSheet = nil + } + /// Everything one press of Save is about to do, in execution order. /// /// Save and Preview SQL both read this, so what the user is shown is what runs. Each step diff --git a/TablePro/Views/Main/MainContentCoordinator.swift b/TablePro/Views/Main/MainContentCoordinator.swift index b5659dbba..40ab97d6d 100644 --- a/TablePro/Views/Main/MainContentCoordinator.swift +++ b/TablePro/Views/Main/MainContentCoordinator.swift @@ -64,6 +64,7 @@ enum ActiveSheet: Identifiable { /// object browser may be pointed somewhere else by the time the sheet appears. case copyObjects(ObjectCopyLaunchRequest) case rewind + case columnReorderReview var id: String { switch self { @@ -79,6 +80,7 @@ enum ActiveSheet: Identifiable { case .createDatabase: "createDatabase" case .copyObjects(let launch): "copyObjects-\(launch.id)" case .rewind: "rewind" + case .columnReorderReview: "columnReorderReview" } } } @@ -346,6 +348,9 @@ final class MainContentCoordinator { /// What restoring the last save would do, once it has been planned against the live rows. internal var rewindPlan: RewindPlan? + /// The rebuild a column drag asked for, held while the user reads it. + internal var columnReorderRequest: ColumnReorderReviewRequest? + /// Continuation for callers that need to await the result of a fire-and-forget save /// (e.g. save-then-close). Set before calling `saveChanges`, resumed by `executeCommitStatements`. @ObservationIgnored internal var saveCompletionContinuation: CheckedContinuation? diff --git a/TablePro/Views/Main/MainContentView.swift b/TablePro/Views/Main/MainContentView.swift index 1b3d859aa..2a12d847d 100644 --- a/TablePro/Views/Main/MainContentView.swift +++ b/TablePro/Views/Main/MainContentView.swift @@ -167,6 +167,12 @@ struct MainContentView: View { if !$0 { coordinator.activeSheet = nil coordinator.exportPreselection = nil + /// Cleared on every dismissal, Cancel included. The request holds the closure + /// that runs the rebuild, and that closure holds the structure view, which + /// holds this coordinator; leaving it set after a cancel keeps the cycle alive + /// for the window's life and offers a stale plan to whatever opens the sheet + /// next. + coordinator.columnReorderRequest = nil } } ) @@ -295,6 +301,29 @@ struct MainContentView: View { await coordinator.applyRewind() } } + case .columnReorderReview: + if let request = coordinator.columnReorderRequest { + SQLReviewSheet( + isPresented: dismissBinding, + statements: request.scriptStatements, + databaseType: connection.type, + warning: request.warning, + primaryAction: request.isRunnable + ? SQLReviewSheet.PrimaryAction( + title: String(localized: "Rebuild Table"), + isDestructive: true, + perform: { + await request.perform() + coordinator.columnReorderRequest = nil + coordinator.activeSheet = nil + } + ) + : nil, + onOpenInEditor: { + coordinator.openColumnReorderScriptInEditor(request) + } + ) + } } } diff --git a/TablePro/Views/Results/DataGridCoordinator.swift b/TablePro/Views/Results/DataGridCoordinator.swift index d640411bc..fc40fab4e 100644 --- a/TablePro/Views/Results/DataGridCoordinator.swift +++ b/TablePro/Views/Results/DataGridCoordinator.swift @@ -76,6 +76,7 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData var displayCache: RowDisplayCache { displayState.cache } private var pendingScrollAnchorRow: Int? weak var delegate: (any DataGridViewDelegate)? + var rowReorder: DataGridRowReorder = .disabled weak var activeFKPreviewPopover: NSPopover? weak var activeCellEditorPopover: NSPopover? weak var activePoppedOutEditor: JSONViewerWindowController? diff --git a/TablePro/Views/Results/DataGridRowReorder.swift b/TablePro/Views/Results/DataGridRowReorder.swift new file mode 100644 index 000000000..034dd448f --- /dev/null +++ b/TablePro/Views/Results/DataGridRowReorder.swift @@ -0,0 +1,26 @@ +// +// DataGridRowReorder.swift +// TablePro +// + +import Foundation + +/// Whether this grid offers row reordering, and what to tell the user when it does not. +/// +/// The grid used to infer this from `delegate != nil`, which is true of every grid in the app, so +/// it registered the drag type, answered a drop with `.move` and accepted it on engines that could +/// not reorder anything. The handler behind it was optional and evaluated to nothing, so the row +/// lifted, the insertion gap opened, the drop was taken and nothing happened. +struct DataGridRowReorder: Equatable { + var isEnabled: Bool + /// Shown as a help tag on the row number, which is the handle the drag starts from. Nil where + /// the grid never offers reordering at all and the absence needs no explaining. + var unavailableReason: String? + + init(isEnabled: Bool = false, unavailableReason: String? = nil) { + self.isEnabled = isEnabled + self.unavailableReason = unavailableReason + } + + static let disabled = DataGridRowReorder() +} diff --git a/TablePro/Views/Results/DataGridUpdateSnapshot.swift b/TablePro/Views/Results/DataGridUpdateSnapshot.swift index 586c5f3c5..51c3667cf 100644 --- a/TablePro/Views/Results/DataGridUpdateSnapshot.swift +++ b/TablePro/Views/Results/DataGridUpdateSnapshot.swift @@ -15,7 +15,7 @@ struct DataGridUpdateSnapshot: Equatable { let displayFormats: [ValueDisplayFormat?] let configuration: DataGridConfiguration let isEditable: Bool - let hasMoveDelegate: Bool + let rowReorder: DataGridRowReorder let rowHeight: CGFloat let alternatingRows: Bool let reloadVersion: Int diff --git a/TablePro/Views/Results/DataGridView+RowActions.swift b/TablePro/Views/Results/DataGridView+RowActions.swift index 2e6c53627..0a3552df1 100644 --- a/TablePro/Views/Results/DataGridView+RowActions.swift +++ b/TablePro/Views/Results/DataGridView+RowActions.swift @@ -282,10 +282,15 @@ extension TableViewCoordinator { private static let rowDragType = NSPasteboard.PasteboardType("com.TablePro.rowDrag") + /// Writes the reorder type only where a reorder can actually run. The text and HTML flavours + /// are written either way: dragging a row into another app is a copy, and it stays available on + /// an engine whose columns cannot move. func tableView(_ tableView: NSTableView, pasteboardWriterForRow row: Int) -> (any NSPasteboardWriting)? { guard delegate != nil else { return nil } let item = NSPasteboardItem() - item.setString(String(row), forType: Self.rowDragType) + if rowReorder.isEnabled { + item.setString(String(row), forType: Self.rowDragType) + } if let values = displayRow(at: row)?.values { let tableRows = tableRowsProvider() @@ -310,7 +315,7 @@ extension TableViewCoordinator { proposedRow row: Int, proposedDropOperation dropOperation: NSTableView.DropOperation ) -> NSDragOperation { - guard delegate != nil else { return [] } + guard delegate != nil, rowReorder.isEnabled else { return [] } guard info.draggingSource as? NSTableView === tableView else { return [] } guard info.draggingPasteboard.availableType(from: [Self.rowDragType]) != nil else { return [] } guard dropOperation == .above else { @@ -326,7 +331,7 @@ extension TableViewCoordinator { row: Int, dropOperation: NSTableView.DropOperation ) -> Bool { - guard let delegate else { return false } + guard let delegate, rowReorder.isEnabled else { return false } guard let item = info.draggingPasteboard.pasteboardItems?.first, let rowString = item.string(forType: Self.rowDragType), let fromRow = Int(rowString) else { diff --git a/TablePro/Views/Results/DataGridView.swift b/TablePro/Views/Results/DataGridView.swift index be77886b5..17d8bba72 100644 --- a/TablePro/Views/Results/DataGridView.swift +++ b/TablePro/Views/Results/DataGridView.swift @@ -37,6 +37,8 @@ struct DataGridView: NSViewRepresentable { var displayFormats: [ValueDisplayFormat?] = [] var delegate: (any DataGridViewDelegate)? var layoutPersister: (any ColumnLayoutPersisting)? + /// Whether a row may be dragged to a new position, and why not when it may not. + var rowReorder: DataGridRowReorder = .disabled @Binding var selectedRowIndices: Set @Binding var sortState: SortState @@ -143,8 +145,8 @@ struct DataGridView: NSViewRepresentable { coordinator.isRebuildingColumns = false coordinator.updateColumnPresentations(from: initialRows) - let hasMoveRow = delegate != nil - if hasMoveRow { + coordinator.rowReorder = rowReorder + if rowReorder.isEnabled { tableView.registerForDraggedTypes([NSPasteboard.PasteboardType("com.TablePro.rowDrag")]) tableView.draggingDestinationFeedbackStyle = .gap } @@ -201,7 +203,7 @@ struct DataGridView: NSViewRepresentable { displayFormats: displayFormats, configuration: configuration, isEditable: isEditable, - hasMoveDelegate: delegate != nil, + rowReorder: rowReorder, rowHeight: rowHeight, alternatingRows: alternatingRows, reloadVersion: changeManager.reloadVersion, @@ -227,7 +229,7 @@ struct DataGridView: NSViewRepresentable { columnCount: columnCount, rowHeight: rowHeight, alternatingRows: alternatingRows, - hasMoveDelegate: snapshot.hasMoveDelegate, + rowReorder: snapshot.rowReorder, contentChanged: contentChanged, columnComments: columnComments ) @@ -246,7 +248,7 @@ struct DataGridView: NSViewRepresentable { columnCount: Int, rowHeight: CGFloat, alternatingRows: Bool, - hasMoveDelegate: Bool, + rowReorder: DataGridRowReorder, contentChanged: Bool, columnComments: [String: String] ) { @@ -261,12 +263,13 @@ struct DataGridView: NSViewRepresentable { } } + coordinator.rowReorder = rowReorder let rowDragType = NSPasteboard.PasteboardType("com.TablePro.rowDrag") let hasDragRegistered = tableView.registeredDraggedTypes.contains(rowDragType) - if hasMoveDelegate && !hasDragRegistered { + if rowReorder.isEnabled && !hasDragRegistered { tableView.registerForDraggedTypes([rowDragType]) tableView.draggingDestinationFeedbackStyle = .gap - } else if !hasMoveDelegate && hasDragRegistered { + } else if !rowReorder.isEnabled && hasDragRegistered { let remaining = tableView.registeredDraggedTypes.filter { $0 != rowDragType } tableView.unregisterDraggedTypes() if !remaining.isEmpty { diff --git a/TablePro/Views/Results/Extensions/DataGridView+Columns.swift b/TablePro/Views/Results/Extensions/DataGridView+Columns.swift index 749875a53..d454f915c 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+Columns.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+Columns.swift @@ -26,13 +26,17 @@ extension TableViewCoordinator { guard let column = tableColumn else { return nil } guard column.identifier != ColumnIdentitySchema.rowNumberIdentifier else { let tableRows = tableRowsProvider() - return cellRegistry.makeRowNumberCell( + let cell = cellRegistry.makeRowNumberCell( in: tableView, row: row, pageOffset: paginationOffsetProvider(), cachedRowCount: displayIDs?.count ?? tableRows.count, visualState: visualState(for: row) ) + /// The row number is the handle a reorder drag starts from, so it is where the reason + /// the drag is withheld belongs. Reused cells carry the last value, so clear it. + cell.toolTip = rowReorder.unavailableReason + return cell } guard DataGridAccessibility.isActive, diff --git a/TablePro/Views/Structure/StructureColumnReorderHandler.swift b/TablePro/Views/Structure/StructureColumnReorderHandler.swift index a68ff6695..eec441af4 100644 --- a/TablePro/Views/Structure/StructureColumnReorderHandler.swift +++ b/TablePro/Views/Structure/StructureColumnReorderHandler.swift @@ -2,8 +2,7 @@ // StructureColumnReorderHandler.swift // TablePro // -// Orchestrates column reorder via ALTER TABLE ... MODIFY COLUMN ... AFTER -// when the user drags a row in the Structure tab's column list. +// Turns a drag in the Structure tab's column list into the statements that reorder the table. // import Foundation @@ -19,6 +18,7 @@ enum StructureColumnReorderHandler { case notSupported case invalidIndices case sqlGenerationFailed + case schemaChanged case executionFailed(String) var errorDescription: String? { @@ -30,109 +30,194 @@ enum StructureColumnReorderHandler { case .invalidIndices: return String(localized: "Invalid column indices for reorder operation") case .sqlGenerationFailed: - return String(localized: "Failed to generate SQL for column reorder") + return String( + localized: """ + Could not build the column reorder for this table. If this engine's driver \ + was installed before column reorder shipped, update it in Settings > Plugins. + """ + ) + case .schemaChanged: + return String( + localized: """ + The table changed while the script was open. Nothing was run. Close and \ + reopen the structure tab, then try again. + """ + ) case .executionFailed(let message): return String(format: String(localized: "Column reorder failed: %@"), message) } } } - /// Move a column from one position to another in the table's column order. + /// A plan and the fingerprint of the schema it was built from. + /// + /// The fingerprint is what makes a reviewed rebuild safe to run later: a plan ends in a `DROP`, + /// and anything another connection added while the sheet was open is inside the table the plan + /// is about to drop and absent from the one that replaces it. + struct PreparedReorder { + let plan: PluginColumnReorderPlan + let fingerprint: String? + let scope: DatabaseScope + } + + /// The order a drag asks for, as column names. /// /// - Parameters: /// - fromIndex: The source row index in the NSTableView (0-based). - /// - toIndex: The drop target row index from NSTableView's `acceptDrop`. - /// This is the row ABOVE which the item will be inserted. - /// - workingColumns: The current column definitions in display order. - /// - tableName: The table being modified. - /// - connectionId: The connection to execute the SQL on. - static func moveColumn( + /// - toIndex: The drop target row index from NSTableView's `acceptDrop`, which is the row + /// ABOVE which the item will be inserted, so it may equal `count`. + static func desiredOrder( fromIndex: Int, toIndex: Int, - workingColumns: [EditableColumnDefinition], - tableName: String, - connectionId: UUID - ) async throws -> String { - guard fromIndex >= 0, fromIndex < workingColumns.count, - toIndex >= 0, toIndex <= workingColumns.count else { + columnNames: [String] + ) throws -> [String] { + guard fromIndex >= 0, fromIndex < columnNames.count, + toIndex >= 0, toIndex <= columnNames.count else { throw ReorderError.invalidIndices } + var names = columnNames + let moving = names.remove(at: fromIndex) + /// Removing the source shifts everything below it up by one, so a drop below the source + /// lands one position too low unless the insertion point moves with it. + let insertionIndex = fromIndex < toIndex ? toIndex - 1 : toIndex + names.insert(moving, at: insertionIndex) + return names + } - guard let driver = DatabaseManager.shared.driver(for: connectionId) else { - throw ReorderError.noDriver - } - - guard let adapter = driver as? PluginDriverAdapter else { - throw ReorderError.notSupported - } - - let movingColumn = workingColumns[fromIndex] - let pluginColumn = buildPluginColumn(from: movingColumn) - - // Compute the "after" column name. - // NSTableView acceptDrop toIndex is the row ABOVE which the drop occurs. - // toIndex == 0 means FIRST position (afterColumn = nil). - // Otherwise, build a virtual list with the source removed, then pick - // the column at (insertionIndex - 1) as the "after" target. - let afterColumn: String? - if toIndex == 0 { - afterColumn = nil - } else { - var columnNames = workingColumns.map(\.name) - columnNames.remove(at: fromIndex) - - // Adjust insertion point: if source was above the drop target, the - // indices shift down by one after removal. - let adjustedIndex = fromIndex < toIndex ? toIndex - 1 : toIndex - - // The column just before the insertion point is the "after" target - let afterIndex = adjustedIndex - 1 - if afterIndex >= 0, afterIndex < columnNames.count { - afterColumn = columnNames[afterIndex] - } else { - afterColumn = nil + /// Asks the driver for the statements that produce `desiredOrder`. + /// + /// Runs on the tab's own scope, never on whichever database the connection's shared driver + /// happens to be pointed at. Another tab or window can move that driver between the drag and + /// the drop, and an unqualified `DROP TABLE` would then land on a same-named table elsewhere. + /// + /// Nothing is executed here. A plan whose cost is a table rebuild is reviewed and confirmed + /// before it runs, and only the caller knows which of the two it is looking at. + static func prepare( + desiredOrder: [String], + workingColumns: [EditableColumnDefinition], + tableName: String, + scope: DatabaseScope + ) async throws -> PreparedReorder { + let columns = workingColumns.map { $0.toPlugin() } + let schema = scope.schema + + let prepared = try await DatabaseManager.shared.withScopedDriver( + scope: scope, + route: DatabaseManager.shared.executionRoute(for: scope), + cancellation: .untracked + ) { driver in + guard let adapter = driver as? PluginDriverAdapter else { + throw ReorderError.notSupported + } + let plan = try await adapter.generateColumnReorderPlan( + table: tableName, + schema: schema, + columns: columns, + desiredOrder: desiredOrder + ) + guard let plan, !plan.statements.isEmpty else { + throw ReorderError.sqlGenerationFailed } + let fingerprint = try? await adapter.columnReorderSchemaFingerprint( + table: tableName, schema: schema + ) + return (plan, fingerprint) } - guard let sql = adapter.generateMoveColumnSQL( - table: tableName, - column: pluginColumn, - afterColumn: afterColumn - ) else { - throw ReorderError.sqlGenerationFailed - } + return PreparedReorder(plan: prepared.0, fingerprint: prepared.1, scope: scope) + } + + /// Runs a prepared reorder, once, on the scope it was planned against. + /// + /// Authorization happens once for the whole plan, before any statement runs, and deliberately + /// outside the scoped block: it can await a confirmation sheet and Touch ID, and holding the + /// connection's driver across a human prompt would freeze every other tab on it. Asking per + /// statement was worse than slow, it was wrong: a user could approve through a rebuild's last + /// write and decline the statement after it, by which point there was nothing left to refuse. + static func execute( + _ prepared: PreparedReorder, + tableName: String, + databaseType: DatabaseType + ) async throws { + let plan = prepared.plan + let scope = prepared.scope + let combined = plan.scriptStatements.joined(separator: "\n") let decision = await ExecutionGateProvider.shared.authorize( OperationRequest( - connectionId: connectionId, - databaseType: adapter.connection.type, - sql: sql, - kind: .schemaMutation, + connectionId: scope.connectionId, + databaseType: databaseType, + sql: combined, + kind: plan.cost == .tableRebuild ? .destructiveQuery : .schemaMutation, caller: .userInterface, capabilities: .interactiveUser, - operationDescription: String(localized: "Reorder Column") + operationDescription: String(localized: "Reorder Columns") ) ) guard case .authorized = decision else { throw DatabaseError.queryFailed(decision.deniedReason ?? String(localized: "Operation not permitted")) } - logger.info("Reordering column '\(movingColumn.name)' — \(sql)") + let expectedFingerprint = prepared.fingerprint + try await DatabaseManager.shared.withScopedDriver( + scope: scope, + route: DatabaseManager.shared.executionRoute(for: scope), + cancellation: .protectedWrite + ) { driver in + if let expectedFingerprint, + let adapter = driver as? PluginDriverAdapter, + let current = try? await adapter.columnReorderSchemaFingerprint( + table: tableName, schema: scope.schema + ), + current != expectedFingerprint { + throw ReorderError.schemaChanged + } - do { - _ = try await driver.execute(query: sql) - } catch { - logger.error("Column reorder failed: \(error.localizedDescription, privacy: .public)") - throw ReorderError.executionFailed(error.localizedDescription) - } + for sql in plan.prologue { + _ = try? await driver.execute(query: sql) + } - return sql - } + /// Only the transaction this plan opened is ever rolled back. Rolling back + /// unconditionally would discard a transaction the user had already opened on the same + /// session and never committed. + let usesTransaction = plan.isTransactional && driver.supportsTransactions + if usesTransaction { + try await driver.beginTransaction(mode: .readWrite) + } - /// Delegates to the single converter every other write path uses. Re-listing the fields here - /// silently dropped `charset` and `collation`, so a MySQL drag re-derived them from the table - /// default and transcoded the column's data. - private static func buildPluginColumn(from col: EditableColumnDefinition) -> PluginColumnDefinition { - col.toPlugin() + var completed = 0 + do { + for sql in plan.statements { + logger.info("Reordering columns: \(sql, privacy: .public)") + _ = try await driver.execute(query: sql) + completed += 1 + } + if usesTransaction { + try await driver.commitTransaction() + } + } catch { + if usesTransaction { + do { + try await driver.rollbackTransaction() + } catch { + logger.error("Column reorder rollback failed: \(error.localizedDescription, privacy: .public)") + } + } else if completed > 0 { + /// An engine whose DDL commits statement by statement has nothing to roll back, + /// so it supplies statements that put back what already ran. + for sql in plan.compensation { + _ = try? await driver.execute(query: sql) + } + } + for sql in plan.epilogue { + _ = try? await driver.execute(query: sql) + } + throw ReorderError.executionFailed(error.localizedDescription) + } + + for sql in plan.epilogue { + _ = try? await driver.execute(query: sql) + } + } } } diff --git a/TablePro/Views/Structure/TableStructureView+ColumnReorder.swift b/TablePro/Views/Structure/TableStructureView+ColumnReorder.swift new file mode 100644 index 000000000..0927edb80 --- /dev/null +++ b/TablePro/Views/Structure/TableStructureView+ColumnReorder.swift @@ -0,0 +1,127 @@ +// +// TableStructureView+ColumnReorder.swift +// TablePro +// +// Dragging a column row to a new position, and what happens when the engine cannot. +// + +import Combine +import Foundation +import SwiftUI +import TableProPluginKit + +extension TableStructureView { + /// Whether a column may be dragged right now, and the reason shown on the row number when not. + /// + /// Read by both the grid, which decides whether to offer the drag at all, and the delegate, + /// which decides whether there is a handler behind it. One answer, so the two cannot disagree. + var columnReorderAvailability: ColumnReorderAvailability { + ColumnReorderPolicy.resolve( + support: PluginManager.shared.columnReorderSupport(for: connection.type), + engineName: connection.type.displayName, + isColumnsTab: selectedTab == .columns, + isTable: !isViewObject, + canEditSchema: connection.type.supportsSchemaEditing, + hasStagedChanges: structureChangeManager.hasChanges, + isRearranged: !searchText.isEmpty || structureSortDescriptor != nil + ) + } + + func beginColumnReorder(fromIndex: Int, toIndex: Int) { + let columnsSnapshot = structureChangeManager.workingColumns + let clearTarget = coordinator?.selectedColumnLayoutClearTarget() + let reorderScope = scope + + Task { @MainActor in + do { + let desiredOrder = try StructureColumnReorderHandler.desiredOrder( + fromIndex: fromIndex, + toIndex: toIndex, + columnNames: columnsSnapshot.map(\.name) + ) + let prepared = try await StructureColumnReorderHandler.prepare( + desiredOrder: desiredOrder, + workingColumns: columnsSnapshot, + tableName: tableName, + scope: reorderScope + ) + + switch prepared.plan.cost { + case .metadataOnly: + try await StructureColumnReorderHandler.execute( + prepared, tableName: tableName, databaseType: connection.type + ) + await finishColumnReorder(prepared, clearTarget: clearTarget) + case .tableRebuild: + presentColumnReorderReview(prepared, clearTarget: clearTarget) + @unknown default: + /// A cost this build does not recognise is shown before it runs, never after. + presentColumnReorderReview(prepared, clearTarget: clearTarget) + } + } catch { + reportColumnReorderFailure(error) + } + } + } + + /// A rebuild is never run on the drop. It is shown in full, with what it cannot carry over, and + /// the user either runs it here or takes the script to a query tab. + private func presentColumnReorderReview( + _ prepared: StructureColumnReorderHandler.PreparedReorder, + clearTarget: ColumnLayoutClearTarget? + ) { + guard let coordinator else { return } + coordinator.columnReorderRequest = ColumnReorderReviewRequest( + tableName: tableName, + scope: prepared.scope, + plan: prepared.plan, + perform: { + do { + try await StructureColumnReorderHandler.execute( + prepared, tableName: tableName, databaseType: connection.type + ) + await finishColumnReorder(prepared, clearTarget: clearTarget) + } catch { + reportColumnReorderFailure(error) + } + } + ) + coordinator.activeSheet = .columnReorderReview + } + + private func finishColumnReorder( + _ prepared: StructureColumnReorderHandler.PreparedReorder, + clearTarget: ColumnLayoutClearTarget? + ) async { + await services.queryHistoryManager.record( + QueryHistoryRecordRequest( + query: prepared.plan.scriptStatements + .map { $0.hasSuffix(";") ? $0 : $0 + ";" } + .joined(separator: "\n"), + connectionId: prepared.scope.connectionId, + databaseName: prepared.scope.database, + databaseType: connection.type, + source: .structureDDL, + executionTime: 0, + rowCount: -1, + wasSuccessful: true + ) + ) + isReloadingAfterSave = true + await loadColumns() + loadSchemaForEditing() + isReloadingAfterSave = false + if let clearTarget { + coordinator?.clearColumnLayout(clearTarget) + } + AppCommands.shared.refreshData.send(DataRefreshRequest(connectionId: connection.id)) + } + + private func reportColumnReorderFailure(_ error: any Error) { + AlertHelper.showErrorSheet( + title: String(localized: "Column Reorder Failed"), + message: error.localizedDescription, + window: coordinator?.contentWindow + ) + } +} diff --git a/TablePro/Views/Structure/TableStructureView.swift b/TablePro/Views/Structure/TableStructureView.swift index 20bdb121e..8efa4f87e 100644 --- a/TablePro/Views/Structure/TableStructureView.swift +++ b/TablePro/Views/Structure/TableStructureView.swift @@ -29,11 +29,16 @@ struct TableStructureView: View { let connection: DatabaseConnection let databaseName: String let schemaName: String? + + /// Whether the Structure tab is open on a view rather than a table. Every reorder mechanism + /// emits table DDL, so a view is withheld rather than allowed to fail at the statement. + var isViewObject: Bool = false + let toolbarState: ConnectionToolbarState let coordinator: MainContentCoordinator? let selectionState: GridSelectionState - @Environment(\.appServices) private var services + @Environment(\.appServices) var services /// Derived from the tab's own binding on every render so it can never go stale. var scope: DatabaseScope { @@ -148,6 +153,7 @@ struct TableStructureView: View { connection: DatabaseConnection, databaseName: String, schemaName: String?, + isViewObject: Bool = false, toolbarState: ConnectionToolbarState, coordinator: MainContentCoordinator?, selectionState: GridSelectionState, @@ -157,6 +163,7 @@ struct TableStructureView: View { self.connection = connection self.databaseName = databaseName self.schemaName = schemaName + self.isViewObject = isViewObject self.toolbarState = toolbarState self.coordinator = coordinator self.selectionState = selectionState @@ -462,64 +469,16 @@ struct TableStructureView: View { func updateGridDelegate() { let provider = makeCurrentProvider() - let canEdit = connection.type.supportsSchemaEditing gridDelegate.selectedTab = selectedTab gridDelegate.currentProvider = provider gridDelegate.orderedFields = provider.orderedColumnFields coordinator?.inspectorRowSourceRevision += 1 - let moveRowHandler: ((Int, Int) -> Void)? = { - guard selectedTab == .columns, - canEdit, - !structureChangeManager.hasChanges, - PluginManager.shared.supportsColumnReorder(for: connection.type) else { - return nil - } - return { [self] fromIndex, toIndex in - let columnsSnapshot = structureChangeManager.workingColumns - let columnLayoutClearTarget = coordinator?.selectedColumnLayoutClearTarget() - Task { @MainActor in - do { - let executedSQL = try await StructureColumnReorderHandler.moveColumn( - fromIndex: fromIndex, - toIndex: toIndex, - workingColumns: columnsSnapshot, - tableName: tableName, - connectionId: connection.id - ) - await services.queryHistoryManager.record( - QueryHistoryRecordRequest( - query: executedSQL.hasSuffix(";") ? executedSQL : executedSQL + ";", - connectionId: connection.id, - databaseName: DatabaseManager.shared.browseDatabaseName(for: connection), - databaseType: connection.type, - source: .structureDDL, - executionTime: 0, - rowCount: -1, - wasSuccessful: true - ) - ) - isReloadingAfterSave = true - await loadColumns() - loadSchemaForEditing() - isReloadingAfterSave = false - if let columnLayoutClearTarget { - coordinator?.clearColumnLayout(columnLayoutClearTarget) - } - AppCommands.shared.refreshData.send(DataRefreshRequest(connectionId: connection.id)) - } catch { - AlertHelper.showErrorSheet( - title: String(localized: "Column Reorder Failed"), - message: error.localizedDescription, - window: coordinator?.contentWindow - ) - } - } - } - }() - - gridDelegate.moveRowHandler = moveRowHandler + let availability = columnReorderAvailability + gridDelegate.moveRowHandler = availability.isAvailable ? { [self] fromIndex, toIndex in + beginColumnReorder(fromIndex: fromIndex, toIndex: toIndex) + } : nil } private var structureGrid: some View { @@ -550,6 +509,10 @@ struct TableStructureView: View { databaseType: connection.type ), delegate: gridDelegate, + rowReorder: DataGridRowReorder( + isEnabled: columnReorderAvailability.isAvailable, + unavailableReason: columnReorderAvailability.unavailableReason + ), selectedRowIndices: $selectedRows, sortState: $session.sortState, columnLayout: columnLayoutBinding(for: selectedTab), diff --git a/TableProTests/Core/Database/SwitchDatabaseReconnectFailureTests.swift b/TableProTests/Core/Database/SwitchDatabaseReconnectFailureTests.swift index ae04eca18..8e2d7b50b 100644 --- a/TableProTests/Core/Database/SwitchDatabaseReconnectFailureTests.swift +++ b/TableProTests/Core/Database/SwitchDatabaseReconnectFailureTests.swift @@ -44,7 +44,6 @@ struct SwitchDatabaseReconnectFailureTests { supportsHealthMonitor: false, urlSchemes: ["reconnectswitchfake"], postConnectActions: [], brandColorHex: "#000000", queryLanguageName: "SQL", editorLanguage: .sql, connectionMode: .network, supportsDatabaseSwitching: true, - supportsColumnReorder: false, capabilities: capabilities, schema: .defaults, editor: .defaults, connection: .defaults ) PluginMetadataRegistry.shared.register(snapshot: snapshot, forTypeId: Self.typeId) diff --git a/TableProTests/Core/Plugins/PluginMetadataRegistryBrandingTests.swift b/TableProTests/Core/Plugins/PluginMetadataRegistryBrandingTests.swift index fd5b6b5ec..cd256cacc 100644 --- a/TableProTests/Core/Plugins/PluginMetadataRegistryBrandingTests.swift +++ b/TableProTests/Core/Plugins/PluginMetadataRegistryBrandingTests.swift @@ -46,7 +46,6 @@ struct PluginMetadataRegistryBrandingTests { supportsHealthMonitor: false, urlSchemes: ["brandtest"], postConnectActions: [], brandColorHex: brandColorHex, queryLanguageName: "Q", editorLanguage: .bash, connectionMode: .network, supportsDatabaseSwitching: false, - supportsColumnReorder: false, capabilities: .defaults, schema: .defaults, editor: .defaults, connection: PluginMetadataSnapshot.ConnectionConfig( additionalConnectionFields: fields, diff --git a/TableProTests/Core/Plugins/PluginMetadataRegistryDownloadableTests.swift b/TableProTests/Core/Plugins/PluginMetadataRegistryDownloadableTests.swift index b9dedc36c..25de4bd73 100644 --- a/TableProTests/Core/Plugins/PluginMetadataRegistryDownloadableTests.swift +++ b/TableProTests/Core/Plugins/PluginMetadataRegistryDownloadableTests.swift @@ -63,7 +63,6 @@ struct PluginMetadataRegistryDownloadableTests { supportsHealthMonitor: false, urlSchemes: ["thirdparty"], postConnectActions: [], brandColorHex: "#000000", queryLanguageName: "SQL", editorLanguage: .sql, connectionMode: .network, supportsDatabaseSwitching: true, - supportsColumnReorder: false, capabilities: .defaults, schema: .defaults, editor: .defaults, connection: .defaults ) registry.register(snapshot: snapshot, forTypeId: typeId) diff --git a/TableProTests/Models/Schema/ColumnReorderPlannerTests.swift b/TableProTests/Models/Schema/ColumnReorderPlannerTests.swift new file mode 100644 index 000000000..fc15186c1 --- /dev/null +++ b/TableProTests/Models/Schema/ColumnReorderPlannerTests.swift @@ -0,0 +1,152 @@ +// +// ColumnReorderPlannerTests.swift +// TablePro +// + +import Foundation +import TableProPluginKit +@testable import TablePro +import Testing + +@Suite("Column Reorder Planner") +struct ColumnReorderPlannerTests { + private let current = ["a", "b", "c", "d"] + + // MARK: - Positional moves + + @Test("A drag that changes nothing produces no statement") + func identityOrderProducesNoMoves() { + #expect(PluginColumnReorderPlanner.moves(from: current, to: current).isEmpty) + } + + /// One drag is one statement, whichever way it went. Walking the wanted order and fixing every + /// position that disagrees passes the upward case and emits two statements for the downward + /// one, because dragging a column down makes every column it passed disagree. + @Test("One drag is one move, up or down", arguments: [ + ["a", "c", "b", "d"], + ["a", "c", "d", "b"], + ["b", "a", "c", "d"], + ["a", "b", "d", "c"] + ]) + func oneDragIsOneMove(desired: [String]) { + #expect(PluginColumnReorderPlanner.moves(from: current, to: desired).count == 1) + #expect(applyingMoves(to: desired) == desired) + } + + @Test("A column dragged to the top is anchored on nothing, which is FIRST") + func moveToFrontHasNoAnchor() { + let moves = PluginColumnReorderPlanner.moves(from: current, to: ["d", "a", "b", "c"]) + #expect(moves == [PluginColumnReorderPlanner.Move(column: "d", afterColumn: nil)]) + } + + @Test("Applying the moves in order reproduces the wanted order", arguments: [ + ["d", "b", "a", "c"], + ["d", "c", "b", "a"], + ["b", "d", "a", "c"], + ["c", "a", "d", "b"] + ]) + func movesReproduceDesiredOrder(desired: [String]) { + #expect(applyingMoves(to: desired) == desired) + } + + private func applyingMoves(to desired: [String]) -> [String] { + var working = current + for move in PluginColumnReorderPlanner.moves(from: current, to: desired) { + working.removeAll { $0 == move.column } + if let after = move.afterColumn, let index = working.firstIndex(of: after) { + working.insert(move.column, at: working.index(after: index)) + } else { + working.insert(move.column, at: 0) + } + } + return working + } + + @Test("An order that is not a permutation of the current one has no answer") + func nonPermutationProducesNoMoves() { + #expect(PluginColumnReorderPlanner.moves(from: current, to: ["a", "b", "c"]).isEmpty) + #expect(PluginColumnReorderPlanner.moves(from: current, to: ["a", "b", "c", "e"]).isEmpty) + } + + // MARK: - Append cycle + + @Test("An append-only engine leaves the longest matching prefix alone") + func appendCycleKeepsLongestPrefix() { + #expect(PluginColumnReorderPlanner.appendCycle(from: current, to: ["a", "c", "d", "b"]) == ["b"]) + #expect(PluginColumnReorderPlanner.appendCycle(from: current, to: ["a", "c", "b", "d"]) == ["b", "d"]) + } + + @Test("An unchanged order cycles nothing") + func appendCycleOfIdentityIsEmpty() { + #expect(PluginColumnReorderPlanner.appendCycle(from: current, to: current).isEmpty) + } + + @Test("Appending the cycled columns in order reproduces the wanted order") + func appendCycleReproducesDesiredOrder() { + for desired in [["d", "c", "b", "a"], ["b", "a", "d", "c"], ["a", "c", "d", "b"]] { + var working = current + for column in PluginColumnReorderPlanner.appendCycle(from: current, to: desired) { + working.removeAll { $0 == column } + working.append(column) + } + #expect(working == desired, "cycling failed to reach \(desired)") + } + } + + @Test("An order that is not a permutation of the current one has no cycle") + func nonPermutationProducesNoCycle() { + #expect(PluginColumnReorderPlanner.appendCycle(from: current, to: ["a", "b"]).isEmpty) + } + + // MARK: - Exhaustive + + /// Both planners are asked for every permutation of five columns. A tie in the common + /// subsequence can pick a different set to leave alone without changing how many columns move, + /// so the guarantee worth pinning is the result, not the choice. + @Test("Every permutation of five columns is reached, by both mechanisms, in the minimum moves") + func everyPermutationIsReachable() { + let start = ["a", "b", "c", "d", "e"] + for desired in permutations(of: start) { + let moves = PluginColumnReorderPlanner.moves(from: start, to: desired) + var byMove = start + for move in moves { + byMove.removeAll { $0 == move.column } + if let after = move.afterColumn, let index = byMove.firstIndex(of: after) { + byMove.insert(move.column, at: byMove.index(after: index)) + } else { + byMove.insert(move.column, at: 0) + } + } + #expect(byMove == desired, "moves did not reach \(desired)") + #expect(moves.count == start.count - longestCommonSubsequenceLength(start, desired)) + + var byCycle = start + for column in PluginColumnReorderPlanner.appendCycle(from: start, to: desired) { + byCycle.removeAll { $0 == column } + byCycle.append(column) + } + #expect(byCycle == desired, "cycling did not reach \(desired)") + } + } + + private func permutations(of values: [String]) -> [[String]] { + guard values.count > 1 else { return [values] } + return values.indices.flatMap { index -> [[String]] in + var rest = values + let picked = rest.remove(at: index) + return permutations(of: rest).map { [picked] + $0 } + } + } + + private func longestCommonSubsequenceLength(_ lhs: [String], _ rhs: [String]) -> Int { + var lengths = Array(repeating: Array(repeating: 0, count: rhs.count + 1), count: lhs.count + 1) + for i in stride(from: lhs.count - 1, through: 0, by: -1) { + for j in stride(from: rhs.count - 1, through: 0, by: -1) { + lengths[i][j] = lhs[i] == rhs[j] + ? lengths[i + 1][j + 1] + 1 + : max(lengths[i + 1][j], lengths[i][j + 1]) + } + } + return lengths[0][0] + } +} diff --git a/TableProTests/Models/Schema/ColumnReorderPolicyTests.swift b/TableProTests/Models/Schema/ColumnReorderPolicyTests.swift new file mode 100644 index 000000000..edd7402f6 --- /dev/null +++ b/TableProTests/Models/Schema/ColumnReorderPolicyTests.swift @@ -0,0 +1,98 @@ +// +// ColumnReorderPolicyTests.swift +// TablePro +// + +import Foundation +@testable import TablePro +import Testing + +@Suite("Column Reorder Policy") +struct ColumnReorderPolicyTests { + private func resolve( + support: ColumnReorderSupport = .alter, + isColumnsTab: Bool = true, + isTable: Bool = true, + canEditSchema: Bool = true, + hasStagedChanges: Bool = false, + isRearranged: Bool = false + ) -> ColumnReorderAvailability { + ColumnReorderPolicy.resolve( + support: support, + engineName: "PostgreSQL", + isColumnsTab: isColumnsTab, + isTable: isTable, + canEditSchema: canEditSchema, + hasStagedChanges: hasStagedChanges, + isRearranged: isRearranged + ) + } + + @Test("A positional engine on a clean column list can reorder") + func alterEngineIsAvailable() { + #expect(resolve() == .available(.alter)) + } + + @Test("A rebuild engine is available too, and says so, because the cost is decided later") + func rebuildEngineIsAvailable() { + #expect(resolve(support: .rebuild) == .available(.rebuild)) + } + + @Test("An engine that cannot reorder names itself in the reason") + func unsupportedEngineExplainsItself() { + let availability = resolve(support: .unsupported) + #expect(!availability.isAvailable) + #expect(availability.unavailableReason?.contains("PostgreSQL") == true) + } + + @Test("A list that has no order to change is not explained, only withheld") + func nonColumnTabIsNotApplicable() { + #expect(resolve(isColumnsTab: false) == .notApplicable) + #expect(resolve(isColumnsTab: false).unavailableReason == nil) + } + + @Test("An engine whose structure is read-only is withheld before its reorder support is read") + func readOnlyStructureOutranksSupport() { + let availability = resolve(support: .alter, canEditSchema: false) + #expect(!availability.isAvailable) + #expect(availability.unavailableReason?.contains("PostgreSQL") == true) + } + + @Test("Staged edits withhold the drag, because a reorder runs against the saved table") + func stagedChangesWithholdTheDrag() { + let availability = resolve(hasStagedChanges: true) + #expect(!availability.isAvailable) + #expect(availability.unavailableReason != nil) + } + + @Test("Staged edits on an engine that cannot reorder report the engine, not the edits") + func unsupportedOutranksStagedChanges() { + let availability = resolve(support: .unsupported, hasStagedChanges: true) + #expect(availability.unavailableReason?.contains("PostgreSQL") == true) + } + + /// A drop reports a position in what is on screen. Filtered or sorted, that is not the table's + /// order, and the delegate hands the position over without mapping it back, so the drag is + /// withheld rather than acted on against the wrong column. + /// Every mechanism emits table DDL, and the SQLite one looks its target up as a table, so a + /// view drag would end in a statement error instead of an explanation. + @Test("A view is withheld, whatever the engine can do to a table") + func viewWithholdsTheDrag() { + let availability = resolve(isTable: false) + #expect(!availability.isAvailable) + #expect(availability.unavailableReason != nil) + } + + @Test("A filtered or sorted column list withholds the drag") + func rearrangedListWithholdsTheDrag() { + let availability = resolve(isRearranged: true) + #expect(!availability.isAvailable) + #expect(availability.unavailableReason != nil) + } + + @Test("Staged edits outrank a rearranged list, because saving is the first thing to do") + func stagedChangesOutrankRearrangement() { + let staged = resolve(hasStagedChanges: true, isRearranged: true) + #expect(staged.unavailableReason == resolve(hasStagedChanges: true).unavailableReason) + } +} diff --git a/TableProTests/Models/Schema/SQLiteTableDDLTests.swift b/TableProTests/Models/Schema/SQLiteTableDDLTests.swift new file mode 100644 index 000000000..4047e7b69 --- /dev/null +++ b/TableProTests/Models/Schema/SQLiteTableDDLTests.swift @@ -0,0 +1,190 @@ +// +// SQLiteTableDDLTests.swift +// TablePro +// + +import Foundation +import TableProPluginKit +@testable import TablePro +import Testing + +@Suite("SQLite Table DDL") +struct SQLiteTableDDLTests { + /// The statement SQLite stores for a table carrying every trap the splitter has to survive: a + /// comma inside a string default, a comma inside a type's parentheses, a comma inside a + /// generated expression, a table `CHECK` and a table `UNIQUE`. + private let createSQL = """ + CREATE TABLE x( + a INTEGER PRIMARY KEY, + b TEXT NOT NULL DEFAULT 'hi, there' COLLATE NOCASE, + c DECIMAL(10,2) CHECK (c > 0), + d TEXT GENERATED ALWAYS AS (b || ',' || a) VIRTUAL, + pid INTEGER REFERENCES parent(id), + CHECK (length(b) < 100), + UNIQUE(a, b) + ) + """ + + @Test("Every column is found, and a table constraint is not mistaken for one") + func parseSeparatesColumnsFromTableConstraints() throws { + let parsed = try #require(SQLiteTableDDL.parse(createTableSQL: createSQL)) + #expect(parsed.columnNames == ["a", "b", "c", "d", "pid"]) + #expect(parsed.entries.count == 7) + } + + @Test("A quoted column name keeps its spelling and its commas") + func parseHandlesQuotedIdentifiers() throws { + let sql = "CREATE TABLE t(\"a, b\" TEXT, `c,d` INT, [e,f] INT, plain INT)" + let parsed = try #require(SQLiteTableDDL.parse(createTableSQL: sql)) + #expect(parsed.columnNames == ["a, b", "c,d", "e,f", "plain"]) + } + + @Test("A statement with no column list is refused rather than half parsed") + func parseRefusesCreateTableAsSelect() { + #expect(SQLiteTableDDL.parse(createTableSQL: "CREATE TABLE t AS SELECT 1") == nil) + #expect(SQLiteTableDDL.parse(createTableSQL: "CREATE TABLE t AS SELECT (1)") == nil) + } + + /// `sqlite_master` stores an FTS5 table as a `CREATE VIRTUAL TABLE` whose parentheses parse + /// exactly like a column list. Rebuilding one recreates it as a plain table and drops the index + /// and its shadow tables with the original, so it has to be refused before that. + @Test("A virtual table is refused, whatever its module", arguments: [ + "CREATE VIRTUAL TABLE docs USING fts5(title, body)", + "CREATE VIRTUAL TABLE t USING rtree(id, minX, maxX)", + "CREATE VIRTUAL TABLE IF NOT EXISTS v USING fts4(a, b)" + ]) + func parseRefusesVirtualTables(sql: String) { + #expect(SQLiteTableDDL.parse(createTableSQL: sql) == nil) + } + + @Test("The ordinary forms are still accepted", arguments: [ + "CREATE TABLE t(a INT)", + "CREATE TEMP TABLE t(a INT)", + "CREATE TEMPORARY TABLE t(a INT)", + "CREATE TABLE IF NOT EXISTS t(a INT)", + "CREATE TABLE \"my (odd) name\"(a INT)" + ]) + func parseAcceptsOrdinaryTables(sql: String) { + #expect(SQLiteTableDDL.parse(createTableSQL: sql) != nil) + } + + @Test("Reordering moves the column definitions verbatim and leaves the constraints in place") + func reorderMovesDefinitionsVerbatim() throws { + let parsed = try #require(SQLiteTableDDL.parse(createTableSQL: createSQL)) + let sql = try #require( + SQLiteTableDDL.reordered(parsed, to: ["pid", "a", "d", "b", "c"], tableName: "x_new") + ) + #expect(sql.contains("DEFAULT 'hi, there' COLLATE NOCASE")) + #expect(sql.contains("DECIMAL(10,2) CHECK (c > 0)")) + #expect(sql.contains("GENERATED ALWAYS AS (b || ',' || a) VIRTUAL")) + #expect(sql.contains("CHECK (length(b) < 100)")) + #expect(sql.contains("UNIQUE(a, b)")) + #expect(sql.hasPrefix("CREATE TABLE \"x_new\" (")) + + let reparsed = try #require(SQLiteTableDDL.parse(createTableSQL: sql)) + #expect(reparsed.columnNames == ["pid", "a", "d", "b", "c"]) + } + + @Test("Trailing table options survive the rewrite") + func reorderKeepsTrailingOptions() throws { + let sql = "CREATE TABLE t(a INT, b INT, PRIMARY KEY(a)) WITHOUT ROWID" + let parsed = try #require(SQLiteTableDDL.parse(createTableSQL: sql)) + let rewritten = try #require(SQLiteTableDDL.reordered(parsed, to: ["b", "a"], tableName: "t_new")) + #expect(rewritten.hasSuffix("WITHOUT ROWID")) + } + + @Test("An order that is not a permutation of the columns is refused") + func reorderRefusesNonPermutation() throws { + let parsed = try #require(SQLiteTableDDL.parse(createTableSQL: createSQL)) + #expect(SQLiteTableDDL.reordered(parsed, to: ["a", "b"], tableName: "x_new") == nil) + #expect(SQLiteTableDDL.reordered(parsed, to: ["a", "b", "c", "d", "zz"], tableName: "x_new") == nil) + } + + // MARK: - Plan + + /// Named apart from the `plan` each test binds. Shadowing it compiled here and failed on the + /// CI toolchain with "cannot call value of non-function type", because a local declaration is + /// in scope inside its own initializer. + private func makePlan(desiredOrder: [String]) -> PluginColumnReorderPlan? { + SQLiteColumnReorderPlanner.plan( + tableName: "x", + createTableSQL: createSQL, + desiredOrder: desiredOrder, + copyableColumns: ["a", "b", "c", "pid"], + dependentObjectSQL: ["CREATE INDEX ix_x_b ON x(b)"], + autoincrementHighWaterMark: nil, + foreignKeysWereOn: true, + isRunnable: true + ) + } + + @Test("The script follows SQLite's documented rebuild, in its order") + func planFollowsDocumentedProcedure() throws { + let plan = try #require(makePlan(desiredOrder: ["pid", "a", "d", "b", "c"])) + #expect(plan.cost == .tableRebuild) + #expect(plan.statements[0].hasPrefix("CREATE TABLE \"x_tablepro_reorder\"")) + #expect(plan.statements[2] == "DROP TABLE \"x\"") + #expect(plan.statements[3] == "ALTER TABLE \"x_tablepro_reorder\" RENAME TO \"x\"") + #expect(plan.statements.contains("CREATE INDEX ix_x_b ON x(b)")) + } + + /// The transaction is the executor's, not the plan's. Both places that run a plan open one + /// already, so a `BEGIN` in the statements would nest inside theirs and fail. + @Test("The plan carries no transaction control of its own") + func planCarriesNoTransactionStatements() throws { + let plan = try #require(makePlan(desiredOrder: ["pid", "a", "d", "b", "c"])) + #expect(plan.isTransactional) + for keyword in ["BEGIN", "COMMIT", "ROLLBACK"] { + #expect(!plan.statements.contains { $0.uppercased().hasPrefix(keyword) }) + } + } + + /// Restored to what it was, not forced on. This driver opens connections with foreign keys off, + /// so forcing them on turns later writes on the same connection into constraint failures. + @Test("The foreign-key pragma is put back the way it was", arguments: [true, false]) + func planRestoresTheForeignKeyPragma(wasOn: Bool) throws { + let plan = try #require(SQLiteColumnReorderPlanner.plan( + tableName: "x", + createTableSQL: createSQL, + desiredOrder: ["pid", "a", "d", "b", "c"], + copyableColumns: ["a", "b", "c", "pid"], + dependentObjectSQL: [], + autoincrementHighWaterMark: nil, + foreignKeysWereOn: wasOn, + isRunnable: true + )) + #expect(plan.prologue == ["PRAGMA foreign_keys = off"]) + #expect(plan.epilogue == ["PRAGMA foreign_keys = \(wasOn ? "on" : "off")"]) + } + + /// `DROP TABLE` takes the table's `sqlite_sequence` row with it, so without this the rebuilt + /// table is seeded from the rows copied rather than the highest id ever issued, and the next + /// insert reuses one that was already handed out. + @Test("An AUTOINCREMENT table keeps its high-water mark") + func planRestoresTheAutoincrementHighWaterMark() throws { + let plan = try #require(SQLiteColumnReorderPlanner.plan( + tableName: "x", + createTableSQL: createSQL, + desiredOrder: ["pid", "a", "d", "b", "c"], + copyableColumns: ["a", "b", "c", "pid"], + dependentObjectSQL: [], + autoincrementHighWaterMark: 42, + foreignKeysWereOn: false, + isRunnable: true + )) + #expect(plan.statements.contains("UPDATE sqlite_sequence SET seq = 42 WHERE name = 'x'")) + } + + @Test("The copy names only the columns INSERT accepts, leaving the generated one out") + func planExcludesGeneratedColumnsFromTheCopy() throws { + let plan = try #require(makePlan(desiredOrder: ["pid", "a", "d", "b", "c"])) + let insert = try #require(plan.statements.first { $0.hasPrefix("INSERT INTO") }) + #expect(insert.contains("(\"a\", \"b\", \"c\", \"pid\")")) + #expect(!insert.contains("\"d\"")) + } + + @Test("An order that changes nothing produces no plan") + func planRefusesAnUnchangedOrder() { + #expect(makePlan(desiredOrder: ["a", "b", "c", "d", "pid"]) == nil) + } +} diff --git a/TableProTests/Views/Results/DataGridUpdateSnapshotTests.swift b/TableProTests/Views/Results/DataGridUpdateSnapshotTests.swift index 770f966fd..e37360eff 100644 --- a/TableProTests/Views/Results/DataGridUpdateSnapshotTests.swift +++ b/TableProTests/Views/Results/DataGridUpdateSnapshotTests.swift @@ -26,7 +26,7 @@ struct DataGridUpdateSnapshotTests { displayFormats: displayFormats, configuration: DataGridConfiguration(), isEditable: true, - hasMoveDelegate: false, + rowReorder: .disabled, rowHeight: 24, alternatingRows: true, reloadVersion: reloadVersion, diff --git a/docs/features/table-structure.mdx b/docs/features/table-structure.mdx index 5a8c5a7f8..b23b8f95a 100644 --- a/docs/features/table-structure.mdx +++ b/docs/features/table-structure.mdx @@ -44,7 +44,18 @@ Add a column with **+** at the right of the status bar or `Cmd+Shift+N`. Select Flag **Primary Key** on one column, or several for a composite key in one `PRIMARY KEY (col1, col2)` clause. On an existing table that becomes a drop of the old constraint followed by an add. -Drag a column row to reorder it (MySQL and MariaDB only). That one runs immediately as `ALTER TABLE … MODIFY COLUMN … AFTER` and goes to query history rather than the queue. Dragging is off while unsaved changes exist. +Drag a column row to reorder it. The row number carries a tooltip saying why when it is off: while unsaved changes exist, while the list is filtered or sorted, on a view, and on an engine that cannot change column order. + +What the drag does depends on the engine: + +| Engine | What happens | +|--------|--------------| +| MySQL, MariaDB, ClickHouse | Runs `ALTER TABLE … MODIFY COLUMN … FIRST \| AFTER` straight away. No rows are read or written | +| Oracle | Runs a pair of `MODIFY (col INVISIBLE)` and `MODIFY (col VISIBLE)` statements per column that has to move, which appends it to the end of the order. Needs Oracle 12.1. No rows are read or written | +| SQLite, local-file libSQL | Shows the rebuild script, and runs it when you confirm. The table is recreated in the wanted order, the rows are copied, and the indexes and triggers are put back | +| PostgreSQL, Turso, remote libSQL, Cloudflare D1 | Shows the rebuild script for you to read and run. TablePro does not run it: the script ends in `DROP TABLE`, and what it cannot carry over is listed above it | + +Everything that runs goes to query history rather than the change queue. ## Indexes tab