From 27a4215424cea84bdece40dfc13e309d22db6a57 Mon Sep 17 00:00:00 2001 From: Ben Barnett Date: Fri, 8 Dec 2023 18:22:34 +0000 Subject: [PATCH] Add schemaName parameter to foreign key checking functions --- GRDB/Core/Database+Schema.swift | 40 ++++-- GRDB/Documentation.docc/DatabaseSchema.md | 4 +- GRDB/Documentation.docc/Migrations.md | 2 +- GRDB/Migration/DatabaseMigrator.swift | 4 +- Tests/GRDBTests/ForeignKeyInfoTests.swift | 164 ++++++++++++++++++++++ 5 files changed, 201 insertions(+), 13 deletions(-) diff --git a/GRDB/Core/Database+Schema.swift b/GRDB/Core/Database+Schema.swift index b145eb6196..f636f91698 100644 --- a/GRDB/Core/Database+Schema.swift +++ b/GRDB/Core/Database+Schema.swift @@ -692,10 +692,27 @@ extension Database { /// Returns a cursor over foreign key violations in the table. /// - /// - throws: A ``DatabaseError`` whenever an SQLite error occurs, or if no - /// such table exists in the main or temp schema, or in an - /// attached database. - public func foreignKeyViolations(in tableName: String) throws -> RecordCursor { + /// When `schemaName` is not specified, known schemas are checked in + /// SQLite resolution order and the first matching table is used. + /// + /// - throws: A ``DatabaseError`` whenever an SQLite error occurs, if + /// the specified schema does not exist, or if no such table or view + /// with this name exists in the main or temp schema, or in an attached + /// database. + public func foreignKeyViolations( + in tableName: String, + in schemaName: String? = nil) + throws -> RecordCursor + { + if let schemaName { + let schemaID = try schemaIdentifier(named: schemaName) + if try exists(type: .table, name: tableName, in: schemaID) { + return try foreignKeyViolations(in: TableIdentifier(schemaID: schemaID, name: tableName)) + } else { + throw DatabaseError.noSuchTable(tableName) + } + } + for schemaIdentifier in try schemaIdentifiers() { if try exists(type: .table, name: tableName, in: schemaIdentifier) { return try foreignKeyViolations(in: TableIdentifier(schemaID: schemaIdentifier, name: tableName)) @@ -724,14 +741,21 @@ extension Database { /// Throws an error if there exists a foreign key violation in the table. /// + /// When `schemaName` is not specified, known schemas are checked in + /// SQLite resolution order and the first matching table is used. + /// /// On the first foreign key violation found in the table, this method /// throws a ``DatabaseError`` with extended code /// `SQLITE_CONSTRAINT_FOREIGNKEY`. /// /// If you are looking for the list of foreign key violations, prefer - /// ``foreignKeyViolations(in:)`` instead. - public func checkForeignKeys(in tableName: String) throws { - try checkForeignKeys(from: foreignKeyViolations(in: tableName)) + /// ``foreignKeyViolations(in:in:)`` instead. + /// + /// - throws: A ``DatabaseError`` as described above; when a + /// specified schema does not exist; if no such table or view with this + /// name exists in the main or temp schema or in an attached database. + public func checkForeignKeys(in tableName: String, in schemaName: String? = nil) throws { + try checkForeignKeys(from: foreignKeyViolations(in: tableName, in: schemaName)) } private func checkForeignKeys(from violations: RecordCursor) throws { @@ -1095,7 +1119,7 @@ public struct IndexInfo { /// /// You get instances of `ForeignKeyViolation` from the `Database` methods /// ``Database/foreignKeyViolations()`` and -/// ``Database/foreignKeyViolations(in:)`` methods. +/// ``Database/foreignKeyViolations(in:in:)`` methods. /// /// For example: /// diff --git a/GRDB/Documentation.docc/DatabaseSchema.md b/GRDB/Documentation.docc/DatabaseSchema.md index 3d1bc8e948..abfd53021b 100644 --- a/GRDB/Documentation.docc/DatabaseSchema.md +++ b/GRDB/Documentation.docc/DatabaseSchema.md @@ -410,9 +410,9 @@ extension Team: TableRecord { ### Integrity Checks - ``Database/checkForeignKeys()`` -- ``Database/checkForeignKeys(in:)`` +- ``Database/checkForeignKeys(in:in:)`` - ``Database/foreignKeyViolations()`` -- ``Database/foreignKeyViolations(in:)`` +- ``Database/foreignKeyViolations(in:in:)`` - ``ForeignKeyViolation`` ### Sunsetted Methods diff --git a/GRDB/Documentation.docc/Migrations.md b/GRDB/Documentation.docc/Migrations.md index 0af07bddc7..ee7ed0f837 100644 --- a/GRDB/Documentation.docc/Migrations.md +++ b/GRDB/Documentation.docc/Migrations.md @@ -233,7 +233,7 @@ To prevent a migration from committing foreign key violations on disk, you can: } ``` -As in the above example, check for foreign key violations with the ``Database/checkForeignKeys()`` and ``Database/checkForeignKeys(in:)`` methods. They throw a nicely detailed ``DatabaseError`` that contains a lot of debugging information: +As in the above example, check for foreign key violations with the ``Database/checkForeignKeys()`` and ``Database/checkForeignKeys(in:in:)`` methods. They throw a nicely detailed ``DatabaseError`` that contains a lot of debugging information: ```swift // SQLite error 19: FOREIGN KEY constraint violation - from book(authorId) to author(id), diff --git a/GRDB/Migration/DatabaseMigrator.swift b/GRDB/Migration/DatabaseMigrator.swift index 9757c85b07..88b178c7ce 100644 --- a/GRDB/Migration/DatabaseMigrator.swift +++ b/GRDB/Migration/DatabaseMigrator.swift @@ -50,7 +50,7 @@ public struct DatabaseMigrator { /// ``DatabaseMigrator/disablingDeferredForeignKeyChecks()``. /// /// In this case, you can perform your own deferred foreign key checks - /// with ``Database/checkForeignKeys(in:)`` or + /// with ``Database/checkForeignKeys(in:in:)`` or /// ``Database/checkForeignKeys()``: /// /// ```swift @@ -118,7 +118,7 @@ public struct DatabaseMigrator { /// The returned migrator is _unsafe_, because it no longer guarantees the /// integrity of the database. It is now _your_ responsibility to register /// migrations that do not break foreign key constraints. See - /// ``Database/checkForeignKeys()`` and ``Database/checkForeignKeys(in:)``. + /// ``Database/checkForeignKeys()`` and ``Database/checkForeignKeys(in:in:)``. /// /// Running migrations without foreign key checks can improve migration /// performance on huge databases. diff --git a/Tests/GRDBTests/ForeignKeyInfoTests.swift b/Tests/GRDBTests/ForeignKeyInfoTests.swift index 87ca1e749e..85a103d0d2 100644 --- a/Tests/GRDBTests/ForeignKeyInfoTests.swift +++ b/Tests/GRDBTests/ForeignKeyInfoTests.swift @@ -319,4 +319,168 @@ class ForeignKeyInfoTests: GRDBTestCase { } } } + + func testForeignKeyViolationsUnknownSchema() throws { + let dbQueue = try makeDatabaseQueue() + try dbQueue.writeWithoutTransaction { db in + try db.execute(sql: "CREATE TABLE parent (id PRIMARY KEY)") + try db.execute(sql: "CREATE TABLE child (parentId REFERENCES parent)") + do { + _ = try db.foreignKeyViolations(in: "child", in: "invalid") + XCTFail("Expected Error") + } catch let error as DatabaseError { + XCTAssertEqual(error.resultCode, .SQLITE_ERROR) + XCTAssertEqual(error.message, "no such schema: invalid") + XCTAssertEqual(error.description, "SQLite error 1: no such schema: invalid") + } + + do { + _ = try db.checkForeignKeys(in: "child", in: "invalid") + XCTFail("Expected Error") + } catch let error as DatabaseError { + XCTAssertEqual(error.resultCode, .SQLITE_ERROR) + XCTAssertEqual(error.message, "no such schema: invalid") + XCTAssertEqual(error.description, "SQLite error 1: no such schema: invalid") + } + } + } + + func testForeignKeyViolationsMainSchema() throws { + let dbQueue = try makeDatabaseQueue() + try dbQueue.writeWithoutTransaction { db in + try db.execute(sql: """ + CREATE TABLE parent(id TEXT NOT NULL PRIMARY KEY); + CREATE TABLE child(id INTEGER NOT NULL PRIMARY KEY, parentId TEXT REFERENCES parent(id)); + PRAGMA foreign_keys = OFF; + INSERT INTO child (id, parentId) VALUES (13, '1'); + """) + do { + let violations = try Array(db.foreignKeyViolations(in: "child", in: "main")) + XCTAssertEqual(violations.count, 1) + } + do { + _ = try db.checkForeignKeys(in: "child", in: "main") + XCTFail("Expected Error") + } catch let error as DatabaseError { + XCTAssertEqual(DatabaseError.SQLITE_CONSTRAINT_FOREIGNKEY, error.extendedResultCode) + } + } + } + + func testForeignKeyViolationsInSpecifiedSchemaWithTableNameCollisions() throws { + let attached = try makeDatabaseQueue(filename: "attached1") + try attached.inDatabase { db in + try db.execute(sql: """ + CREATE TABLE parent(id TEXT NOT NULL PRIMARY KEY); + CREATE TABLE child(id INTEGER NOT NULL PRIMARY KEY, parentId TEXT REFERENCES parent(id)); + PRAGMA foreign_keys = OFF; + INSERT INTO child (id, parentId) VALUES (20, '1'); + """) + } + let main = try makeDatabaseQueue(filename: "main") + try main.inDatabase { db in + try db.execute(sql: """ + CREATE TABLE parent(id TEXT NOT NULL PRIMARY KEY); + CREATE TABLE child(id INTEGER NOT NULL PRIMARY KEY, parentId TEXT REFERENCES parent(id)); + PRAGMA foreign_keys = OFF; + INSERT INTO child (id, parentId) VALUES (10, '1'); + """) + try db.execute(literal: "ATTACH DATABASE \(attached.path) AS attached") + + do { + let violations = try Array(try db.foreignKeyViolations(in: "child", in: "attached")) + XCTAssertEqual(violations.count, 1) + if let violation = violations.first(where: { $0.originRowID == 20 }) { + XCTAssertEqual(violation.originTable, "child") + XCTAssertEqual(violation.destinationTable, "parent") + } else { + XCTFail("Missing violation") + } + } + + do { + _ = try db.checkForeignKeys(in: "child", in: "attached") + XCTFail("Expected Error") + } catch let error as DatabaseError { + XCTAssertEqual(DatabaseError.SQLITE_CONSTRAINT_FOREIGNKEY, error.extendedResultCode) + } + } + } + + // The `child` table in the attached database should not + // be found unless explicitly specified as it is after + // `main.child` in resolution order. + func testForeignKeyViolationsInUnspecifiedSchemaWithTableNameCollisions() throws { + let attached = try makeDatabaseQueue(filename: "attached1") + try attached.inDatabase { db in + try db.execute(sql: """ + CREATE TABLE parent(id TEXT NOT NULL PRIMARY KEY); + CREATE TABLE child(id INTEGER NOT NULL PRIMARY KEY, parentId TEXT REFERENCES parent(id)); + PRAGMA foreign_keys = OFF; + INSERT INTO child (id, parentId) VALUES (20, '1'); + """) + } + let main = try makeDatabaseQueue(filename: "main") + try main.inDatabase { db in + try db.execute(sql: """ + CREATE TABLE parent(id TEXT NOT NULL PRIMARY KEY); + CREATE TABLE child(id INTEGER NOT NULL PRIMARY KEY, parentId TEXT REFERENCES parent(id)); + PRAGMA foreign_keys = OFF; + INSERT INTO child (id, parentId) VALUES (10, '1'); + """) + try db.execute(literal: "ATTACH DATABASE \(attached.path) AS attached") + + do { + let violations = try Array(try db.foreignKeyViolations(in: "child")) + XCTAssertEqual(violations.count, 1) + if let violation = violations.first(where: { $0.originRowID == 10 }) { + XCTAssertEqual(violation.originTable, "child") + XCTAssertEqual(violation.destinationTable, "parent") + } else { + XCTFail("Missing violation") + } + } + + do { + _ = try db.checkForeignKeys(in: "child") + XCTFail("Expected Error") + } catch let error as DatabaseError { + XCTAssertEqual(DatabaseError.SQLITE_CONSTRAINT_FOREIGNKEY, error.extendedResultCode) + } + } + } + + func testForeignKeyViolationsInUnspecifiedSchemaFindsAttachedDatabase() throws { + let attached = try makeDatabaseQueue(filename: "attached1") + try attached.inDatabase { db in + try db.execute(sql: """ + CREATE TABLE parent(id TEXT NOT NULL PRIMARY KEY); + CREATE TABLE child(id INTEGER NOT NULL PRIMARY KEY, parentId TEXT REFERENCES parent(id)); + PRAGMA foreign_keys = OFF; + INSERT INTO child (id, parentId) VALUES (20, '1'); + """) + } + let main = try makeDatabaseQueue(filename: "main") + try main.inDatabase { db in + try db.execute(literal: "ATTACH DATABASE \(attached.path) AS attached") + + do { + let violations = try Array(try db.foreignKeyViolations(in: "child")) + XCTAssertEqual(violations.count, 1) + if let violation = violations.first(where: { $0.originRowID == 20 }) { + XCTAssertEqual(violation.originTable, "child") + XCTAssertEqual(violation.destinationTable, "parent") + } else { + XCTFail("Missing violation") + } + } + + do { + _ = try db.checkForeignKeys(in: "child") + XCTFail("Expected Error") + } catch let error as DatabaseError { + XCTAssertEqual(DatabaseError.SQLITE_CONSTRAINT_FOREIGNKEY, error.extendedResultCode) + } + } + } }