From 10259d0272d61077f80e726a57132e733447f698 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Sat, 26 Sep 2026 12:01:48 +0700 Subject: [PATCH 1/2] fix(structure): hold only the columns a save changes to having a name and a type --- CHANGELOG.md | 2 + .../StructureChangeManager.swift | 72 +++-- TablePro/Models/Schema/ColumnDefinition.swift | 30 +- .../StructureChangeValidationTests.swift | 279 +++++++++++++++++- .../Models/Schema/ColumnDefinitionTests.swift | 86 ++++++ .../Schema/SQLiteColumnDeclarationTests.swift | 12 +- .../Schema/SQLiteTableRespecifierTests.swift | 15 +- .../CloseTabBeforeFirstClickUITests.swift | 81 +---- .../StructureTypelessColumnUITests.swift | 75 +++++ .../Support/SeededSQLiteSession.swift | 109 +++++++ docs/databases/sqlite.mdx | 2 + 11 files changed, 653 insertions(+), 110 deletions(-) create mode 100644 TableProUITests/StructureTypelessColumnUITests.swift create mode 100644 TableProUITests/Support/SeededSQLiteSession.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c57dc014e..eec28437ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -192,6 +192,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Structure grid and inspector taking edits the object or engine refuses, such as a materialized view's Type. - **Delete** and **Duplicate** in a structure row's menu doing nothing on an object that refuses them. - **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. - 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/TablePro/Core/SchemaTracking/StructureChangeManager.swift b/TablePro/Core/SchemaTracking/StructureChangeManager.swift index be4755b206..e108305759 100644 --- a/TablePro/Core/SchemaTracking/StructureChangeManager.swift +++ b/TablePro/Core/SchemaTracking/StructureChangeManager.swift @@ -401,30 +401,9 @@ final class StructureChangeManager: ObservableObject, ChangeManaging { private func validate() { validationErrors.removeAll() - for column in workingColumns { - if !column.isValid { - validationErrors[.column(column.id)] = String(localized: "Column must have a name and a data type") - } else if isStaged(.column(column.id)), introducesNullDefaultOnNotNull(column) { - validationErrors[.column(column.id)] = String( - format: String(localized: "%@ does not allow NULL, so its default cannot be NULL"), column.name - ) - } - } - - let columnNames = workingColumns.filter { column in - column.isValid && !isColumnPendingDeletion(column.id) - }.map { $0.name } - let duplicateColumns = Dictionary(grouping: columnNames, by: { $0 }) - .filter { $0.value.count > 1 } - .map { $0.key } - - for duplicate in duplicateColumns { - for column in workingColumns.filter({ $0.name == duplicate && !isColumnPendingDeletion($0.id) }) { - validationErrors[.column(column.id)] = String( - format: String(localized: "Duplicate column name: %@"), duplicate - ) - } - } + let keptColumns = columnsAfterSave + validateColumns(keptColumns) + let columnNames = keptColumns.map(\.name) for index in workingIndexes where isStaged(.index(index.id)) && !index.isValid { validationErrors[.index(index.id)] = String(localized: "Index must have a name and at least one column") @@ -502,10 +481,47 @@ final class StructureChangeManager: ObservableObject, ChangeManaging { } } - for columnName in workingPrimaryKey { - if !columnNames.contains(columnName) { - validationErrors[.primaryKey] = String( - format: String(localized: "Primary key references a column that does not exist: %@"), columnName + /// 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` + /// carries the key over itself; dropping a key column is the database's to allow or refuse. + for columnName in workingPrimaryKey where isStaged(.primaryKey) && !namesAColumn(columnName, in: columnNames) { + validationErrors[.primaryKey] = String( + format: String(localized: "Primary key references a column that does not exist: %@"), columnName + ) + } + } + + /// 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) } + } + + /// Only a column this save adds or changes is held to being complete. + /// + /// An untouched column is the database's own, and a typeless SQLite column or an empty MongoDB + /// field name is no reason to refuse an edit made somewhere else. A duplicate name blocks only + /// when the save put one of its columns there; an untouched pair stays the database's to judge. + /// Every name the table will hold is compared, a blank one it was read with included, while a + /// blank row still to be named is incomplete rather than a duplicate. + private func validateColumns(_ keptColumns: [EditableColumnDefinition]) { + let loadedColumns = Dictionary(currentColumns.map { ($0.id, $0) }, uniquingKeysWith: { first, _ in first }) + for column in keptColumns where isStaged(.column(column.id)) { + if column.isIncomplete(over: loadedColumns[column.id]) { + validationErrors[.column(column.id)] = String(localized: "Column must have a name and a data type") + } else if introducesNullDefaultOnNotNull(column) { + validationErrors[.column(column.id)] = String( + format: String(localized: "%@ does not allow NULL, so its default cannot be NULL"), column.name + ) + } + } + + let savablyNamed = keptColumns.filter { $0.hasSavableName(over: loadedColumns[$0.id]) } + let sameNamed = Dictionary(grouping: savablyNamed, by: \.name) + for (name, columns) in sameNamed where columns.count > 1 { + guard columns.contains(where: { isStaged(.column($0.id)) }) else { continue } + for column in columns { + validationErrors[.column(column.id)] = String( + format: String(localized: "Duplicate column name: %@"), name ) } } diff --git a/TablePro/Models/Schema/ColumnDefinition.swift b/TablePro/Models/Schema/ColumnDefinition.swift index 0131734bea..15930c756d 100644 --- a/TablePro/Models/Schema/ColumnDefinition.swift +++ b/TablePro/Models/Schema/ColumnDefinition.swift @@ -198,10 +198,34 @@ struct EditableColumnDefinition: Hashable, Codable, Identifiable { defaultValue = nil } + var hasName: Bool { !name.trimmingCharacters(in: .whitespaces).isEmpty } + + var hasDataType: Bool { !dataType.trimmingCharacters(in: .whitespaces).isEmpty } + /// Check if this definition is valid (not a placeholder) - var isValid: Bool { - !name.trimmingCharacters(in: .whitespaces).isEmpty && - !dataType.trimmingCharacters(in: .whitespaces).isEmpty + var isValid: Bool { hasName && hasDataType } + + /// Whether this column, as edited, lacks something a save has to write. + /// + /// A column the save adds is written from nothing, so it needs a name and a type. A loaded one + /// only has to keep what it had. SQLite reports a column declared without a type as type `''`, + /// and SQLite and MongoDB both hold a column or field whose name is empty, so an edit is never + /// asked for a name or a type the column did not have when it was read. + func isIncomplete(over loaded: EditableColumnDefinition?) -> Bool { + !hasSavableName(over: loaded) || !hasSavableDataType(over: loaded) + } + + /// Whether the name this column is saved under is one the table holds. + /// + /// A blank name is a real name when the column was read with one: SQLite keeps `""` and `" "` + /// as two columns, and renaming one onto the other fails with "duplicate column name". A blank + /// name on a column that is new, or that had a name when it was read, is still to be filled in. + func hasSavableName(over loaded: EditableColumnDefinition?) -> Bool { + hasName || loaded?.hasName == false + } + + func hasSavableDataType(over loaded: EditableColumnDefinition?) -> Bool { + hasDataType || loaded?.hasDataType == false } /// Create from existing ColumnInfo diff --git a/TableProTests/Core/SchemaTracking/StructureChangeValidationTests.swift b/TableProTests/Core/SchemaTracking/StructureChangeValidationTests.swift index 302e690faf..e05e11ea1f 100644 --- a/TableProTests/Core/SchemaTracking/StructureChangeValidationTests.swift +++ b/TableProTests/Core/SchemaTracking/StructureChangeValidationTests.swift @@ -4,9 +4,9 @@ // import Foundation +@testable import TablePro import TableProPluginKit import Testing -@testable import TablePro /// The gate that stops an incomplete row reaching DDL generation. /// @@ -161,4 +161,281 @@ struct StructureChangeValidationTests { #expect(summary.contains("Foreign key")) #expect(summary.contains("Index")) } + + // MARK: - Columns the save does not touch + + private func column(_ name: String, type: String, isPrimaryKey: Bool = false) -> ColumnInfo { + ColumnInfo( + name: name, dataType: type, isNullable: !isPrimaryKey, isPrimaryKey: isPrimaryKey, + defaultValue: nil, extra: nil, charset: nil, collation: nil, comment: nil + ) + } + + private func schemaManager( + loading columns: [ColumnInfo], + primaryKey: [String], + table: String = "notes" + ) -> StructureChangeManager { + let manager = StructureChangeManager() + manager.loadSchema(tableName: table, columns: columns, indexes: [], foreignKeys: [], primaryKey: primaryKey) + return manager + } + + /// `CREATE TABLE notes (id INTEGER PRIMARY KEY, body, tag TEXT)`. Measured on SQLite 3.53.4, + /// `PRAGMA table_xinfo` reports `body`'s type as the empty string, and the driver passes it on. + private func notesWithATypelessColumn() -> StructureChangeManager { + schemaManager( + loading: [ + column("id", type: "INTEGER", isPrimaryKey: true), + column("body", type: ""), + column("tag", type: "TEXT") + ], + primaryKey: ["id"] + ) + } + + private func edit( + _ name: String, + in manager: StructureChangeManager, + _ change: (inout EditableColumnDefinition) -> Void + ) throws { + var column = try #require(manager.workingColumns.first(where: { $0.name == name })) + change(&column) + manager.updateColumn(id: column.id, with: column) + } + + private func delete(_ name: String, in manager: StructureChangeManager) throws { + let column = try #require(manager.workingColumns.first(where: { $0.name == name })) + manager.deleteColumn(id: column.id) + } + + @Test("A column declared without a type does not block a save that leaves it alone") + func untouchedTypelessColumnDoesNotBlockTheSave() throws { + let manager = notesWithATypelessColumn() + try edit("tag", in: manager) { $0.name = "label" } + #expect(manager.canCommit) + #expect(manager.validationSummary.isEmpty) + } + + /// MongoDB 7 stores fields named `""` and `" "`, and the flattener lists both as columns. + @Test("A field with an empty name does not block a save that leaves it alone") + func untouchedEmptyNamedFieldDoesNotBlockTheSave() throws { + let manager = schemaManager( + loading: [ + column("_id", type: "ObjectId", isPrimaryKey: true), + column("", type: "VARCHAR"), + column(" ", type: "VARCHAR"), + column("a", type: "INTEGER") + ], + primaryKey: ["_id"], + table: "c" + ) + try edit("a", in: manager) { $0.name = "alpha" } + #expect(manager.canCommit) + #expect(manager.validationSummary.isEmpty) + } + + @Test("Renaming a column that has no type does not ask for one") + func renamingATypelessColumnDoesNotAskForAType() throws { + let manager = notesWithATypelessColumn() + try edit("body", in: manager) { $0.name = "content" } + #expect(manager.canCommit) + } + + @Test("Making a column that has no type NOT NULL does not ask for a type") + func nullabilityEditOnATypelessColumnDoesNotAskForAType() throws { + let manager = notesWithATypelessColumn() + try edit("body", in: manager) { $0.setNullable(false) } + #expect(manager.canCommit) + } + + @Test("Naming a field that had no name is not refused") + func namingAnEmptyNamedFieldIsAllowed() throws { + let manager = schemaManager( + loading: [column("_id", type: "ObjectId", isPrimaryKey: true), column(" ", type: "VARCHAR")], + primaryKey: ["_id"], + table: "c" + ) + try edit(" ", in: manager) { $0.name = "spaces" } + #expect(manager.canCommit) + } + + @Test("Deleting a column that has no type does not block the save") + func deletingATypelessColumnDoesNotBlockTheSave() throws { + let manager = notesWithATypelessColumn() + try delete("body", in: manager) + #expect(manager.canCommit) + } + + @Test("A primary key on a column with no type is found") + func primaryKeyOnATypelessColumnIsFound() throws { + let manager = schemaManager( + loading: [column("k", type: "", isPrimaryKey: true), column("v", type: "INTEGER")], + primaryKey: ["k"], + table: "keyed" + ) + try edit("v", in: manager) { $0.name = "value" } + #expect(manager.canCommit) + #expect(manager.validationSummary.isEmpty) + } + + /// Measured on SQLite 3.54: `RENAME COLUMN id TO note_id` keeps the key, and `pk` reads 1 on + /// the renamed column. + @Test("Renaming the primary key column does not block the save") + func renamingThePrimaryKeyColumnDoesNotBlockTheSave() throws { + let manager = notesWithATypelessColumn() + try edit("id", in: manager) { $0.name = "note_id" } + #expect(manager.canCommit) + #expect(manager.validationSummary.isEmpty) + } + + /// MySQL and PostgreSQL drop the key with the column, and SQLite refuses with "cannot drop + /// PRIMARY KEY column". Either way it is the database's answer, not a missing column. + @Test("Deleting a primary key column is left to the database") + func deletingAPrimaryKeyColumnIsLeftToTheDatabase() throws { + let manager = notesWithATypelessColumn() + try delete("id", in: manager) + #expect(manager.canCommit) + } + + @Test("An index on a column with no type is not refused as naming a missing column") + func indexOnATypelessColumnIsFound() { + let manager = notesWithATypelessColumn() + manager.addIndex( + EditableIndexDefinition( + id: UUID(), name: "notes_body", columns: ["body"], type: .btree, isUnique: false, + isPrimary: false, comment: nil + ) + ) + #expect(manager.canCommit) + #expect(manager.validationSummary.isEmpty) + } + + /// Measured on SQLite 3.53.4: `ADD COLUMN body INTEGER` beside a typeless `body` fails with + /// "duplicate column name: body". A column with no type still holds its name. + @Test("Adding a column named like one that has no type is a duplicate") + func addingAColumnNamedLikeATypelessOneIsADuplicate() { + let manager = notesWithATypelessColumn() + var added = EditableColumnDefinition.placeholder() + added.name = "body" + added.dataType = "INTEGER" + manager.addColumn(added) + #expect(!manager.canCommit) + #expect(manager.validationSummary.contains("Duplicate column name: body")) + #expect(!manager.validationSummary.contains("must have a name")) + } + + @Test("Two loaded columns with one name do not block an unrelated save") + func untouchedDuplicateNamesDoNotBlockAnUnrelatedSave() throws { + let manager = schemaManager( + loading: [column("x", type: "INTEGER"), column("x", type: "INTEGER"), column("y", type: "TEXT")], + primaryKey: [] + ) + try edit("y", in: manager) { $0.name = "z" } + #expect(manager.canCommit) + } + + // MARK: - Edits that still block the save + + @Test("Clearing a column's type blocks the save") + func clearingAColumnsTypeBlocksTheSave() throws { + let manager = notesWithATypelessColumn() + try edit("tag", in: manager) { $0.dataType = "" } + #expect(!manager.canCommit) + #expect(manager.validationSummary.contains("Column must have a name and a data type")) + } + + @Test("Clearing a column's name blocks the save") + func clearingAColumnsNameBlocksTheSave() throws { + let manager = notesWithATypelessColumn() + try edit("tag", in: manager) { $0.name = " " } + #expect(!manager.canCommit) + #expect(manager.validationSummary.contains("Column must have a name and a data type")) + } + + @Test("Clearing the name of a column that has no type blocks the save") + func clearingATypelessColumnsNameBlocksTheSave() throws { + let manager = notesWithATypelessColumn() + try edit("body", in: manager) { $0.name = "" } + #expect(!manager.canCommit) + } + + @Test("The blank column the add button stages blocks the save") + func blankAddedColumnBlocksTheSave() { + let manager = notesWithATypelessColumn() + manager.addNewColumn() + #expect(!manager.canCommit) + #expect(manager.validationSummary.contains("Column must have a name and a data type")) + } + + @Test("Two blank added columns are incomplete, not duplicates") + func twoBlankRowsAreIncompleteNotDuplicates() { + let manager = notesWithATypelessColumn() + manager.addNewColumn() + manager.addNewColumn() + #expect(!manager.canCommit) + #expect(!manager.validationSummary.contains("Duplicate")) + #expect(manager.validationSummary.contains("Column must have a name and a data type")) + } + + @Test("Renaming a column onto another column's name is a duplicate") + func renamingOntoAnExistingNameIsADuplicate() throws { + let manager = notesWithATypelessColumn() + try edit("tag", in: manager) { $0.name = "id" } + #expect(!manager.canCommit) + #expect(manager.validationSummary.contains("Duplicate column name: id")) + } + + // MARK: - Blank names the table already holds + + /// `CREATE TABLE blanks (id INTEGER PRIMARY KEY, "" TEXT, " " TEXT, tag TEXT)`. SQLite holds + /// `""` and `" "` as two columns, and measured on 3.54.0 `RENAME COLUMN " " TO ""` fails with + /// "duplicate column name: ". + private func tableWithBlankNames() -> StructureChangeManager { + schemaManager( + loading: [ + column("id", type: "INTEGER", isPrimaryKey: true), + column("", type: "TEXT"), + column(" ", type: "TEXT"), + column("tag", type: "TEXT") + ], + primaryKey: ["id"], + table: "blanks" + ) + } + + @Test("Renaming a column onto a blank name the table holds is a duplicate") + func renamingOntoALoadedBlankNameIsADuplicate() throws { + let manager = tableWithBlankNames() + try edit(" ", in: manager) { $0.name = "" } + #expect(!manager.canCommit) + #expect(manager.validationSummary.contains("Duplicate column name")) + } + + /// Measured on SQLite 3.54.0: `RENAME COLUMN "" TO " "` beside `" "` succeeds. + @Test("Blank names of different lengths are different names") + func blankNamesOfDifferentLengthsDoNotCollide() throws { + let manager = tableWithBlankNames() + try edit("", in: manager) { $0.name = " " } + #expect(manager.canCommit) + #expect(manager.validationSummary.isEmpty) + } + + @Test("A blank added column beside a blank name the table holds is incomplete, not a duplicate") + func blankAddedColumnBesideALoadedBlankNameIsIncomplete() { + let manager = tableWithBlankNames() + manager.addNewColumn() + #expect(!manager.canCommit) + #expect(!manager.validationSummary.contains("Duplicate")) + #expect(manager.validationSummary.contains("Column must have a name and a data type")) + } + + @Test("Clearing a name beside a blank name the table holds is incomplete, not a duplicate") + func clearedNameBesideALoadedBlankNameIsIncomplete() throws { + let manager = tableWithBlankNames() + try edit("tag", in: manager) { $0.name = "" } + #expect(!manager.canCommit) + #expect(!manager.validationSummary.contains("Duplicate")) + #expect(manager.validationSummary.contains("Column must have a name and a data type")) + } } diff --git a/TableProTests/Models/Schema/ColumnDefinitionTests.swift b/TableProTests/Models/Schema/ColumnDefinitionTests.swift index 6223608267..a406979252 100644 --- a/TableProTests/Models/Schema/ColumnDefinitionTests.swift +++ b/TableProTests/Models/Schema/ColumnDefinitionTests.swift @@ -88,6 +88,92 @@ struct ColumnDefinitionTests { #expect(column.isValid == false) } + // MARK: - Completeness against the loaded column + + private func column(name: String, dataType: String) -> EditableColumnDefinition { + var column = EditableColumnDefinition.placeholder() + column.name = name + column.dataType = dataType + return column + } + + @Test("A blank or whitespace-only name is no name, and the same for a type") + func nameAndTypeIgnoreWhitespace() { + #expect(!column(name: "", dataType: "INT").hasName) + #expect(!column(name: " ", dataType: "INT").hasName) + #expect(column(name: "a", dataType: "INT").hasName) + #expect(!column(name: "a", dataType: "").hasDataType) + #expect(!column(name: "a", dataType: " ").hasDataType) + #expect(column(name: "a", dataType: "INT").hasDataType) + } + + @Test("A column with nothing loaded behind it needs a name and a type") + func addedColumnNeedsBoth() { + #expect(EditableColumnDefinition.placeholder().isIncomplete(over: nil)) + #expect(column(name: "a", dataType: "").isIncomplete(over: nil)) + #expect(column(name: "", dataType: "INT").isIncomplete(over: nil)) + #expect(!column(name: "a", dataType: "INT").isIncomplete(over: nil)) + } + + /// SQLite reports a column declared without a type as type `''`. + @Test("A loaded column with no type is not asked for one") + func typelessLoadedColumnStaysComplete() { + let loaded = column(name: "body", dataType: "") + var renamed = loaded + renamed.name = "content" + #expect(!loaded.isIncomplete(over: loaded)) + #expect(!renamed.isIncomplete(over: loaded)) + } + + /// SQLite accepts `CREATE TABLE e("" TEXT)`, and MongoDB stores a field named `""`. + @Test("A loaded column with no name is not asked for one") + func namelessLoadedColumnStaysComplete() { + let loaded = column(name: "", dataType: "TEXT") + var retyped = loaded + retyped.dataType = "INTEGER" + #expect(!loaded.isIncomplete(over: loaded)) + #expect(!retyped.isIncomplete(over: loaded)) + } + + @Test("Clearing a type or a name the loaded column had is incomplete") + func clearingWhatWasLoadedIsIncomplete() { + let loaded = column(name: "tag", dataType: "INTEGER") + var untyped = loaded + untyped.dataType = "" + var unnamed = loaded + unnamed.name = " " + #expect(untyped.isIncomplete(over: loaded)) + #expect(unnamed.isIncomplete(over: loaded)) + } + + /// SQLite keeps `""` and `" "` as two columns, and renaming one onto the other fails with + /// "duplicate column name", so a blank name read from the table is compared like any other. + @Test("A blank name is a savable name only where the column was read with one") + func blankNameIsSavableOnlyOverALoadedBlankName() { + let loadedBlank = column(name: " ", dataType: "TEXT") + var renamedBlank = loadedBlank + renamedBlank.name = "" + let loadedNamed = column(name: "tag", dataType: "TEXT") + var cleared = loadedNamed + cleared.name = "" + #expect(renamedBlank.hasSavableName(over: loadedBlank)) + #expect(!cleared.hasSavableName(over: loadedNamed)) + #expect(!EditableColumnDefinition.placeholder().hasSavableName(over: nil)) + #expect(column(name: "a", dataType: "").hasSavableName(over: nil)) + } + + @Test("A missing type is savable only where the column was read without one") + func missingTypeIsSavableOnlyOverATypelessLoadedColumn() { + let typeless = column(name: "body", dataType: "") + let typed = column(name: "tag", dataType: "TEXT") + var untyped = typed + untyped.dataType = " " + #expect(typeless.hasSavableDataType(over: typeless)) + #expect(!untyped.hasSavableDataType(over: typed)) + #expect(!column(name: "a", dataType: "").hasSavableDataType(over: nil)) + #expect(column(name: "a", dataType: "INT").hasSavableDataType(over: nil)) + } + // MARK: - Round-trip Conversion Tests @Test("from(ColumnInfo) creates EditableColumnDefinition with matching fields") diff --git a/TableProTests/Models/Schema/SQLiteColumnDeclarationTests.swift b/TableProTests/Models/Schema/SQLiteColumnDeclarationTests.swift index 15c586ba4e..434b6836fe 100644 --- a/TableProTests/Models/Schema/SQLiteColumnDeclarationTests.swift +++ b/TableProTests/Models/Schema/SQLiteColumnDeclarationTests.swift @@ -4,8 +4,8 @@ // import Foundation -import TableProPluginKit @testable import TablePro +import TableProPluginKit import Testing /// The column-definition grammar the rebuild rewrites through. @@ -114,6 +114,16 @@ struct SQLiteColumnDeclarationTests { #expect(rewrite("a NOT NULL", type: "TEXT") == "a TEXT NOT NULL") } + /// Measured on 3.54: `CREATE TABLE n2(body NOT NULL, tag TEXT)` keeps `body`'s type empty with + /// `notnull` 1, so a nullability or default edit on a typeless column must not invent a type. + @Test("A column with no type keeps none when its nullability or default changes") + func editsAnUntypedColumnWithoutTypingIt() { + #expect(rewrite("body", isNullable: false) == "body NOT NULL") + #expect(rewrite("body NOT NULL", isNullable: true) == "body") + #expect(rewrite("body", defaultValue: "'x'") == "body DEFAULT 'x'") + #expect(rewrite("body DEFAULT 'x'", defaultValue: "") == "body") + } + /// Dropping the constraint takes its `ON CONFLICT` tail with it: measured, an orphaned /// `ON CONFLICT ROLLBACK` is a parse error. @Test("Dropping NOT NULL takes its whole clause") diff --git a/TableProTests/Models/Schema/SQLiteTableRespecifierTests.swift b/TableProTests/Models/Schema/SQLiteTableRespecifierTests.swift index cfa1117d03..d6260db302 100644 --- a/TableProTests/Models/Schema/SQLiteTableRespecifierTests.swift +++ b/TableProTests/Models/Schema/SQLiteTableRespecifierTests.swift @@ -4,8 +4,8 @@ // import Foundation -import TableProPluginKit @testable import TablePro +import TableProPluginKit import Testing struct SQLiteTableRespecifierTests { @@ -246,6 +246,19 @@ struct SQLiteTableRespecifierTests { #expect(parsed.columnNames == ["a", "b"]) } + /// `PRAGMA table_info` reports a column declared without a type as having the empty string for + /// one. Measured on 3.54, the declaration this writes reads back with that same empty type and + /// `notnull` 1, so the edit neither invents a type nor loses the column's affinity. + @Test("A nullability edit on a column with no type leaves it typeless") + func altersAnUntypedColumn() throws { + let respecified = try respecify( + PluginTableRespecification(alteredColumns: [PluginColumnAlteration(column: "body", isNullable: false)]), + sql: "CREATE TABLE notes(id INTEGER PRIMARY KEY, body, tag TEXT)" + ) + #expect(respecified.createTableSQL == "CREATE TABLE \"x_new\" (\n id INTEGER PRIMARY KEY,\n body NOT NULL,\n tag TEXT\n)") + #expect(respecified.carriedColumns.map { $0.name } == ["id", "body", "tag"]) + } + // MARK: - Order @Test("A wanted order rearranges the columns and the copy list together") diff --git a/TableProUITests/CloseTabBeforeFirstClickUITests.swift b/TableProUITests/CloseTabBeforeFirstClickUITests.swift index eb9e77d4fd..a207480ce3 100644 --- a/TableProUITests/CloseTabBeforeFirstClickUITests.swift +++ b/TableProUITests/CloseTabBeforeFirstClickUITests.swift @@ -1,4 +1,3 @@ -import SQLite3 import XCTest /// Command W closed the whole connection instead of the current tab in a window nobody had clicked @@ -32,7 +31,11 @@ final class CloseTabBeforeFirstClickUITests: UITestCase { /// Two restored connections, so the strip is on screen by the time Command W is pressed, which /// is where anyone who reopens more than one connection lands. func testCommandWInARestoredSessionClosesATabAndKeepsBothConnections() throws { - try seedSession(connectionNames: ["Restored A", "Restored B"], tabsEach: 3) + try seedSQLiteSession( + connectionNames: ["Restored A", "Restored B"], + databaseSQL: "CREATE TABLE items (id INTEGER PRIMARY KEY, name TEXT); INSERT INTO items (name) VALUES ('one');", + tabsEach: 3 + ) let app = try launchApp() let window = app.windows["main"] let strip = window.tables.matching(identifier: "workspace-rail").firstMatch @@ -58,78 +61,4 @@ final class CloseTabBeforeFirstClickUITests: UITestCase { private func tabCount(in window: XCUIElement) -> Int { window.descendants(matching: .any).matching(identifier: "editor-tab").count } - - // MARK: - Fixture - - /// The files the app reads to reopen the last session: the connections, which of them were - /// open, and each one's tabs. - private func seedSession(connectionNames: [String], tabsEach: Int) throws { - let root = try XCTUnwrap(sandboxRoot, "setUpWithError did not prepare a sandbox") - let supportDirectory = root.appendingPathComponent("TablePro", isDirectory: true) - let tabStateDirectory = supportDirectory.appendingPathComponent("TabState", isDirectory: true) - try FileManager.default.createDirectory(at: tabStateDirectory, withIntermediateDirectories: true) - - var connections: [[String: Any]] = [] - var connectionIds: [String] = [] - for (index, name) in connectionNames.enumerated() { - let id = UUID().uuidString - let databaseURL = root.appendingPathComponent("restored-\(index).sqlite") - makeDatabase(at: databaseURL) - connections.append(connectionPayload(id: id, name: name, databasePath: databaseURL.path, sortOrder: index)) - connectionIds.append(id) - try writeJSON( - tabState(tabCount: tabsEach), - to: tabStateDirectory.appendingPathComponent("\(id).json") - ) - } - try writeJSON(connections, to: supportDirectory.appendingPathComponent("connections.json")) - try writeJSON(connectionIds, to: supportDirectory.appendingPathComponent("LastOpenConnections.json")) - } - - private func connectionPayload(id: String, name: String, databasePath: String, sortOrder: Int) -> [String: Any] { - [ - "id": id, - "name": name, - "host": "", - "port": 0, - "database": databasePath, - "username": "", - "type": "SQLite", - "sshEnabled": false, - "sshHost": "", - "sshUsername": "", - "sshAuthMethod": "password", - "sshPrivateKeyPath": "", - "sortOrder": sortOrder, - ] - } - - /// Query tabs, so each one mounts the SQL editor and nothing depends on a table's rows loading. - private func tabState(tabCount: Int) -> [String: Any] { - let tabs: [[String: Any]] = (1 ... tabCount).map { number in - [ - "id": UUID().uuidString, - "title": "Query \(number)", - "query": "SELECT \(number);", - "tabType": ["query": [String: Any]()], - "tableName": NSNull(), - "databaseName": "", - "isView": false, - ] - } - return ["tabs": tabs, "selectedTabId": tabs[0]["id"] ?? ""] - } - - private func writeJSON(_ object: Any, to url: URL) throws { - try JSONSerialization.data(withJSONObject: object, options: [.sortedKeys]) - .write(to: url, options: .atomic) - } - - private func makeDatabase(at url: URL) { - var handle: OpaquePointer? - defer { sqlite3_close(handle) } - XCTAssertEqual(sqlite3_open(url.path, &handle), SQLITE_OK, "Could not create \(url.path)") - let statement = "CREATE TABLE items (id INTEGER PRIMARY KEY, name TEXT); INSERT INTO items (name) VALUES ('one');" - XCTAssertEqual(sqlite3_exec(handle, statement, nil, nil, nil), SQLITE_OK) - } } diff --git a/TableProUITests/StructureTypelessColumnUITests.swift b/TableProUITests/StructureTypelessColumnUITests.swift new file mode 100644 index 0000000000..ce702a58db --- /dev/null +++ b/TableProUITests/StructureTypelessColumnUITests.swift @@ -0,0 +1,75 @@ +// +// StructureTypelessColumnUITests.swift +// TableProUITests +// + +import XCTest + +/// SQLite reads a column declared without a type, `body` in `CREATE TABLE notes (tag TEXT, body)`, +/// with an empty type. Save held every column in the table to having one, the untouched ones and +/// the one being dropped included, so any change to such a table was refused as "Some Changes Are +/// Incomplete" before it ran. +/// +/// Either row of the grid can take the click. Dropping `tag` leaves the typeless `body` untouched, +/// and dropping `body` is the typeless column itself on its way out; both were refused. +final class StructureTypelessColumnUITests: UITestCase { + func testDroppingAColumnFromATableWithATypelessColumnSaves() throws { + let databases = try seedSQLiteSession( + connectionNames: ["Typeless"], + databaseSQL: "CREATE TABLE notes (tag TEXT, body); INSERT INTO notes VALUES ('a', 1);" + ) + let database = try XCTUnwrap(databases.first) + let app = try launchApp() + let window = app.windows.firstMatch + + 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 grid = window.tables.matching(identifier: "data-grid").firstMatch + XCTAssertTrue(grid.waitToExist(timeout: 30), "The structure editor must draw its column grid") + XCTAssertTrue( + waitForPredicate(timeout: 30) { + grid.frame.width > 0 && grid.frame.height > 0 && grid.tableRows.count == 2 + }, + "notes has two columns, so the grid must list two rows" + ) + + gridPoint(in: grid, of: window, dy: 40).click() + XCTAssertTrue( + waitForPredicate(timeout: 10) { grid.tableRows.allElementsBoundByIndex.contains { $0.isSelected } }, + "The click must select a column row" + ) + + let remove = window.buttons["structure-footer-remove"].firstMatch + XCTAssertTrue(remove.waitToExist(timeout: 20), "The Columns tab must offer a remove button") + XCTAssertTrue(waitForPredicate(timeout: 10) { remove.isEnabled }, "Removing the selected column must be offered") + remove.click() + + app.typeKey("s", modifierFlags: .command) + + let sheet = app.sheets.firstMatch + XCTAssertTrue(sheet.waitToExist(timeout: 20), "Dropping a column must ask before it runs") + let text = sheet.staticTexts.allElementsBoundByIndex + .map { ($0.value as? String) ?? $0.label } + .joined(separator: " ") + XCTAssertFalse( + text.contains("must have a name and a data type"), + "A column with no type must not stop the save, got: \(text)" + ) + let apply = sheet.buttons["Apply Changes"].firstMatch + XCTAssertTrue(apply.waitToExist(timeout: 5), "The drop must be offered for confirmation, got: \(text)") + apply.click() + + XCTAssertTrue( + waitForPredicate(timeout: 30) { grid.tableRows.count == 1 }, + "The grid must list the one column the save kept" + ) + XCTAssertEqual( + sqliteStrings("SELECT name FROM pragma_table_xinfo('notes')", in: database).count, + 1, + "The column must be gone from the file, not only from the grid" + ) + } +} diff --git a/TableProUITests/Support/SeededSQLiteSession.swift b/TableProUITests/Support/SeededSQLiteSession.swift new file mode 100644 index 0000000000..d0dbad5ebb --- /dev/null +++ b/TableProUITests/Support/SeededSQLiteSession.swift @@ -0,0 +1,109 @@ +// +// SeededSQLiteSession.swift +// TableProUITests +// + +import SQLite3 +import XCTest + +/// The files the app reads to reopen the last session: the connections, which of them were open, +/// and each one's tabs. Written before launch, they make the app restore a session of SQLite +/// connections the test built, the way it does after a relaunch. +internal extension UITestCase { + /// Each connection gets its own database file built from `databaseSQL`, and `tabsEach` query + /// tabs, so nothing restored depends on a table's rows loading. Returns the database files in + /// connection order. + @discardableResult + func seedSQLiteSession(connectionNames: [String], databaseSQL: String, tabsEach: Int = 1) throws -> [URL] { + let root = try XCTUnwrap(sandboxRoot, "setUpWithError did not prepare a sandbox") + let supportDirectory = root.appendingPathComponent("TablePro", isDirectory: true) + let tabStateDirectory = supportDirectory.appendingPathComponent("TabState", isDirectory: true) + try FileManager.default.createDirectory(at: tabStateDirectory, withIntermediateDirectories: true) + + var connections: [[String: Any]] = [] + var connectionIds: [String] = [] + var databaseURLs: [URL] = [] + for (index, name) in connectionNames.enumerated() { + let id = UUID().uuidString + let databaseURL = root.appendingPathComponent("restored-\(index).sqlite") + makeDatabase(at: databaseURL, sql: databaseSQL) + connections.append(connectionPayload(id: id, name: name, databasePath: databaseURL.path, sortOrder: index)) + connectionIds.append(id) + databaseURLs.append(databaseURL) + try writeJSON( + tabState(tabCount: tabsEach), + to: tabStateDirectory.appendingPathComponent("\(id).json") + ) + } + try writeJSON(connections, to: supportDirectory.appendingPathComponent("connections.json")) + try writeJSON(connectionIds, to: supportDirectory.appendingPathComponent("LastOpenConnections.json")) + return databaseURLs + } + + /// The first column of every row `sql` returns, read straight from the file rather than through + /// the app, so an assertion sees what the database holds and not what the grid shows. + func sqliteStrings(_ sql: String, in databaseURL: URL) -> [String] { + var handle: OpaquePointer? + defer { sqlite3_close(handle) } + guard sqlite3_open_v2(databaseURL.path, &handle, SQLITE_OPEN_READONLY, nil) == SQLITE_OK else { + XCTFail("Could not open \(databaseURL.path)") + return [] + } + var statement: OpaquePointer? + defer { sqlite3_finalize(statement) } + guard sqlite3_prepare_v2(handle, sql, -1, &statement, nil) == SQLITE_OK else { + XCTFail("Could not prepare \(sql): \(String(cString: sqlite3_errmsg(handle)))") + return [] + } + var values: [String] = [] + while sqlite3_step(statement) == SQLITE_ROW { + values.append(sqlite3_column_text(statement, 0).map { String(cString: $0) } ?? "") + } + return values + } + + private func connectionPayload(id: String, name: String, databasePath: String, sortOrder: Int) -> [String: Any] { + [ + "id": id, + "name": name, + "host": "", + "port": 0, + "database": databasePath, + "username": "", + "type": "SQLite", + "sshEnabled": false, + "sshHost": "", + "sshUsername": "", + "sshAuthMethod": "password", + "sshPrivateKeyPath": "", + "sortOrder": sortOrder, + ] + } + + private func tabState(tabCount: Int) -> [String: Any] { + let tabs: [[String: Any]] = (1 ... tabCount).map { number in + [ + "id": UUID().uuidString, + "title": "Query \(number)", + "query": "SELECT \(number);", + "tabType": ["query": [String: Any]()], + "tableName": NSNull(), + "databaseName": "", + "isView": false, + ] + } + return ["tabs": tabs, "selectedTabId": tabs[0]["id"] ?? ""] + } + + private func writeJSON(_ object: Any, to url: URL) throws { + try JSONSerialization.data(withJSONObject: object, options: [.sortedKeys]) + .write(to: url, options: .atomic) + } + + private func makeDatabase(at url: URL, sql: String) { + var handle: OpaquePointer? + defer { sqlite3_close(handle) } + XCTAssertEqual(sqlite3_open(url.path, &handle), SQLITE_OK, "Could not create \(url.path)") + XCTAssertEqual(sqlite3_exec(handle, sql, nil, nil, nil), SQLITE_OK, "Could not run \(sql)") + } +} diff --git a/docs/databases/sqlite.mdx b/docs/databases/sqlite.mdx index 038237d330..e7d08e1b4f 100644 --- a/docs/databases/sqlite.mdx +++ b/docs/databases/sqlite.mdx @@ -108,6 +108,8 @@ A rebuild writes the table with the definition you asked for, copies the rows ac Each column keeps its own stored text, so a `CHECK`, a `COLLATE`, a `GENERATED ALWAYS AS` and a `DEFAULT` containing a comma all survive a rebuild that touched a different column. +A column declared without a type, such as `body` in `CREATE TABLE notes (id INTEGER PRIMARY KEY, body)`, has an empty **Type** cell. `CREATE TABLE … AS SELECT` declares one for every selected expression that is not a plain column or a `CAST`, so `price + 1 AS total` arrives typeless. Rename, index, drop or edit such a column like any other and it stays typeless. A type is required only on a column you add, or on one whose type you clear. + Changing a column's type re-reads every value in it through the new affinity. Text that looks like a number becomes one, so `'007'` in a column retyped to `INTEGER` is stored as `7`. The review sheet says so before the script runs. A virtual table, such as an FTS5 table, cannot be recreated from what SQLite stored for it. The save is refused and names the table. From 41ebd87efdd76d188f24d994185b0c11e8618650 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Sat, 26 Sep 2026 19:55:14 +0700 Subject: [PATCH 2/2] test(structure): answer the execution gate's review before asserting a typeless column drop --- .../StructureTypelessColumnUITests.swift | 20 ++++++++++++++----- .../Support/SeededSQLiteSession.swift | 1 + 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/TableProUITests/StructureTypelessColumnUITests.swift b/TableProUITests/StructureTypelessColumnUITests.swift index ce702a58db..9b05161cf1 100644 --- a/TableProUITests/StructureTypelessColumnUITests.swift +++ b/TableProUITests/StructureTypelessColumnUITests.swift @@ -62,14 +62,24 @@ final class StructureTypelessColumnUITests: UITestCase { XCTAssertTrue(apply.waitToExist(timeout: 5), "The drop must be offered for confirmation, got: \(text)") apply.click() + let execute = window.sheets.buttons["sql-review-execute"].firstMatch + XCTAssertTrue( + execute.waitToExist(timeout: 20), + "Apply Changes hands the drop to the execution gate, which shows the statement before it runs" + ) + let statement = (window.sheets.textViews.firstMatch.value as? String) ?? "" + XCTAssertTrue(statement.contains("DROP COLUMN"), "The gate must be showing the drop, got: \(statement)") + execute.click() + + XCTAssertTrue( + waitForPredicate(timeout: 30) { + sqliteStrings("SELECT name FROM pragma_table_xinfo('notes')", in: database).count == 1 + }, + "The drop must reach the file" + ) XCTAssertTrue( waitForPredicate(timeout: 30) { grid.tableRows.count == 1 }, "The grid must list the one column the save kept" ) - XCTAssertEqual( - sqliteStrings("SELECT name FROM pragma_table_xinfo('notes')", in: database).count, - 1, - "The column must be gone from the file, not only from the grid" - ) } } diff --git a/TableProUITests/Support/SeededSQLiteSession.swift b/TableProUITests/Support/SeededSQLiteSession.swift index d0dbad5ebb..d4f27812df 100644 --- a/TableProUITests/Support/SeededSQLiteSession.swift +++ b/TableProUITests/Support/SeededSQLiteSession.swift @@ -49,6 +49,7 @@ internal extension UITestCase { XCTFail("Could not open \(databaseURL.path)") return [] } + sqlite3_busy_timeout(handle, 5_000) var statement: OpaquePointer? defer { sqlite3_finalize(statement) } guard sqlite3_prepare_v2(handle, sql, -1, &statement, nil) == SQLITE_OK else {