From faff41df548dca40e726d37f655d880a17ebbec8 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Sat, 26 Sep 2026 19:30:13 +0700 Subject: [PATCH] fix(structure): let a save reuse the name of an index or check constraint it deletes --- CHANGELOG.md | 6 + .../MySQLCreateTableSQL.swift | 10 + .../MySQLDriverPlugin/MySQLPluginDriver.swift | 3 +- .../SchemaOperationRefusal.swift | 3 +- .../SchemaStatementGenerator.swift | 121 +++++- .../StructureChangeManager.swift | 158 ++++++-- TablePro/Models/Schema/IndexDefinition.swift | 15 - TablePro/Resources/Localizable.xcstrings | 3 + .../CheckConstraintStatementTests.swift | 95 +++++ .../SchemaOperationRefusalTests.swift | 94 ++++- .../SchemaStatementGeneratorPluginTests.swift | 170 ++++++++- ...tureChangeManagerClusteredIndexTests.swift | 85 +++++ .../StructureChangeNameReuseTests.swift | 345 ++++++++++++++++++ .../DynamoDBTableManagementTests.swift | 41 +++ .../Plugins/MySQLIndexKeyWriterTests.swift | 10 + .../StructureIndexNameReuseUITests.swift | 134 +++++++ docs/databases/dynamodb.mdx | 2 +- docs/features/table-structure.mdx | 12 +- 18 files changed, 1241 insertions(+), 66 deletions(-) create mode 100644 TableProTests/Core/SchemaTracking/StructureChangeNameReuseTests.swift create mode 100644 TableProUITests/StructureIndexNameReuseUITests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index fd43cf0c69..7013d11212 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -196,6 +196,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **New Trigger** offered on a materialized view. - Structure tab refusing every save on a SQLite, libSQL or Cloudflare D1 table with a column that has no declared type. - Structure tab refusing to save a renamed or dropped primary key column. +- Structure tab refusing to save an index or check constraint named like one deleted in the same save. +- Structure tab refusing every save on a SQLite table with two check constraints of one name. +- Primary key lost on MySQL and MariaDB when a save replaced the `PRIMARY` index row and changed a column. +- Structure tab save failing partway when index or check constraint renames swapped names or took a freed one. +- Clustered index lost on SQL Server when a duplicate of it replaced the original. +- Structure tab accepting a check constraint named like another in a different letter case. - Compressed dump named `.GZ` rather than `.gz` reaching the parser still compressed. - **SQL** offered as an import format on MongoDB. - **Save** permanently dim on a Custom provider for an OpenAI-compatible server that wants no API key. diff --git a/Plugins/MySQLDriverPlugin/MySQLCreateTableSQL.swift b/Plugins/MySQLDriverPlugin/MySQLCreateTableSQL.swift index be602eeb2b..882d76a635 100644 --- a/Plugins/MySQLDriverPlugin/MySQLCreateTableSQL.swift +++ b/Plugins/MySQLDriverPlugin/MySQLCreateTableSQL.swift @@ -128,6 +128,16 @@ internal func mysqlModifyIndexSQL( + "ADD \(mysqlIndexDefinitionSQL(newIndex))" } +/// `PRIMARY`, in any case, names the primary key and nothing else: MariaDB 13 refuses +/// `ADD INDEX primary (b)` with ERROR 1280. Refused before the save, because a save that splits a +/// replacement of the key's row drops the key first and then cannot add anything back. +internal func mysqlReservedIndexNameRefusal(for index: PluginIndexDefinition) -> String? { + guard index.name.caseInsensitiveCompare("PRIMARY") == .orderedSame else { return nil } + return String( + localized: "PRIMARY is the primary key's name, and no other index can take it. Rename the index, or change the key on the Columns tab." + ) +} + /// `CONSTRAINT name` is optional in MySQL's grammar and the server invents one when it is left out, /// so a blank name writes no clause rather than `CONSTRAINT `` `` ``, which is a syntax error. internal func mysqlForeignKeyDefinitionSQL(_ foreignKey: PluginForeignKeyDefinition) -> String { diff --git a/Plugins/MySQLDriverPlugin/MySQLPluginDriver.swift b/Plugins/MySQLDriverPlugin/MySQLPluginDriver.swift index b37391dc15..1d75aea633 100644 --- a/Plugins/MySQLDriverPlugin/MySQLPluginDriver.swift +++ b/Plugins/MySQLDriverPlugin/MySQLPluginDriver.swift @@ -1057,7 +1057,8 @@ final class MySQLPluginDriver: PluginDatabaseDriver, @unchecked Sendable { func schemaOperationRefusal(_ operation: PluginSchemaOperation) -> String? { guard case .addIndex(let index) = operation else { return nil } let identity = serverIdentity - return MySQLFunctionalKeyParts.refusal(for: index, banner: identity.banner, flavor: identity.flavor) + return mysqlReservedIndexNameRefusal(for: index) + ?? MySQLFunctionalKeyParts.refusal(for: index, banner: identity.banner, flavor: identity.flavor) } func generateAddForeignKeySQL(table: String, fk: PluginForeignKeyDefinition) -> String? { diff --git a/TablePro/Core/SchemaTracking/SchemaOperationRefusal.swift b/TablePro/Core/SchemaTracking/SchemaOperationRefusal.swift index bf0860faf8..e1d467f493 100644 --- a/TablePro/Core/SchemaTracking/SchemaOperationRefusal.swift +++ b/TablePro/Core/SchemaTracking/SchemaOperationRefusal.swift @@ -20,7 +20,8 @@ internal enum SchemaOperationRefusal { case .addIndex(let index): return driver.schemaOperationRefusal(.addIndex(index.toPlugin())) case .modifyIndex(let old, let new): - return driver.schemaOperationRefusal(.modifyIndex(old: old.toPlugin(), new: new.toPlugin())) + return driver.schemaOperationRefusal(.dropIndex(old.toPlugin())) + ?? driver.schemaOperationRefusal(.modifyIndex(old: old.toPlugin(), new: new.toPlugin())) ?? driver.schemaOperationRefusal(.addIndex(new.toPlugin())) case .deleteIndex(let index): return driver.schemaOperationRefusal(.dropIndex(index.toPlugin())) diff --git a/TablePro/Core/SchemaTracking/SchemaStatementGenerator.swift b/TablePro/Core/SchemaTracking/SchemaStatementGenerator.swift index 891862c155..e25f6e131e 100644 --- a/TablePro/Core/SchemaTracking/SchemaStatementGenerator.swift +++ b/TablePro/Core/SchemaTracking/SchemaStatementGenerator.swift @@ -44,7 +44,9 @@ struct SchemaStatementGenerator { func generate(changes: [SchemaChange]) throws -> [SchemaStatement] { var statements: [SchemaStatement] = [] - let refusals = changes.lazy.compactMap { SchemaOperationRefusal.reason(for: $0, driver: pluginDriver) } + let refusals = Self.replacingIndexesInPlace(changes).lazy.compactMap { + SchemaOperationRefusal.reason(for: $0, driver: pluginDriver) + } if let reason = refusals.first { throw SchemaOperationRefusedError(reason: reason) } @@ -70,7 +72,7 @@ struct SchemaStatementGenerator { // MARK: - Dependency Ordering - private func sortByDependency(_ changes: [SchemaChange]) -> [SchemaChange] { + private func sortByDependency(_ staged: [SchemaChange]) -> [SchemaChange] { // Execution order for safety: // 1. Drop foreign keys first (includes modify FK, which requires drop+recreate) // 2. Drop indexes (a modified index drops here and is added at 6, unless the driver @@ -80,6 +82,10 @@ struct SchemaStatementGenerator { // 5. Modify primary key // 6. Add indexes // 7. Add foreign keys + // Every drop of an index or a check constraint runs before any add or rename of one, and a + // rename or add runs after the rename that frees its name. The structure editor counts a + // deleted or renamed row's name as free on that basis. + let changes = Self.replacingIndexesInPlace(staged) var constraintDeletes: [SchemaChange] = [] var constraintModifies: [SchemaChange] = [] @@ -144,8 +150,83 @@ struct SchemaStatementGenerator { } } - return constraintDeletes + constraintModifies + fkDeletes + indexDeletes + columnDeletes - + columnModifies + columnAdds + pkChanges + indexAdds + fkAdds + constraintAdds + let constraintHandoffs = Self.inNameOrder(constraintModifies) + let indexHandoffs = Self.inNameOrder(indexAdds) + + return constraintDeletes + constraintHandoffs.drops + constraintHandoffs.ordered + fkDeletes + + indexDeletes + indexHandoffs.drops + columnDeletes + columnModifies + columnAdds + pkChanges + + indexHandoffs.ordered + fkAdds + constraintAdds + } + + /// Orders changes that each give up one name and take another in a single step, a rename or a + /// one-statement index replacement, so that none takes a name before the change holding it has + /// let it go. Staging order is kept wherever nothing forces another. + /// + /// Changes that hold each other's names in a cycle, such as two indexes trading names, cannot run + /// whole in any order, so each is split into a drop, returned to run with the other drops, and an + /// add that then waits for nothing. Measured on MariaDB 13.0.2: renaming `c` to `b` before `b` to + /// `a` fails with ERROR 1061 or 1826 after the drops in the same save have run, and a swap fails + /// at its first `ALTER TABLE`. + private static func inNameOrder(_ changes: [SchemaChange]) -> (drops: [SchemaChange], ordered: [SchemaChange]) { + var pending = changes + var drops: [SchemaChange] = [] + while true { + let handoff = orderedByNameHandoff(pending) + let halves = handoff.cyclic.compactMap(splitIntoDropAndAdd) + guard !halves.isEmpty else { return (drops, handoff.ordered + handoff.cyclic) } + drops += halves.map(\.drop) + pending = handoff.ordered + handoff.cyclic.filter { splitIntoDropAndAdd($0) == nil } + halves.map(\.add) + } + } + + private static func orderedByNameHandoff( + _ changes: [SchemaChange] + ) -> (ordered: [SchemaChange], cyclic: [SchemaChange]) { + var pending = changes + var ordered: [SchemaChange] = [] + while let next = pending.indices.first(where: { !takesAHeldName(at: $0, in: pending) }) { + ordered.append(pending.remove(at: next)) + } + return (ordered, pending) + } + + /// Names compare without regard to case, as MySQL, MariaDB and SQLite resolve index and check + /// constraint names. On PostgreSQL, which keeps `c` and `C` apart, that only adds an ordering + /// nothing needed. + private static func takesAHeldName(at position: Int, in pending: [SchemaChange]) -> Bool { + guard let taken = nameHandoff(of: pending[position]).taken?.lowercased() else { return false } + return pending.indices.contains { other in + other != position && nameHandoff(of: pending[other]).released?.lowercased() == taken + } + } + + private static func nameHandoff(of change: SchemaChange) -> (released: String?, taken: String?) { + switch change { + case .modifyIndex(let old, let new): + return (old.name, new.name) + case .modifyCheckConstraint(let old, let new): + return (old.name, new.name) + case .addIndex(let index): + return (nil, index.name) + case .addCheckConstraint(let constraint): + return (nil, constraint.name) + case .addColumn, .modifyColumn, .deleteColumn, .deleteIndex, .addForeignKey, .modifyForeignKey, + .deleteForeignKey, .modifyPrimaryKey, .deleteCheckConstraint: + return (nil, nil) + } + } + + private static func splitIntoDropAndAdd(_ change: SchemaChange) -> (drop: SchemaChange, add: SchemaChange)? { + switch change { + case .modifyIndex(let old, let new): + return (.deleteIndex(old), .addIndex(new)) + case .modifyCheckConstraint(let old, let new): + return (.deleteCheckConstraint(old), .addCheckConstraint(new)) + case .addColumn, .modifyColumn, .deleteColumn, .addIndex, .deleteIndex, .addForeignKey, + .modifyForeignKey, .deleteForeignKey, .modifyPrimaryKey, .addCheckConstraint, + .deleteCheckConstraint: + return nil + } } private static func changesColumns(_ change: SchemaChange) -> Bool { @@ -158,6 +239,38 @@ struct SchemaStatementGenerator { } } + /// An index deleted and another added under its name in the same save replace that index, which + /// is what `.modifyIndex` describes. Each add takes the first unpaired delete of its name, so a + /// second pass finds nothing left to pair. + /// + /// Paired, the save asks the driver about a replacement rather than a drop and an unrelated add, + /// and a driver that replaces an index in one statement does so: MySQL and MariaDB run + /// `DROP INDEX idx_a, ADD INDEX idx_a (…)` as one `ALTER TABLE`, and keep the old index when the + /// server refuses the new one. Everywhere else it splits back into the drop and the add. + static func replacingIndexesInPlace(_ changes: [SchemaChange]) -> [SchemaChange] { + var unpairedDrops: [EditableIndexDefinition] = changes.compactMap { change in + guard case .deleteIndex(let index) = change else { return nil } + return index + } + var replaced: [UUID: EditableIndexDefinition] = [:] + for case .addIndex(let added) in changes { + guard let position = unpairedDrops.firstIndex(where: { $0.name == added.name }) else { continue } + replaced[added.id] = unpairedDrops.remove(at: position) + } + let pairedDropIDs = Set(replaced.values.map(\.id)) + return changes.compactMap { change in + switch change { + case .deleteIndex(let dropped) where pairedDropIDs.contains(dropped.id): + return nil + case .addIndex(let added): + guard let dropped = replaced[added.id] else { return change } + return .modifyIndex(old: dropped, new: added) + default: + return change + } + } + } + // MARK: - Statement Generation private func generateStatements(for change: SchemaChange) throws -> [SchemaStatement] { diff --git a/TablePro/Core/SchemaTracking/StructureChangeManager.swift b/TablePro/Core/SchemaTracking/StructureChangeManager.swift index e108305759..e28a617682 100644 --- a/TablePro/Core/SchemaTracking/StructureChangeManager.swift +++ b/TablePro/Core/SchemaTracking/StructureChangeManager.swift @@ -35,6 +35,10 @@ final class StructureChangeManager: ObservableObject, ChangeManaging { @Published var tableName: String? + /// Indexes added as `CLUSTERED`, whose type `settleClusteredAdditions` keeps deciding until the + /// user picks one for the row. + private var indexesAddedClustered: Set = [] + // MARK: - Undo/Redo Support /// Private `NSUndoManager` owned by this change manager. Each @@ -114,6 +118,7 @@ final class StructureChangeManager: ObservableObject, ChangeManaging { pendingChanges.removeAll() changeOrder.removeAll() validationErrors.removeAll() + indexesAddedClustered.removeAll() undoManager.removeAllActions() // Increment reloadVersion to trigger DataGridView column width recalculation @@ -163,11 +168,11 @@ final class StructureChangeManager: ObservableObject, ChangeManaging { stageAddition(column, using: Self.columnOperations) } - /// A deleted index is left out of what the copy is added beside, because the save drops every - /// index before it adds one. func addIndex(_ index: EditableIndexDefinition) { - let remaining = workingIndexes.filter { pendingChanges[.index($0.id)]?.isDelete != true } - stageAddition(index.addedBeside(remaining), using: Self.indexOperations) + if index.type == .clustered { + indexesAddedClustered.insert(index.id) + } + stageAddition(index, using: Self.indexOperations) } func addForeignKey(_ foreignKey: EditableForeignKeyDefinition) { @@ -191,6 +196,9 @@ final class StructureChangeManager: ObservableObject, ChangeManaging { // MARK: - Index Operations func updateIndex(id: UUID, with newIndex: EditableIndexDefinition) { + if workingIndexes.first(where: { $0.id == id })?.type != newIndex.type { + indexesAddedClustered.remove(id) + } stageEdit(id: id, with: newIndex, using: Self.indexOperations) } @@ -239,7 +247,7 @@ final class StructureChangeManager: ObservableObject, ChangeManaging { registerUndo(operations.addActionName) { target in target.applySchemaUndo(operations.additionUndo(entity)) } - validate() + workingCopyDidChange() } private func stageEdit( @@ -275,7 +283,7 @@ final class StructureChangeManager: ObservableObject, ChangeManaging { self[keyPath: operations.working][workingIndex] = newEntity } - validate() + workingCopyDidChange() } private func stageDeletion(id: UUID, using operations: SchemaEntityOperations) { @@ -298,7 +306,7 @@ final class StructureChangeManager: ObservableObject, ChangeManaging { untrackChangeKey(key) } - validate() + workingCopyDidChange() } private static let columnOperations = SchemaEntityOperations( @@ -393,11 +401,47 @@ final class StructureChangeManager: ObservableObject, ChangeManaging { guard pendingChanges[key]?.isDelete == true else { return } pendingChanges.removeValue(forKey: key) untrackChangeKey(key) - validate() + workingCopyDidChange() } // MARK: - Validation + /// Runs after every change to the working copy, undo and redo included: first what the change + /// decides for rows it did not touch, then validation. + private func workingCopyDidChange() { + settleClusteredAdditions() + validate() + } + + /// A SQL Server table keeps its rows in the order of one clustered index, and its primary key is + /// that index unless it says otherwise. An index added as `CLUSTERED`, which is what Duplicate + /// and a paste stage for a copy of that index, takes the place only when no other index the + /// table keeps after the save holds it, and is written `NONCLUSTERED` beside one that does, the + /// type the server reports for every other index. Two `CLUSTERED` indexes are refused with + /// "Cannot create more than one clustered index on table". + /// + /// Decided again after every change, because the place frees when its holder is deleted, and + /// Duplicate, edit the copy, then delete the original is how an index is replaced. Decided once, + /// the copy stayed `NONCLUSTERED` and the save left the table with no clustered index. A row + /// whose type was changed to anything else is the user's own choice and is left alone. + private func settleClusteredAdditions() { + var placeIsTaken = workingIndexes.contains { index in + !hasTypeSettledByTheEditor(index) && !isPendingDeletion(.index(index.id)) && index.type.ordersTableRows + } + for position in workingIndexes.indices where hasTypeSettledByTheEditor(workingIndexes[position]) { + var index = workingIndexes[position] + index.type = placeIsTaken ? .nonclustered : .clustered + placeIsTaken = true + guard index != workingIndexes[position] else { continue } + workingIndexes[position] = index + pendingChanges[.index(index.id)] = .addIndex(index) + } + } + + private func hasTypeSettledByTheEditor(_ index: EditableIndexDefinition) -> Bool { + indexesAddedClustered.contains(index.id) && (index.type == .clustered || index.type == .nonclustered) + } + private func validate() { validationErrors.removeAll() @@ -415,17 +459,8 @@ final class StructureChangeManager: ObservableObject, ChangeManaging { ) } - let indexNames = workingIndexes.filter { $0.isValid }.map { $0.name } - let duplicateIndexes = Dictionary(grouping: indexNames, by: { $0 }) - .filter { $0.value.count > 1 } - .map { $0.key } - - for duplicate in duplicateIndexes { - for index in workingIndexes.filter({ $0.name == duplicate }) { - validationErrors[.index(index.id)] = String( - format: String(localized: "Duplicate index name: %@"), duplicate - ) - } + flagDuplicateNames(using: Self.indexOperations, name: \.name, isNamed: \.isValid, comparedAs: { $0 }) { + String(format: String(localized: "Duplicate index name: %@"), $0) } /// Only a row this save actually edits is checked against the columns. @@ -468,18 +503,12 @@ final class StructureChangeManager: ObservableObject, ChangeManaging { ) } - let constraintNames = workingCheckConstraints.filter { $0.isValid }.map { $0.name } - let duplicateConstraints = Dictionary(grouping: constraintNames, by: { $0 }) - .filter { $0.value.count > 1 } - .map { $0.key } - - for duplicate in duplicateConstraints { - for constraint in workingCheckConstraints.filter({ $0.name == duplicate }) { - validationErrors[.checkConstraint(constraint.id)] = String( - format: String(localized: "Duplicate constraint name: %@"), duplicate - ) - } + flagDuplicateNames( + using: Self.checkConstraintOperations, name: \.name, isNamed: \.isValid, comparedAs: Self.constraintNameKey + ) { + String(format: String(localized: "Duplicate constraint name: %@"), $0) } + flagChangesToASharedConstraintName() /// Checked only when this save changes the key, as the index and foreign key rows are. A /// rename leaves the loaded key naming the old spelling, and every engine's `RENAME COLUMN` @@ -493,7 +522,7 @@ final class StructureChangeManager: ObservableObject, ChangeManaging { /// Every column the table keeps after this save, whatever state its name and type are in. private var columnsAfterSave: [EditableColumnDefinition] { - workingColumns.filter { !isColumnPendingDeletion($0.id) } + workingColumns.filter { !isPendingDeletion(.column($0.id)) } } /// Only a column this save adds or changes is held to being complete. @@ -527,6 +556,59 @@ final class StructureChangeManager: ObservableObject, ChangeManaging { } } + /// A name counts once for each row the table keeps, and blocks the save only when the save put + /// one of those rows there, the rule columns follow. + /// + /// A row being deleted keeps nothing: the save drops every index and check constraint before it + /// adds or renames one, so its name is free again by then. Deleting an index and adding one + /// under its name used to be refused as a duplicate of the very row being dropped. + /// + /// `comparedAs` gives the key two names must share to be one name to the database. + private func flagDuplicateNames( + using operations: SchemaEntityOperations, + name: KeyPath, + isNamed: KeyPath, + comparedAs key: (String) -> String, + message: (String) -> String + ) { + let kept = self[keyPath: operations.working].filter { + $0[keyPath: isNamed] && !isPendingDeletion(operations.identifier($0.id)) + } + for rows in Dictionary(grouping: kept, by: { key($0[keyPath: name]) }).values where rows.count > 1 { + guard rows.contains(where: { isStaged(operations.identifier($0.id)) }) else { continue } + for row in rows { + validationErrors[operations.identifier(row.id)] = message(row[keyPath: name]) + } + } + } + + /// SQLite and MariaDB treat `c` and `C` as one check constraint name: measured on SQLite 3.54, + /// `ADD CONSTRAINT "C"` beside `c` fails with "constraint C already exists", and MariaDB 13.0.2 + /// refuses it with ERROR 1826. PostgreSQL keeps the two apart, and refusing such a pair there + /// runs nothing. + private static func constraintNameKey(_ name: String) -> String { + name.lowercased() + } + + /// SQLite accepts two table-level check constraints under one name, compares constraint names + /// without regard to case, and drops the first one it finds by that name. So a change to one + /// while another keeps the name can land on the other one. Changing every one of them is safe, + /// because each is dropped by name and added back from its own new definition. PostgreSQL keeps + /// `c` and `C` apart, and refusing a change to one of those runs nothing. + private func flagChangesToASharedConstraintName() { + let loaded = currentCheckConstraints.filter(\.isValid) + for rows in Dictionary(grouping: loaded, by: { Self.constraintNameKey($0.name) }).values where rows.count > 1 { + let changed = rows.filter { pendingChanges[.checkConstraint($0.id)] != nil } + guard !changed.isEmpty, changed.count < rows.count else { continue } + for row in changed { + validationErrors[.checkConstraint(row.id)] = String( + format: String(localized: "More than one check constraint is named %@. Change or delete all of them in the same save."), + row.name + ) + } + } + } + /// Whether this save changes the row, and is not simply removing it. /// /// A row on its way out is not held to being complete: the user struck through a foreign key @@ -537,6 +619,10 @@ final class StructureChangeManager: ObservableObject, ChangeManaging { return !change.isDelete } + private func isPendingDeletion(_ key: SchemaChangeIdentifier) -> Bool { + pendingChanges[key]?.isDelete == true + } + /// Identifiers compare case insensitively, the way every engine TablePro edits resolves them. /// SQLite accepts a column declared `ID` and referenced as `id`, and its pragmas report each /// spelling as written. @@ -553,13 +639,6 @@ final class StructureChangeManager: ObservableObject, ChangeManaging { return loaded.isNullable || !loaded.hasNullDefault } - private func isColumnPendingDeletion(_ id: UUID) -> Bool { - if case .deleteColumn = pendingChanges[.column(id)] { - return true - } - return false - } - // MARK: - State Management var canCommit: Bool { @@ -578,6 +657,7 @@ final class StructureChangeManager: ObservableObject, ChangeManaging { pendingChanges.removeAll() changeOrder.removeAll() validationErrors.removeAll() + indexesAddedClustered.removeAll() resetWorkingState() reloadVersion += 1 undoManager.removeAllActions() @@ -629,7 +709,7 @@ final class StructureChangeManager: ObservableObject, ChangeManaging { applyPrimaryKeyChangeUndo(old: old) } - validate() + workingCopyDidChange() } private func applyEditUndo( diff --git a/TablePro/Models/Schema/IndexDefinition.swift b/TablePro/Models/Schema/IndexDefinition.swift index e3af900195..b53354367b 100644 --- a/TablePro/Models/Schema/IndexDefinition.swift +++ b/TablePro/Models/Schema/IndexDefinition.swift @@ -258,21 +258,6 @@ struct EditableIndexDefinition: Hashable, Codable, Identifiable { return copy } - /// This index added to a table that keeps `remaining`, the indexes it will still have when the - /// add runs. - /// - /// A SQL Server table keeps its rows in the order of one clustered index, and its primary key is - /// that index unless it says otherwise. So a `CLUSTERED` copy beside one, which is what Duplicate - /// and a paste into the same table stage, is written `NONCLUSTERED`: the type the server reports - /// for every other index. Kept, the save wrote `CREATE CLUSTERED INDEX` and the server refused it - /// with "Cannot create more than one clustered index on table". - func addedBeside(_ remaining: [EditableIndexDefinition]) -> EditableIndexDefinition { - guard type == .clustered, remaining.contains(where: \.type.ordersTableRows) else { return self } - var added = self - added.type = .nonclustered - return added - } - /// A copy under a fresh identity for a paste into a table on `target`, copied from a table on /// `source`. /// diff --git a/TablePro/Resources/Localizable.xcstrings b/TablePro/Resources/Localizable.xcstrings index 55d3752f85..344d692d2d 100644 --- a/TablePro/Resources/Localizable.xcstrings +++ b/TablePro/Resources/Localizable.xcstrings @@ -184363,6 +184363,9 @@ }, "This database does not store whole documents" : { + }, + "PRIMARY is the primary key's name, and no other index can take it. Rename the index, or change the key on the Columns tab." : { + } }, "version" : "1.1" diff --git a/TableProTests/Core/SchemaTracking/CheckConstraintStatementTests.swift b/TableProTests/Core/SchemaTracking/CheckConstraintStatementTests.swift index 413b36c0ab..8184477206 100644 --- a/TableProTests/Core/SchemaTracking/CheckConstraintStatementTests.swift +++ b/TableProTests/Core/SchemaTracking/CheckConstraintStatementTests.swift @@ -140,6 +140,101 @@ struct CheckConstraintStatementTests { #expect(kinds[2].contains("ADD CONSTRAINT ck_new")) } + /// The structure editor lets a new constraint take a deleted one's name on the strength of this + /// order, whichever of the two was staged first. + @Test("a deleted constraint is dropped before another is added under its name") + func deletedNameIsFreeBeforeAnAdd() throws { + let statements = try generator(ConstraintDDLDriver()).generate(changes: [ + .addCheckConstraint(constraint(name: "ck", expression: "qty < 10")), + .deleteCheckConstraint(constraint(name: "ck", expression: "qty > 0")) + ]) + + #expect(statements.map(\.sql) == [ + "ALTER TABLE orders DROP CONSTRAINT ck;", + "ALTER TABLE orders ADD CONSTRAINT ck CHECK (qty < 10);" + ]) + } + + @Test("a deleted constraint is dropped before another is renamed onto its name") + func deletedNameIsFreeBeforeARename() throws { + let kept = constraint(name: "ck_b", expression: "qty < 10") + var renamed = kept + renamed.name = "ck" + + let statements = try generator(ConstraintDDLDriver()).generate(changes: [ + .modifyCheckConstraint(old: kept, new: renamed), + .deleteCheckConstraint(constraint(name: "ck", expression: "qty > 0")) + ]) + + #expect(statements.map(\.sql) == [ + "ALTER TABLE orders DROP CONSTRAINT ck;", + "ALTER TABLE orders RENAME CONSTRAINT ck_b TO ck;" + ]) + } + + private func renamed(_ name: String, to newName: String, expression: String) -> SchemaChange { + let old = constraint(name: name, expression: expression) + var new = old + new.name = newName + return .modifyCheckConstraint(old: old, new: new) + } + + /// `c` renamed to `b`, `a` deleted, then `b` renamed to `a`, in the order the user staged them. + private var renameChain: [SchemaChange] { + [ + renamed("c", to: "b", expression: "z > 0"), + .deleteCheckConstraint(constraint(name: "a", expression: "x > 0")), + renamed("b", to: "a", expression: "y > 0") + ] + } + + /// Measured on PostgreSQL 17.11: after `DROP CONSTRAINT a`, `RENAME CONSTRAINT c TO b` fails while + /// `b` exists, and renaming `b` to `a` first, then `c` to `b`, commits. + @Test("a rename onto a name another rename frees runs after that rename") + func renameChainRunsInNameOrder() throws { + let statements = try generator(ConstraintDDLDriver()).generate(changes: renameChain) + + #expect(statements.map(\.sql) == [ + "ALTER TABLE orders DROP CONSTRAINT a;", + "ALTER TABLE orders RENAME CONSTRAINT b TO a;", + "ALTER TABLE orders RENAME CONSTRAINT c TO b;" + ]) + } + + /// MySQL, MariaDB and SQLite have no `RENAME CONSTRAINT`, so each rename is a drop and an add. + /// Measured on MariaDB 13.0.2: in staging order the add of `b` fails with ERROR 1826 after `a` and + /// `c` are dropped, and the table is left with `b` alone. + @Test("a rename chain on an engine with no RENAME CONSTRAINT runs in name order") + func renameChainWithoutRenameRunsInNameOrder() throws { + let driver = ConstraintDDLDriver() + driver.supportsRename = false + + let statements = try generator(driver).generate(changes: renameChain) + + #expect(statements.map(\.sql) == [ + "ALTER TABLE orders DROP CONSTRAINT a;", + "ALTER TABLE orders DROP CONSTRAINT b;", + "ALTER TABLE orders ADD CONSTRAINT a CHECK (y > 0);", + "ALTER TABLE orders DROP CONSTRAINT c;", + "ALTER TABLE orders ADD CONSTRAINT b CHECK (z > 0);" + ]) + } + + @Test("two constraints trading names are dropped and added back") + func swappedNamesAreDroppedAndAddedBack() throws { + let statements = try generator(ConstraintDDLDriver()).generate(changes: [ + renamed("a", to: "b", expression: "x > 0"), + renamed("b", to: "a", expression: "y > 0") + ]) + + #expect(statements.map(\.sql) == [ + "ALTER TABLE orders DROP CONSTRAINT a;", + "ALTER TABLE orders DROP CONSTRAINT b;", + "ALTER TABLE orders ADD CONSTRAINT b CHECK (x > 0);", + "ALTER TABLE orders ADD CONSTRAINT a CHECK (y > 0);" + ]) + } + @Test("adding a check scans every existing row, so it counts as a data migration") func addingAConstraintRequiresDataMigration() { let change = SchemaChange.addCheckConstraint(constraint(name: "ck", expression: "a > 0")) diff --git a/TableProTests/Core/SchemaTracking/SchemaOperationRefusalTests.swift b/TableProTests/Core/SchemaTracking/SchemaOperationRefusalTests.swift index 8b8c2ec94f..18860b31dd 100644 --- a/TableProTests/Core/SchemaTracking/SchemaOperationRefusalTests.swift +++ b/TableProTests/Core/SchemaTracking/SchemaOperationRefusalTests.swift @@ -47,6 +47,9 @@ private final class RefusingDDLDriver: PluginDatabaseDriver, @unchecked Sendable func generateAddIndexSQL(table: String, index: PluginIndexDefinition) -> String? { "CREATE INDEX \(index.name) ON \(table)" } + func generateModifyIndexSQL(table: String, oldIndexName: String, newIndex: PluginIndexDefinition) -> String? { + "ALTER TABLE \(table) REPLACE INDEX \(oldIndexName) ON (\(newIndex.columns.joined(separator: ", ")))" + } func generateAddCheckConstraintSQL(table: String, constraint: PluginCheckConstraintDefinition) -> String? { "ALTER TABLE \(table) ADD CONSTRAINT \(constraint.name) CHECK (\(constraint.expression))" } @@ -150,7 +153,7 @@ struct SchemaOperationRefusalTests { driver.refuse = { operation in switch operation { case .modifyIndex(let old, let new): return "modify \(old.name) to \(new.name)" - case .dropIndex(let index): return "drop \(index.name)" + case .dropIndex(let index) where index.name == "ix_old": return "drop \(index.name)" default: return nil } } @@ -160,6 +163,95 @@ struct SchemaOperationRefusalTests { #expect(refusal(of: .deleteIndex(index("ix_old", type: .btree)), driver: driver) == "drop ix_old") } + /// A replacement is a drop and an add, so an index the driver will not drop is refused for that, + /// ahead of the advice about replacing it. DynamoDB answered a local index's replacement with + /// "delete it and save", and the delete was then refused for a different reason. + @Test("Replacing an index asks first whether the old one can be dropped") + func replacementAsksTheDropFirst() { + let driver = RefusingDDLDriver() + driver.refuse = { operation in + switch operation { + case .dropIndex(let index): return "drop \(index.name)" + case .modifyIndex: return "replace" + default: return nil + } + } + let modify = SchemaChange.modifyIndex(old: index("ix", type: .btree), new: index("ix", type: .hash)) + + #expect(refusal(of: modify, driver: driver) == "drop ix") + } + + @Test("An index deleted and added back under its name asks the driver about one replacement") + func sameNameDropAndAddAskForAReplacement() { + let driver = RefusingDDLDriver() + driver.refuse = { operation in + guard case .modifyIndex(let old, _) = operation else { return nil } + return "replace \(old.name)" + } + var replacement = index("ix", type: .btree) + replacement.columns = ["total"] + + #expect(refusalOfBatch([.deleteIndex(index("ix", type: .btree)), .addIndex(replacement)], driver: driver) == "replace ix") + } + + @Test("What the driver is asked about and the statements it writes describe the same replacement") + func refusalAndStatementsAgreeOnTheReplacement() throws { + let driver = RefusingDDLDriver() + var asked: [String] = [] + driver.refuse = { operation in + switch operation { + case .modifyIndex(let old, let new): asked.append("modify \(old.name) \(old.columns) to \(new.columns)") + case .addIndex(let index): asked.append("add \(index.name)") + case .dropIndex(let index): asked.append("drop \(index.name)") + default: break + } + return nil + } + var replacement = index("ix", type: .btree) + replacement.columns = ["total"] + + let statements = try SchemaStatementGenerator(tableName: "orders", pluginDriver: driver) + .generate(changes: [.addIndex(replacement), .deleteIndex(index("ix", type: .btree))]) + + #expect(asked == ["drop ix", "modify ix [\"qty\"] to [\"total\"]", "add ix"]) + #expect(statements.map(\.sql) == ["ALTER TABLE orders REPLACE INDEX ix ON (total);"]) + } + + @Test("Indexes deleted and added under different names are not asked about as a replacement") + func differentNamesAreNotAReplacement() { + let driver = RefusingDDLDriver() + var askedAboutReplacement = false + driver.refuse = { operation in + if case .modifyIndex = operation { askedAboutReplacement = true } + return nil + } + + _ = refusalOfBatch([.deleteIndex(index("ix_a", type: .btree)), .addIndex(index("ix_b", type: .btree))], driver: driver) + #expect(!askedAboutReplacement) + } + + /// Measured on MariaDB 13.0.2: with a column change in the same save the replacement splits, + /// `DROP INDEX PRIMARY` commits, `ADD UNIQUE INDEX PRIMARY` fails with ERROR 1280, and the table + /// is left with no key. The one-statement form fails the same way and keeps the key. + @Test("A copy of MySQL's PRIMARY row put in place of the original is refused before anything runs") + func mysqlPrimaryReplacementIsRefused() throws { + let driver = RefusingDDLDriver() + driver.refuse = { operation in + guard case .addIndex(let index) = operation else { return nil } + return mysqlReservedIndexNameRefusal(for: index) + } + var primary = index("PRIMARY", type: .btree) + primary.isPrimary = true + var copy = primary.withNewIdentity() + copy.isPrimary = false + let reason = try #require(mysqlReservedIndexNameRefusal(for: copy.toPlugin())) + + #expect(refusalOfBatch([.addIndex(copy), .deleteIndex(primary)], driver: driver) == reason) + #expect(refusalOfBatch( + [.addIndex(copy), .deleteIndex(primary), .addColumn(column("qty", generated: false))], driver: driver + ) == reason) + } + @Test("A refusal is reported ahead of a change in the same save that the driver cannot generate") func refusalWinsOverUngeneratableChange() { let unsupportedDrop = SchemaChange.deleteIndex(index("ix_old", type: .btree)) diff --git a/TableProTests/Core/SchemaTracking/SchemaStatementGeneratorPluginTests.swift b/TableProTests/Core/SchemaTracking/SchemaStatementGeneratorPluginTests.swift index 12712f672e..907f58d7e6 100644 --- a/TableProTests/Core/SchemaTracking/SchemaStatementGeneratorPluginTests.swift +++ b/TableProTests/Core/SchemaTracking/SchemaStatementGeneratorPluginTests.swift @@ -411,6 +411,174 @@ struct SchemaStatementGeneratorPluginTests { ]) } + // MARK: - An index deleted and added back under its name + + private func replacingDriver(canReplaceInOneStatement: Bool) -> MockPluginDriver { + let mock = MockPluginDriver() + mock.addColumnHandler = { table, col in "ALTER TABLE \(table) ADD COLUMN \(col.name)" } + mock.dropIndexHandler = { _, name in "DROP INDEX \(name)" } + mock.addIndexHandler = { table, idx in "CREATE INDEX \(idx.name) ON \(table) (\(idx.columns.joined(separator: ", ")))" } + if canReplaceInOneStatement { + mock.modifyIndexHandler = { table, oldName, idx in + "ALTER TABLE \(table) DROP INDEX \(oldName), ADD INDEX \(idx.name) (\(idx.columns.joined(separator: ", ")))" + } + } + return mock + } + + /// Measured on MariaDB 13.0.2: `DROP INDEX idx_a, ADD UNIQUE INDEX idx_a (b)` over duplicate + /// values fails with ERROR 1062 and keeps `idx_a`, where the two statements on their own drop it + /// and then fail. + @Test("An index deleted and added back under its name is replaced in one statement where the driver has one") + func sameNameDropAndAddIsOneReplacement() throws { + let stmts = try SchemaStatementGenerator(tableName: "users", pluginDriver: replacingDriver(canReplaceInOneStatement: true)) + .generate(changes: [ + .addIndex(makeIndex(name: "idx_a", columns: ["title"])), + .deleteIndex(makeIndex(name: "idx_a", columns: ["body"])) + ]) + + #expect(stmts.map(\.sql) == ["ALTER TABLE users DROP INDEX idx_a, ADD INDEX idx_a (title);"]) + } + + @Test("An index deleted and added back under its name is dropped before it is added where the driver cannot replace it") + func sameNameDropAndAddDropsFirst() throws { + let stmts = try SchemaStatementGenerator(tableName: "users", pluginDriver: replacingDriver(canReplaceInOneStatement: false)) + .generate(changes: [ + .addIndex(makeIndex(name: "idx_a", columns: ["title"])), + .deleteIndex(makeIndex(name: "idx_a", columns: ["body"])) + ]) + + #expect(stmts.map(\.sql) == ["DROP INDEX idx_a;", "CREATE INDEX idx_a ON users (title);"]) + } + + @Test("An index deleted and added back under its name is split around a column the same save adds") + func sameNameDropAndAddSplitsAroundColumnWork() throws { + let stmts = try SchemaStatementGenerator(tableName: "users", pluginDriver: replacingDriver(canReplaceInOneStatement: true)) + .generate(changes: [ + .addIndex(makeIndex(name: "idx_a", columns: ["title"])), + .addColumn(makeColumn(name: "title")), + .deleteIndex(makeIndex(name: "idx_a", columns: ["body"])) + ]) + + #expect(stmts.map(\.sql) == [ + "DROP INDEX idx_a;", + "ALTER TABLE users ADD COLUMN title;", + "CREATE INDEX idx_a ON users (title);" + ]) + } + + @Test("A deleted index's name is free before another index is renamed onto it") + func deletedNameIsDroppedBeforeARenameTakesIt() throws { + let stmts = try SchemaStatementGenerator(tableName: "users", pluginDriver: replacingDriver(canReplaceInOneStatement: true)) + .generate(changes: [ + .modifyIndex(old: makeIndex(name: "idx_b", columns: ["title"]), new: makeIndex(name: "idx_a", columns: ["title"])), + .deleteIndex(makeIndex(name: "idx_a", columns: ["body"])) + ]) + + #expect(stmts.map(\.sql) == [ + "DROP INDEX idx_a;", + "ALTER TABLE users DROP INDEX idx_b, ADD INDEX idx_a (title);" + ]) + } + + @Test("Indexes deleted and added under different names stay a drop and an add") + func differentNamesAreNotPaired() throws { + let stmts = try SchemaStatementGenerator(tableName: "users", pluginDriver: replacingDriver(canReplaceInOneStatement: true)) + .generate(changes: [ + .addIndex(makeIndex(name: "idx_b", columns: ["title"])), + .deleteIndex(makeIndex(name: "idx_a", columns: ["body"])) + ]) + + #expect(stmts.map(\.sql) == ["DROP INDEX idx_a;", "CREATE INDEX idx_b ON users (title);"]) + } + + @Test("Each add under a deleted index's name takes one delete, and a second pass pairs nothing more") + func pairingTakesOneDeletePerAdd() { + let dropped = makeIndex(name: "idx_a", columns: ["body"]) + let replacement = makeIndex(name: "idx_a", columns: ["title"]) + let extra = makeIndex(name: "idx_a", columns: ["email"]) + let other = makeIndex(name: "idx_b", columns: ["body"]) + let staged: [SchemaChange] = [.deleteIndex(dropped), .addIndex(replacement), .addIndex(extra), .deleteIndex(other)] + + let paired = SchemaStatementGenerator.replacingIndexesInPlace(staged) + #expect(paired == [.modifyIndex(old: dropped, new: replacement), .addIndex(extra), .deleteIndex(other)]) + #expect(SchemaStatementGenerator.replacingIndexesInPlace(paired) == paired) + } + + // MARK: - Renames that pass names along + + private func renamed(_ name: String, to newName: String, on column: String) -> SchemaChange { + .modifyIndex(old: makeIndex(name: name, columns: [column]), new: makeIndex(name: newName, columns: [column])) + } + + /// Measured on MariaDB 13.0.2 with indexes `a`, `b` and `c`: in staging order, replacing `c` with + /// `b` fails with ERROR 1061 while `b` still exists, and the save stops with `a` already dropped. + @Test("A rename onto a name another rename frees runs after that rename") + func renameChainRunsInNameOrder() throws { + let stmts = try SchemaStatementGenerator(tableName: "users", pluginDriver: replacingDriver(canReplaceInOneStatement: true)) + .generate(changes: [ + renamed("c", to: "b", on: "z"), + .deleteIndex(makeIndex(name: "a", columns: ["x"])), + renamed("b", to: "a", on: "y") + ]) + + #expect(stmts.map(\.sql) == [ + "DROP INDEX a;", + "ALTER TABLE users DROP INDEX b, ADD INDEX a (y);", + "ALTER TABLE users DROP INDEX c, ADD INDEX b (z);" + ]) + } + + @Test("An index added under a name a rename frees is added after that rename") + func addWaitsForTheRenameThatFreesItsName() throws { + let stmts = try SchemaStatementGenerator(tableName: "users", pluginDriver: replacingDriver(canReplaceInOneStatement: true)) + .generate(changes: [ + .addIndex(makeIndex(name: "a", columns: ["x"])), + renamed("a", to: "c", on: "y") + ]) + + #expect(stmts.map(\.sql) == [ + "ALTER TABLE users DROP INDEX a, ADD INDEX c (y);", + "CREATE INDEX a ON users (x);" + ]) + } + + /// Measured on MariaDB 13.0.2: `DROP INDEX a, ADD INDEX b (x)` fails with ERROR 1061 while `b` + /// exists, whichever of the two goes first. Dropping both and then adding both works. + @Test("Two indexes trading names are dropped and added back") + func swappedNamesAreDroppedAndAddedBack() throws { + let stmts = try SchemaStatementGenerator(tableName: "users", pluginDriver: replacingDriver(canReplaceInOneStatement: true)) + .generate(changes: [renamed("a", to: "b", on: "x"), renamed("b", to: "a", on: "y")]) + + #expect(stmts.map(\.sql) == [ + "DROP INDEX a;", + "DROP INDEX b;", + "CREATE INDEX b ON users (x);", + "CREATE INDEX a ON users (y);" + ]) + } + + @Test("A three-way rotation of index names is dropped and added back, and a rename outside it keeps its place") + func rotationIsSplitAndTheRestStaysWhole() throws { + let stmts = try SchemaStatementGenerator(tableName: "users", pluginDriver: replacingDriver(canReplaceInOneStatement: true)) + .generate(changes: [ + renamed("a", to: "b", on: "x"), + renamed("d", to: "e", on: "w"), + renamed("b", to: "c", on: "y"), + renamed("c", to: "a", on: "z") + ]) + + #expect(stmts.map(\.sql) == [ + "DROP INDEX a;", + "DROP INDEX b;", + "DROP INDEX c;", + "ALTER TABLE users DROP INDEX d, ADD INDEX e (w);", + "CREATE INDEX b ON users (x);", + "CREATE INDEX c ON users (y);", + "CREATE INDEX a ON users (z);" + ]) + } + @Test("Modify foreign key generates drop and create via plugin") func modifyForeignKeyViaPlugin() throws { let mock = MockPluginDriver() @@ -539,7 +707,7 @@ struct SchemaStatementGeneratorPluginTests { @Test("Modify column with type change is destructive") func modifyColumnTypeChangeDestructive() throws { let mock = MockPluginDriver() - mock.modifyColumnHandler = { _, oldCol, newCol in + mock.modifyColumnHandler = { _, _, newCol in "ALTER TABLE users MODIFY COLUMN \(newCol.name) \(newCol.dataType)" } diff --git a/TableProTests/Core/SchemaTracking/StructureChangeManagerClusteredIndexTests.swift b/TableProTests/Core/SchemaTracking/StructureChangeManagerClusteredIndexTests.swift index 61f9eeb345..64eeb1108e 100644 --- a/TableProTests/Core/SchemaTracking/StructureChangeManagerClusteredIndexTests.swift +++ b/TableProTests/Core/SchemaTracking/StructureChangeManagerClusteredIndexTests.swift @@ -92,6 +92,91 @@ struct StructureChangeManagerClusteredIndexTests { #expect(manager.workingIndexes.last?.type == .clustered) } + private static let clusteredIndex = IndexInfo( + name: "cx_orders_placed", columns: ["placed_at"], isUnique: false, isPrimary: false, type: "CLUSTERED" + ) + + /// Duplicate, edit the copy, then delete the original is how the Structure tab replaces an index. + /// Left `NONCLUSTERED`, the copy is created after the original is dropped, and the table ends the + /// save with no clustered index. + @Test("A duplicated clustered index takes the clustered place once the original is deleted") + func duplicateTakesThePlaceOfTheDeletedOriginal() throws { + let manager = Self.manager(indexes: [Self.clusteredIndex]) + let original = try #require(manager.workingIndexes.first) + manager.addIndex(original.withNewIdentity()) + var copy = try #require(manager.workingIndexes.last) + #expect(copy.type == .nonclustered) + copy.columns = ["placed_at", "id"] + manager.updateIndex(id: copy.id, with: copy) + + manager.deleteIndex(id: original.id) + + let replacement = try #require(manager.workingIndexes.last) + #expect(replacement.type == .clustered) + #expect(manager.canCommit) + #expect(manager.getChangesArray() == [.addIndex(replacement), .deleteIndex(original)]) + #expect( + MSSQLTableDefinitionSQL.indexDefinition(replacement.toPlugin(), qualifiedTable: "[dbo].[orders]") + == "CREATE CLUSTERED INDEX [cx_orders_placed] ON [dbo].[orders] ([placed_at], [id])" + ) + } + + @Test("Bringing the original back gives it the clustered place again") + func undoingTheDeletionGivesThePlaceBack() throws { + let manager = Self.manager(indexes: [Self.clusteredIndex]) + let original = try #require(manager.workingIndexes.first) + manager.addIndex(original.withNewIdentity()) + manager.deleteIndex(id: original.id) + #expect(manager.workingIndexes.last?.type == .clustered) + + manager.undo() + + let copy = try #require(manager.workingIndexes.last) + #expect(copy.type == .nonclustered) + #expect(manager.getChangesArray() == [.addIndex(copy)]) + } + + @Test("When the first of two clustered copies is removed, the second takes the place") + func removingTheFirstCopyHandsThePlaceOn() throws { + let manager = Self.manager(indexes: []) + let first = Self.copied("ix_first", type: .clustered) + manager.addIndex(first) + manager.addIndex(Self.copied("ix_second", type: .clustered)) + + manager.deleteIndex(id: first.id) + + #expect(manager.workingIndexes.map(\.name) == ["ix_second"]) + #expect(manager.workingIndexes.map(\.type) == [.clustered]) + } + + @Test("A copy whose type was changed keeps the type it was given") + func aTypeChosenForTheCopyIsKept() throws { + let manager = Self.manager(indexes: [Self.clusteredIndex]) + let original = try #require(manager.workingIndexes.first) + manager.addIndex(original.withNewIdentity()) + var copy = try #require(manager.workingIndexes.last) + copy.type = .hash + manager.updateIndex(id: copy.id, with: copy) + + manager.deleteIndex(id: original.id) + + #expect(manager.workingIndexes.last?.type == .hash) + } + + @Test("A clustered copy set to NONCLUSTERED stays that way while the clustered place is free") + func nonclusteredChosenForTheCopyIsKept() throws { + let manager = Self.manager(indexes: []) + manager.addIndex(Self.copied("ix_copy", type: .clustered)) + var copy = try #require(manager.workingIndexes.last) + #expect(copy.type == .clustered) + copy.type = .nonclustered + manager.updateIndex(id: copy.id, with: copy) + + manager.addIndex(Self.copied("ix_other", type: .hash)) + + #expect(manager.workingIndexes.first?.type == .nonclustered) + } + @Test("Every other type is added as it was copied") func otherTypesAreUntouched() throws { let manager = Self.manager(indexes: [Self.clusteredPrimaryKey]) diff --git a/TableProTests/Core/SchemaTracking/StructureChangeNameReuseTests.swift b/TableProTests/Core/SchemaTracking/StructureChangeNameReuseTests.swift new file mode 100644 index 0000000000..d4e02f007c --- /dev/null +++ b/TableProTests/Core/SchemaTracking/StructureChangeNameReuseTests.swift @@ -0,0 +1,345 @@ +// +// StructureChangeNameReuseTests.swift +// TableProTests +// + +import Foundation +@testable import TablePro +import Testing + +/// A name belongs to the rows the table keeps after the save. +/// +/// The save drops every index and check constraint before it adds or renames one, so a name a row +/// is being deleted under is free by the time anything takes it. The duplicate check still counted +/// the deleted row, and "delete idx_a, add a new idx_a" was refused as a duplicate of the row on +/// its way out. +@MainActor +struct StructureChangeNameReuseTests { + private func manager( + indexes: [IndexInfo] = [], + checkConstraints: [CheckConstraintInfo] = [] + ) -> StructureChangeManager { + let manager = StructureChangeManager() + manager.loadSchema( + tableName: "notes", + columns: ["id", "body", "title", "a"].map { name in + ColumnInfo( + name: name, dataType: "INTEGER", isNullable: name != "id", isPrimaryKey: name == "id", + defaultValue: nil, extra: nil, charset: nil, collation: nil, comment: nil + ) + }, + indexes: indexes, + foreignKeys: [], + checkConstraints: checkConstraints, + primaryKey: ["id"] + ) + return manager + } + + private func loadedIndex(_ name: String, on column: String) -> IndexInfo { + IndexInfo(name: name, columns: [column], isUnique: false, isPrimary: false, type: "BTREE") + } + + private func newIndex(_ name: String, on column: String) -> EditableIndexDefinition { + EditableIndexDefinition( + id: UUID(), name: name, columns: [column], type: .btree, isUnique: false, isPrimary: false, comment: nil + ) + } + + private func newConstraint(_ name: String, _ expression: String) -> EditableCheckConstraintDefinition { + EditableCheckConstraintDefinition(id: UUID(), name: name, expression: expression, columns: [], isValidated: true) + } + + private func index(named name: String, in manager: StructureChangeManager) throws -> EditableIndexDefinition { + try #require(manager.workingIndexes.first { $0.name == name }) + } + + private func constraint( + at position: Int, + in manager: StructureChangeManager + ) throws -> EditableCheckConstraintDefinition { + let constraints = manager.workingCheckConstraints + try #require(constraints.indices.contains(position)) + return constraints[position] + } + + private func errors(in manager: StructureChangeManager) -> Set { + Set(manager.validationErrors.values) + } + + // MARK: - Indexes + + /// Measured on SQLite 3.54, PostgreSQL 17.11 and MariaDB 13.0.2: `CREATE INDEX idx_a` fails while + /// `idx_a` exists, and succeeds straight after `DROP INDEX idx_a` in the same save. + @Test("Deleting an index frees its name for a new one in the same save") + func deletingAnIndexFreesItsNameForANewOne() throws { + let manager = manager(indexes: [loadedIndex("idx_a", on: "body")]) + manager.deleteIndex(id: try index(named: "idx_a", in: manager).id) + manager.addIndex(newIndex("idx_a", on: "title")) + + #expect(manager.canCommit) + #expect(manager.validationErrors.isEmpty) + } + + @Test("An index renamed onto a deleted index's name saves") + func renamingAnIndexOntoADeletedOnesName() throws { + let manager = manager(indexes: [loadedIndex("idx_a", on: "body"), loadedIndex("idx_b", on: "title")]) + manager.deleteIndex(id: try index(named: "idx_a", in: manager).id) + var renamed = try index(named: "idx_b", in: manager) + renamed.name = "idx_a" + manager.updateIndex(id: renamed.id, with: renamed) + + #expect(manager.canCommit) + #expect(manager.validationErrors.isEmpty) + } + + /// The row menu's **Duplicate** stages a copy under the same name, which is the everyday way to + /// build a replacement before the original goes. + @Test("Duplicating an index and then deleting the original saves") + func duplicatingAnIndexThenDeletingTheOriginalSaves() throws { + let manager = manager(indexes: [loadedIndex("idx_a", on: "body")]) + let original = try index(named: "idx_a", in: manager) + manager.addIndex(original.withNewIdentity()) + #expect(!manager.canCommit) + #expect(errors(in: manager) == ["Duplicate index name: idx_a"]) + + manager.deleteIndex(id: original.id) + #expect(manager.canCommit) + #expect(manager.validationErrors.isEmpty) + } + + @Test("Bringing the deleted index back makes the replacement a duplicate again") + func undoingTheDeletionBringsTheDuplicateBack() throws { + let manager = manager(indexes: [loadedIndex("idx_a", on: "body")]) + let original = try index(named: "idx_a", in: manager) + manager.addIndex(newIndex("idx_a", on: "title")) + manager.deleteIndex(id: original.id) + #expect(manager.canCommit) + + manager.undoDelete(for: .indexes, at: 0) + #expect(!manager.canCommit) + #expect(manager.validationErrors.count == 2) + #expect(errors(in: manager) == ["Duplicate index name: idx_a"]) + } + + @Test("Undoing the deletion makes the replacement a duplicate again") + func undoRestoresTheDuplicate() throws { + let manager = manager(indexes: [loadedIndex("idx_a", on: "body")]) + let original = try index(named: "idx_a", in: manager) + manager.addIndex(newIndex("idx_a", on: "title")) + manager.deleteIndex(id: original.id) + + manager.undo() + #expect(!manager.canCommit) + #expect(errors(in: manager) == ["Duplicate index name: idx_a"]) + } + + @Test("An index added under the name of one the table keeps is a duplicate") + func addingAnIndexUnderAKeptIndexesNameIsADuplicate() { + let manager = manager(indexes: [loadedIndex("idx_a", on: "body")]) + manager.addIndex(newIndex("idx_a", on: "title")) + + #expect(!manager.canCommit) + #expect(manager.validationErrors.count == 2) + #expect(errors(in: manager) == ["Duplicate index name: idx_a"]) + } + + @Test("Two added indexes under one name are duplicates") + func twoAddedIndexesWithOneNameAreDuplicates() { + let manager = manager() + manager.addIndex(newIndex("idx_a", on: "body")) + manager.addIndex(newIndex("idx_a", on: "title")) + + #expect(!manager.canCommit) + #expect(errors(in: manager) == ["Duplicate index name: idx_a"]) + } + + // MARK: - Check constraints + + /// Measured on SQLite 3.54 and PostgreSQL 17.11: `DROP CONSTRAINT c` then `ADD CONSTRAINT c` + /// saves, and `ADD CONSTRAINT c` alone fails while `c` exists. + @Test("Deleting a check constraint frees its name for a new one in the same save") + func deletingACheckConstraintFreesItsName() throws { + let manager = manager(checkConstraints: [CheckConstraintInfo(name: "c", expression: "a > 0")]) + manager.deleteCheckConstraint(id: try constraint(at: 0, in: manager).id) + manager.addCheckConstraint(newConstraint("c", "a < 10")) + + #expect(manager.canCommit) + #expect(manager.validationErrors.isEmpty) + } + + @Test("A check constraint renamed onto a deleted one's name saves") + func renamingAConstraintOntoADeletedOnesName() throws { + let manager = manager(checkConstraints: [ + CheckConstraintInfo(name: "c", expression: "a > 0"), + CheckConstraintInfo(name: "d", expression: "a < 10") + ]) + manager.deleteCheckConstraint(id: try constraint(at: 0, in: manager).id) + var renamed = try constraint(at: 1, in: manager) + renamed.name = "c" + manager.updateCheckConstraint(id: renamed.id, with: renamed) + + #expect(manager.canCommit) + #expect(manager.validationErrors.isEmpty) + } + + @Test("A check constraint added under the name of one the table keeps is a duplicate") + func addingAConstraintUnderAKeptNameIsADuplicate() { + let manager = manager(checkConstraints: [CheckConstraintInfo(name: "c", expression: "a > 0")]) + manager.addCheckConstraint(newConstraint("c", "a < 10")) + + #expect(!manager.canCommit) + #expect(errors(in: manager) == ["Duplicate constraint name: c"]) + } + + /// Measured on SQLite 3.54: `ADD CONSTRAINT "C"` beside `c` fails with "constraint C already + /// exists", and MariaDB 13.0.2 refuses it with ERROR 1826. + @Test("A check constraint added beside one whose name differs only in case is a duplicate") + func addingAConstraintUnderACaseVariantOfAKeptNameIsADuplicate() { + let manager = manager(checkConstraints: [CheckConstraintInfo(name: "c", expression: "a > 0")]) + manager.addCheckConstraint(newConstraint("C", "a < 10")) + + #expect(!manager.canCommit) + #expect(errors(in: manager) == ["Duplicate constraint name: c", "Duplicate constraint name: C"]) + } + + // MARK: - Two loaded check constraints under one name + + /// SQLite 3.54 accepts `CONSTRAINT c CHECK (a > 0), CONSTRAINT c CHECK (a < 10)` in one + /// `CREATE TABLE`, and the Structure tab lists both. + private func tableWithTwoConstraintsNamedC(secondName: String = "c") -> StructureChangeManager { + manager(checkConstraints: [ + CheckConstraintInfo(name: "c", expression: "a > 0"), + CheckConstraintInfo(name: secondName, expression: "a < 10") + ]) + } + + private let sharedNameRefusal = "More than one check constraint is named c. Change or delete all of them in the same save." + + @Test("Two loaded check constraints under one name do not block a save that leaves them alone") + func untouchedSameNamedConstraintsDoNotBlockAnUnrelatedSave() { + let manager = tableWithTwoConstraintsNamedC() + manager.addIndex(newIndex("idx_body", on: "body")) + + #expect(manager.canCommit) + #expect(manager.validationErrors.isEmpty) + } + + /// Measured on SQLite 3.54: `DROP CONSTRAINT c` removes the first `c` in the table's text, + /// whichever row was picked. + @Test("Deleting one of two check constraints under one name is refused") + func droppingOneOfTwoSameNamedConstraintsIsRefused() throws { + let manager = tableWithTwoConstraintsNamedC() + let second = try constraint(at: 1, in: manager) + manager.deleteCheckConstraint(id: second.id) + + #expect(!manager.canCommit) + #expect(manager.validationErrors[.checkConstraint(second.id)] == sharedNameRefusal) + #expect(manager.validationErrors.count == 1) + } + + @Test("Rewriting one of two check constraints under one name is refused") + func rewritingOneOfTwoSameNamedConstraintsIsRefused() throws { + let manager = tableWithTwoConstraintsNamedC() + var rewritten = try constraint(at: 0, in: manager) + rewritten.expression = "a > 1" + manager.updateCheckConstraint(id: rewritten.id, with: rewritten) + + #expect(!manager.canCommit) + #expect(manager.validationErrors[.checkConstraint(rewritten.id)] == sharedNameRefusal) + } + + @Test("Renaming one of two check constraints under one name is refused") + func renamingOneOfTwoSameNamedConstraintsIsRefused() throws { + let manager = tableWithTwoConstraintsNamedC() + var renamed = try constraint(at: 0, in: manager) + renamed.name = "d" + manager.updateCheckConstraint(id: renamed.id, with: renamed) + + #expect(!manager.canCommit) + #expect(manager.validationErrors[.checkConstraint(renamed.id)] == sharedNameRefusal) + #expect(manager.validationErrors.count == 1) + } + + /// Measured on SQLite 3.54: `DROP CONSTRAINT "C"` on a table holding `c` and `C` drops `c`. + @Test("Check constraint names that differ only in case are one name") + func caseVariantConstraintNamesAreShared() throws { + let manager = tableWithTwoConstraintsNamedC(secondName: "C") + let second = try constraint(at: 1, in: manager) + manager.deleteCheckConstraint(id: second.id) + + #expect(!manager.canCommit) + #expect(manager.validationErrors[.checkConstraint(second.id)]?.hasPrefix("More than one check constraint is named C") == true) + } + + @Test("Deleting every check constraint under a shared name saves") + func droppingBothSameNamedConstraintsSaves() throws { + let manager = tableWithTwoConstraintsNamedC() + let ids = manager.workingCheckConstraints.map(\.id) + for id in ids { + manager.deleteCheckConstraint(id: id) + } + + #expect(manager.canCommit) + #expect(manager.validationErrors.isEmpty) + } + + /// Measured on SQLite 3.54: after `DROP CONSTRAINT c` twice and `ADD CONSTRAINT c`, the add of + /// `C` fails with "constraint C already exists". + @Test("Replacing both check constraints under a shared name with c and C is a duplicate") + func replacingBothWithCaseVariantsIsADuplicate() { + let manager = tableWithTwoConstraintsNamedC() + for id in manager.workingCheckConstraints.map(\.id) { + manager.deleteCheckConstraint(id: id) + } + manager.addCheckConstraint(newConstraint("c", "a > 1")) + manager.addCheckConstraint(newConstraint("C", "a < 9")) + + #expect(!manager.canCommit) + #expect(errors(in: manager) == ["Duplicate constraint name: c", "Duplicate constraint name: C"]) + } + + /// SQLite has no `RENAME CONSTRAINT`, so each rename is a drop by the shared name and an add of + /// the row's own definition. Whichever `c` each drop takes, both go and both come back right. + @Test("Renaming every check constraint under a shared name saves") + func renamingBothSameNamedConstraintsSaves() throws { + let manager = tableWithTwoConstraintsNamedC() + for (position, name) in [(0, "d"), (1, "e")] { + var renamed = try constraint(at: position, in: manager) + renamed.name = name + manager.updateCheckConstraint(id: renamed.id, with: renamed) + } + + #expect(manager.canCommit) + #expect(manager.validationErrors.isEmpty) + } + + /// Measured on SQLite 3.54: `DROP CONSTRAINT c; DROP CONSTRAINT c; ADD CONSTRAINT c CHECK (a < 20)` + /// leaves exactly the rewritten constraint. + @Test("Deleting one check constraint under a shared name and rewriting the other saves") + func deletingOneAndRewritingTheOtherSaves() throws { + let manager = tableWithTwoConstraintsNamedC() + manager.deleteCheckConstraint(id: try constraint(at: 0, in: manager).id) + var rewritten = try constraint(at: 1, in: manager) + rewritten.expression = "a < 20" + manager.updateCheckConstraint(id: rewritten.id, with: rewritten) + + #expect(manager.canCommit) + #expect(manager.validationErrors.isEmpty) + } + + /// Both would be added back as `c`, and the second `ADD CONSTRAINT c` fails on SQLite 3.54 with + /// "constraint c already exists". + @Test("Rewriting both check constraints under their shared name is a duplicate") + func rewritingBothUnderTheSharedNameIsADuplicate() throws { + let manager = tableWithTwoConstraintsNamedC() + for (position, expression) in [(0, "a > 1"), (1, "a < 9")] { + var rewritten = try constraint(at: position, in: manager) + rewritten.expression = expression + manager.updateCheckConstraint(id: rewritten.id, with: rewritten) + } + + #expect(!manager.canCommit) + #expect(errors(in: manager) == ["Duplicate constraint name: c"]) + } +} diff --git a/TableProTests/Plugins/DynamoDB/DynamoDBTableManagementTests.swift b/TableProTests/Plugins/DynamoDB/DynamoDBTableManagementTests.swift index 6ccc626aff..8c4b787223 100644 --- a/TableProTests/Plugins/DynamoDB/DynamoDBTableManagementTests.swift +++ b/TableProTests/Plugins/DynamoDB/DynamoDBTableManagementTests.swift @@ -4,6 +4,7 @@ // import Foundation +@testable import TablePro import TableProPluginKit import Testing @@ -247,6 +248,46 @@ struct DynamoDBTableManagementTests { #expect(Self.driver.schemaOperationRefusal(.dropIndex(primary)) != nil) } + private static func structureIndex(_ name: String, columns: [String], type: String) -> EditableIndexDefinition { + EditableIndexDefinition( + id: UUID(), name: name, columns: columns, type: .init(rawValue: type), isUnique: false, isPrimary: false, + comment: nil + ) + } + + private static func refusalOfSave(_ changes: [SchemaChange]) -> String? { + do { + _ = try SchemaStatementGenerator(tableName: "Orders", pluginDriver: driver).generate(changes: changes) + return nil + } catch let error as SchemaOperationRefusedError { + return error.reason + } catch { + return "unexpected: \(error.localizedDescription)" + } + } + + /// DynamoDB takes one index change per `UpdateTable` and none while the table is `UPDATING`, and + /// the driver sends one request per change. So the delete would run and the add then fail, + /// leaving no index under the name. The save is refused before either request is sent. + @Test("Deleting a global index and adding one under its name in one save is refused before anything runs") + func sameNameGlobalIndexReplacementIsRefused() { + let dropped = Self.structureIndex("byStatus", columns: ["status"], type: "GLOBAL All attributes") + let added = Self.structureIndex("byStatus", columns: ["status", "total"], type: "GLOBAL All attributes") + + #expect(Self.refusalOfSave([.deleteIndex(dropped), .addIndex(added)]) + == Self.driver.schemaOperationRefusal(.modifyIndex(old: dropped.toPlugin(), new: added.toPlugin()))) + #expect(Self.refusalOfSave([.deleteIndex(dropped), .addIndex(added)])?.hasPrefix("A DynamoDB index can't be changed") == true) + } + + @Test("A local index put back under its own name is refused for being part of the table") + func sameNameLocalIndexReplacementIsRefusedAsADrop() { + let dropped = Self.structureIndex("byTotal", columns: ["pk", "total"], type: "LOCAL Keys only") + let added = Self.structureIndex("byTotal", columns: ["pk", "created"], type: "LOCAL Keys only") + + #expect(Self.refusalOfSave([.addIndex(added), .deleteIndex(dropped)]) + == "A local secondary index is part of its table and is removed only with the table.") + } + @Test("A global index from the Structure tab takes its keys from the columns and its projection from the included ones") func addIndex() throws { let index = PluginIndexDefinition( diff --git a/TableProTests/Plugins/MySQLIndexKeyWriterTests.swift b/TableProTests/Plugins/MySQLIndexKeyWriterTests.swift index a05211eb21..11355a99a9 100644 --- a/TableProTests/Plugins/MySQLIndexKeyWriterTests.swift +++ b/TableProTests/Plugins/MySQLIndexKeyWriterTests.swift @@ -108,4 +108,14 @@ struct MySQLIndexKeyWriterTests { ) #expect(mysqlModifyIndexSQL(table: "t", oldIndexName: "ix", newIndex: index, flavor: .databend) == nil) } + + /// Measured on MariaDB 13.0.2: `ADD INDEX PRIMARY (b)`, `ADD INDEX primary (b)` and + /// `ADD INDEX Primary (b)` each fail with ERROR 1280, "Incorrect index name". + @Test("An added index named PRIMARY in any case is refused, and a name that only starts with it is not") + func primaryIsNotAnIndexName() { + for name in ["PRIMARY", "primary", "Primary"] { + #expect(mysqlReservedIndexNameRefusal(for: PluginIndexDefinition(name: name, columns: ["id"])) != nil) + } + #expect(mysqlReservedIndexNameRefusal(for: PluginIndexDefinition(name: "primary_email", columns: ["email"])) == nil) + } } diff --git a/TableProUITests/StructureIndexNameReuseUITests.swift b/TableProUITests/StructureIndexNameReuseUITests.swift new file mode 100644 index 0000000000..34a00f1f53 --- /dev/null +++ b/TableProUITests/StructureIndexNameReuseUITests.swift @@ -0,0 +1,134 @@ +// +// StructureIndexNameReuseUITests.swift +// TableProUITests +// + +import XCTest + +/// Deleting an index and giving another index its name in the same save was refused as "Duplicate +/// index name", counting the row being deleted, although the save drops the old index before +/// anything takes its name. +/// +/// The two indexes cover different columns, so the file tells a save that ran from one that was +/// refused: afterwards the deleted index's name has to be on the other index's column. +final class StructureIndexNameReuseUITests: UITestCase { + private let indexedColumns = ["idx_body": "body", "idx_tag": "tag"] + + func testRenamingAnIndexOntoADeletedIndexsNameSaves() throws { + let databases = try seedSQLiteSession( + connectionNames: ["Indexes"], + databaseSQL: """ + CREATE TABLE notes (id INTEGER PRIMARY KEY, body TEXT, tag TEXT); + CREATE INDEX idx_body ON notes (body); + CREATE INDEX idx_tag ON notes (tag); + """ + ) + let database = try XCTUnwrap(databases.first) + let app = try launchApp() + let window = app.windows.firstMatch + let grid = try openIndexesGrid(app: app, window: window) + + /// Up from whichever row the click took puts the selection on the first row, so the delete + /// and the rename land on different rows without reading the order the catalog lists them in. + gridPoint(in: grid, of: window, dy: 60).click() + app.typeKey(.upArrow, modifierFlags: []) + XCTAssertTrue( + waitForPredicate(timeout: 10) { grid.tableRows.element(boundBy: 0).isSelected }, + "The first index row must be selected" + ) + app.typeKey(.delete, modifierFlags: []) + app.typeKey(.downArrow, modifierFlags: []) + XCTAssertTrue( + waitForPredicate(timeout: 10) { grid.tableRows.element(boundBy: 1).isSelected }, + "The deleted row stays listed until the save, so Down must reach the second index" + ) + + showInspector(in: app) + let nameField = window.textFields + .matching(NSPredicate(format: "value IN %@", Array(indexedColumns.keys))) + .firstMatch + XCTAssertTrue(nameField.waitToExist(timeout: 30), "The inspector must show the second index's name") + let renamed = try XCTUnwrap(nameField.value as? String) + let deleted = try XCTUnwrap(indexedColumns.keys.first { $0 != renamed }) + let renamedColumn = try XCTUnwrap(indexedColumns[renamed]) + + nameField.click() + app.typeKey("a", modifierFlags: .command) + app.typeText(deleted) + app.typeKey(.return, modifierFlags: []) + + app.typeKey("s", modifierFlags: .command) + + /// `DROP INDEX` is a destructive statement, so the save is reviewed before it runs. Before + /// the fix the save stopped earlier, on "Some Changes Are Incomplete". + let sheet = window.sheets.firstMatch + let execute = sheet.buttons["sql-review-execute"].firstMatch + XCTAssertTrue( + execute.waitToExist(timeout: 20), + "The save must reach the review of its statements, got: \(text(of: sheet))" + ) + execute.click() + + XCTAssertTrue( + waitForPredicate(timeout: 30) { grid.tableRows.count == 1 }, + "The save must run and the grid reload with the one index the table kept" + ) + XCTAssertEqual( + sqliteStrings("SELECT name FROM sqlite_master WHERE type = 'index' AND tbl_name = 'notes'", in: database), + [deleted], + "Only the renamed index may be left, under the deleted index's name" + ) + let definition = sqliteStrings("SELECT sql FROM sqlite_master WHERE name = '\(deleted)'", in: database) + .joined(separator: "\n") + XCTAssertTrue( + definition.contains(renamedColumn), + "\(deleted) must now be the index on \(renamedColumn), got: \(definition)" + ) + } + + private func openIndexesGrid(app: XCUIApplication, window: XCUIElement) throws -> XCUIElement { + let table = objectBrowserRow("notes", in: window) + XCTAssertTrue(table.waitToExist(timeout: 60), "The restored connection must list notes") + clickAtCenter(table) + + showStructure(in: app, window: window) + let indexes = window.radioGroups["structure-tab-picker"].firstMatch + .radioButtons + .matching(NSPredicate(format: "label BEGINSWITH %@", "Indexes")) + .firstMatch + XCTAssertTrue(indexes.waitToExist(timeout: 20), "SQLite has indexes, so the tab must be offered") + indexes.click() + + let grid = window.tables.matching(identifier: "data-grid").firstMatch + XCTAssertTrue(grid.waitToExist(timeout: 30), "The structure editor must draw its index grid") + XCTAssertTrue( + waitForPredicate(timeout: 30) { + grid.frame.width > 0 && grid.frame.height > 0 && grid.tableRows.count == 2 + }, + "notes has two indexes, so the grid must list two rows" + ) + return grid + } + + /// `value`, not `label`: an alert's text reaches XCUITest as the static text's value. + private func text(of sheet: XCUIElement) -> String { + sheet.staticTexts.allElementsBoundByIndex + .map { ($0.value as? String) ?? $0.label } + .joined(separator: " ") + } + + /// The inspector remembers whether it was open, so the View menu item's title is the only + /// handle on its state. + private func showInspector(in app: XCUIApplication) { + let menuBar = app.menuBars.firstMatch + XCTAssertTrue(menuBar.waitToExist(timeout: 10)) + menuBar.menuBarItems["View"].click() + + let show = menuBar.menuItems["Show Inspector"] + if show.waitToExist(timeout: 5) { + show.click() + return + } + app.typeKey(.escape, modifierFlags: []) + } +} diff --git a/docs/databases/dynamodb.mdx b/docs/databases/dynamodb.mdx index 30a68628d8..7f4564222a 100644 --- a/docs/databases/dynamodb.mdx +++ b/docs/databases/dynamodb.mdx @@ -137,7 +137,7 @@ Any other action name is refused. Autocomplete inserts a template for the common Create Table form with Primary Key, Capacity, Settings and secondary index sections -In [Table Structure](/features/table-structure), **Indexes** lists the key and every index. Add a global secondary index there with its partition key as the first column and an optional sort key as the second, or drop one; either is an `UpdateTable` request, and DynamoDB builds or removes the index in the background. The key's type comes from the table's items; when no item read so far holds that attribute, the index is refused, so create it from the editor with an `UpdateTable` request that declares `AttributeDefinitions`. An index cannot be edited in place: delete it, save, and add the replacement once the old one is gone. The primary key and a local secondary index go only with the table. **DDL** shows the `CreateTable` request that recreates the table, plus the Time to Live and point-in-time recovery settings it carries. +In [Table Structure](/features/table-structure), **Indexes** lists the key and every index. Add a global secondary index there with its partition key as the first column and an optional sort key as the second, or drop one; either is an `UpdateTable` request, and DynamoDB builds or removes the index in the background. The key's type comes from the table's items; when no item read so far holds that attribute, the index is refused, so create it from the editor with an `UpdateTable` request that declares `AttributeDefinitions`. An index cannot be edited in place: delete it, save, and add the replacement once the old one is gone. Save one index change at a time. DynamoDB takes one index create or delete per `UpdateTable` request and refuses another while the table is updating, so a second index change in the same save fails after the first has run. The primary key and a local secondary index go only with the table. **DDL** shows the `CreateTable` request that recreates the table, plus the Time to Live and point-in-time recovery settings it carries. Right-click a table and choose **Maintenance** for point-in-time recovery, deletion protection, the stream, the table class, switching to on-demand capacity and turning Time to Live off. [Table Operations](/features/table-operations#maintenance) covers the sheet. diff --git a/docs/features/table-structure.mdx b/docs/features/table-structure.mdx index d89ae349ca..847b21bc81 100644 --- a/docs/features/table-structure.mdx +++ b/docs/features/table-structure.mdx @@ -107,10 +107,12 @@ Everything that runs goes to query history rather than the change queue. | Field | Description | |-------|-------------| | **Columns** | Key columns and expressions in key order, such as `tenant_id, lower(email)`. MySQL prefix lengths are written as `email(20)`. A PostgreSQL index's `INCLUDE` columns are not listed, and editing the index keeps them. Pasted into a table on another engine, an index leaves its `INCLUDE` columns behind and each expression becomes a column name the row flags until you replace it | -| **Type** | The type the server reports, such as `HNSW` from pgvector or `CLUSTERED` on SQL Server. The menu lists that type beside BTREE, HASH, FULLTEXT and SPATIAL (MySQL), GIN, GIST, SPGIST (PostgreSQL 9.2 and later) and BRIN (PostgreSQL 9.5 and later). On SQL Server, a duplicated or pasted `CLUSTERED` index becomes `NONCLUSTERED` when the table already has a clustered index. Pasted into a table on another database type, a FULLTEXT or SPATIAL index is refused by PostgreSQL before anything runs, and any other type that engine has no index of is written as its default index | +| **Type** | The type the server reports, such as `HNSW` from pgvector or `CLUSTERED` on SQL Server. The menu lists that type beside BTREE, HASH, FULLTEXT and SPATIAL (MySQL), GIN, GIST, SPGIST (PostgreSQL 9.2 and later) and BRIN (PostgreSQL 9.5 and later). On SQL Server, a duplicated or pasted `CLUSTERED` index is `NONCLUSTERED` while the table keeps another clustered index, and turns `CLUSTERED` again when you delete that index in the same save. Pasted into a table on another database type, a FULLTEXT or SPATIAL index is refused by PostgreSQL before anything runs, and any other type that engine has no index of is written as its default index | | **Unique** | Whether the index enforces uniqueness | | **Condition** | `WHERE` predicate for partial indexes (PostgreSQL, SQLite, libSQL, Cloudflare D1) | +To replace an index, delete it and add the new one under the same name in one save, or choose **Duplicate** on its row, edit the copy and delete the original. The old index is dropped before the new one is created. Renames in one save may pass names along, such as `b` to `a` and `c` to `b`: each runs once the name it takes is free, and two indexes that swap names are dropped and created again. On MySQL and MariaDB, an index you change or replace in a save that changes no column runs as one `ALTER TABLE`, so a replacement the server rejects leaves the original index in place. Neither takes an index named `PRIMARY`, in any letter case, because that name belongs to the primary key: change the key with **Primary Key** on the Columns tab. DynamoDB takes one index change per save, see [DynamoDB](/databases/dynamodb#creating-and-changing-tables). + ### Expression keys Type an expression into **Columns** as `CREATE INDEX` writes it, such as `lower(email)` or `coalesce(first_name, last_name)`. A comma inside parentheses or a string stays part of its key. Text that names one of the table's columns is that column, in any case and with or without parentheses around it. @@ -121,7 +123,7 @@ Type an expression into **Columns** as `CREATE INDEX` writes it, such as `lower( | MySQL | Needs 8.0.13 or later. On an older server, or a MariaDB server, the save stops before anything runs | | MariaDB, TiDB, OceanBase, CockroachDB and every other engine | Read as a column name, which the row flags | -A sort order is not typed here: `lower(email) DESC` is flagged as a column that does not exist. A descending key, a collation or an operator class the index already has stays through a rename or a new condition, and goes when you edit **Columns** or **Type**. On MySQL and MariaDB, a changed index in a save with no column changes is replaced in one `ALTER TABLE`, so a replacement the server rejects leaves the original index in place. +A sort order is not typed here: `lower(email) DESC` is flagged as a column that does not exist. A descending key, a collation or an operator class the index already has stays through a rename or a new condition, and goes when you edit **Columns** or **Type**. ### Invalid indexes @@ -188,7 +190,9 @@ Check constraints are table-level, so a rule spanning two columns is one row her **Columns** is filled from the catalog on PostgreSQL and SQL Server. MySQL, MariaDB and SQLite publish no such catalog, so the cell stays empty there. -Renaming a constraint runs a single `RENAME CONSTRAINT` where the engine has one. Changing the expression drops and re-adds it, because no engine can alter a check in place. Both the re-add and a brand-new constraint scan every existing row and fail if any row violates the rule, so a failed save means the data disagrees with the constraint. +Renaming a constraint runs a single `RENAME CONSTRAINT` where the engine has one. Changing the expression drops and re-adds it, because no engine can alter a check in place. Both the re-add and a brand-new constraint scan every existing row and fail if any row violates the rule, so a failed save means the data disagrees with the constraint. Every drop runs first, and a rename waits for the rename that frees its name, so a constraint added or renamed in the same save can take the name of one you deleted or renamed. Two constraints that swap names are dropped and added back, which scans the table. + +A constraint named `C` beside one named `c` is refused before anything runs, on every engine, because SQLite and MariaDB treat the two as one name. Give the second constraint another name. The tab is hidden on engines with no check constraints. SQL Server lists and edits them, but has computed columns rather than generated ones, so it gets this tab and not the two column fields. @@ -196,6 +200,8 @@ It is hidden on MySQL before 8.0.16, MariaDB before 10.2.1, TiDB before 7.2 and SQLite is the one engine where listing and editing part company. The tab appears on every version, but **+** and **-** need SQLite 3.53.0 or later, the release that added `ADD CONSTRAINT` and `DROP CONSTRAINT` to `ALTER TABLE`. The driver links the system SQLite, so the version is the one macOS ships. On an older one the constraints still list, read-only. +SQLite also accepts two constraints under one name, treats `c` and `C` as the same name, and `DROP CONSTRAINT` removes the first one it finds. When two listed constraints share a name, a save that changes or deletes one of them and leaves the other is refused before anything runs: change or delete all of them in the same save. A save that leaves them both alone goes through. + ## Saving changes