Skip to content

Commit

Permalink
Merge pull request #709 from groue/dev/migration
Browse files Browse the repository at this point in the history 
Simplify DatabaseMigrator API
  • Loading branch information
@groue
groue authored Feb 27, 2020
2 parents fefeb1e + ba02ba4 commit be53b0d
Show file tree
Hide file tree
Showing 5 changed files with 132 additions and 91 deletions.
29 changes: 28 additions & 1 deletion CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,37 @@ GRDB adheres to [Semantic Versioning](https://semver.org/), with one exception:

## Next Release

**New**
### New

- [#706](https://github.com/groue/GRDB.swift/pull/706): Enhance SQLLiteral and SQL interpolation again

### Breaking Change

- [#709](https://github.com/groue/GRDB.swift/pull/709): Simplify DatabaseMigrator API

Database migrations have a new behavior which is a breaking change. However, it is very unlikely to impact your application.

In previous versions of GRDB, a foreign key violation would immediately prevent a migration from successfully complete. Now, foreign key checks are deferred until the end of each migration. This means that some migrations will change their behavior:

```swift
// Used to fail, now succeeds
migrator.registerMigration(...) { db in
try violateForeignKeyConstraint(db)
try fixForeignKeyConstraint(db)
}

// The catch clause is no longer called
migrator.registerMigration(...) { db in
do {
try performChanges(db)
} catch let error as DatabaseError where error.resultCode == .SQL_CONSTRAINT {
// Handle foreign key error
}
}
```

If your application happens to define migrations that are impacted by this change, please open an issue so that we find a way to restore the previous behavior.


## 4.10.0

Expand Down
51 changes: 6 additions & 45 deletions GRDB/Migration/DatabaseMigrator.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,66 +91,27 @@ public struct DatabaseMigrator {
registerMigration(Migration(identifier: identifier, migrate: migrate))
}

#if GRDBCUSTOMSQLITE || GRDBCIPHER
/// Registers an advanced migration, as described at https://www.sqlite.org/lang_altertable.html#otheralter
///
/// // Add a NOT NULL constraint on players.name:
/// migrator.registerMigrationWithDeferredForeignKeyCheck("AddNotNullCheckOnName") { db in
/// try db.create(table: "new_player") { t in
/// t.autoIncrementedPrimaryKey("id")
/// t.column("name", .text).notNull()
/// }
/// try db.execute(sql: "INSERT INTO new_player SELECT * FROM player")
/// try db.drop(table: "player")
/// try db.rename(table: "new_player", to: "player")
/// }
///
/// While your migration code runs with disabled foreign key checks, those
/// are re-enabled and checked at the end of the migration, regardless of
/// eventual errors.
///
/// - parameters:
/// - identifier: The migration identifier.
/// - block: The migration block that performs SQL statements.
/// - precondition: No migration with the same same as already been registered.
///
/// :nodoc:
public mutating func registerMigrationWithDeferredForeignKeyCheck(
_ identifier: String,
migrate: @escaping (Database) throws -> Void)
{
registerMigration(Migration(identifier: identifier, disabledForeignKeyChecks: true, migrate: migrate))
}
#else
@available(OSX 10.10, *)
/// Registers an advanced migration, as described at https://www.sqlite.org/lang_altertable.html#otheralter
/// Registers a migration.
///
/// // Add a NOT NULL constraint on players.name:
/// migrator.registerMigrationWithDeferredForeignKeyCheck("AddNotNullCheckOnName") { db in
/// try db.create(table: "new_player") { t in
/// migrator.registerMigrationWithDeferredForeignKeyCheck("createAuthors") { db in
/// try db.create(table: "author") { t in
/// t.autoIncrementedPrimaryKey("id")
/// t.column("creationDate", .datetime)
/// t.column("name", .text).notNull()
/// }
/// try db.execute(sql: "INSERT INTO new_player SELECT * FROM player")
/// try db.drop(table: "player")
/// try db.rename(table: "new_player", to: "player")
/// }
///
/// While your migration code runs with disabled foreign key checks, those
/// are re-enabled and checked at the end of the migration, regardless of
/// eventual errors.
///
/// - parameters:
/// - identifier: The migration identifier.
/// - block: The migration block that performs SQL statements.
/// - precondition: No migration with the same same as already been registered.
@available(*, deprecated, renamed: "registerMigration(_:migrate:)")
public mutating func registerMigrationWithDeferredForeignKeyCheck(
_ identifier: String,
migrate: @escaping (Database) throws -> Void)
{
registerMigration(Migration(identifier: identifier, disabledForeignKeyChecks: true, migrate: migrate))
registerMigration(identifier, migrate: migrate)
}
#endif

/// Iterate migrations in the same order as they were registered. If a
/// migration has not yet been applied, its block is executed in
Expand Down
62 changes: 31 additions & 31 deletions GRDB/Migration/Migration.swift
View file
Original file line number Diff line number Diff line change
@@ -1,36 +1,27 @@
#if SWIFT_PACKAGE
import CSQLite
#elseif GRDBCIPHER
import SQLCipher
#elseif !GRDBCUSTOMSQLITE && !GRDBCIPHER
import SQLite3
#endif

/// An internal struct that defines a migration.
struct Migration {
let identifier: String
let disabledForeignKeyChecks: Bool
let migrate: (Database) throws -> Void

#if GRDBCUSTOMSQLITE || GRDBCIPHER
init(identifier: String, disabledForeignKeyChecks: Bool = false, migrate: @escaping (Database) throws -> Void) {
self.identifier = identifier
self.disabledForeignKeyChecks = disabledForeignKeyChecks
self.migrate = migrate
}
#else
init(identifier: String, migrate: @escaping (Database) throws -> Void) {
self.identifier = identifier
self.disabledForeignKeyChecks = false
self.migrate = migrate
}

@available(OSX 10.10, *)
// PRAGMA foreign_key_check was introduced in SQLite 3.7.16 http://www.sqlite.org/changes.html#version_3_7_16
// It is available from iOS 8.2 and OS X 10.10
// https://github.com/yapstudios/YapDatabase/wiki/SQLite-version-(bundled-with-OS)
init(identifier: String, disabledForeignKeyChecks: Bool, migrate: @escaping (Database) throws -> Void) {
self.identifier = identifier
self.disabledForeignKeyChecks = disabledForeignKeyChecks
self.migrate = migrate
}
#endif

func run(_ db: Database) throws {
if try disabledForeignKeyChecks && (Bool.fetchOne(db, sql: "PRAGMA foreign_keys") ?? false) {
try runWithDisabledForeignKeys(db)
// PRAGMA foreign_key_check and SQLITE_CONSTRAINT_FOREIGNKEY were
// introduced in SQLite 3.7.16
// http://www.sqlite.org/changes.html#version_3_7_16
if try sqlite3_libversion_number() >= 3007016 && (Bool.fetchOne(db, sql: "PRAGMA foreign_keys") ?? false) {
try runWithDeferredForeignKeysChecks(db)
} else {
try runWithoutDisabledForeignKeys(db)
}
Expand All @@ -45,7 +36,7 @@ struct Migration {
}
}

private func runWithDisabledForeignKeys(_ db: Database) throws {
private func runWithDeferredForeignKeysChecks(_ db: Database) throws {
// Support for database alterations described at
// https://www.sqlite.org/lang_altertable.html#otheralter
//
Expand All @@ -60,26 +51,35 @@ struct Migration {
try migrate(db)
try insertAppliedIdentifier(db)

// > 10. If foreign key constraints were originally enabled then run PRAGMA
// > foreign_key_check to verify that the schema change did not break any foreign key
// > constraints.
if try Row.fetchOne(db, sql: "PRAGMA foreign_key_check") != nil {
// > 10. If foreign key constraints were originally enabled
// > then run PRAGMA foreign_key_check to verify that the
// > schema change did not break any foreign
// > key constraints.
if try db
.makeSelectStatement(sql: "PRAGMA foreign_key_check")
.makeCursor()
.isEmpty() == false
{
// https://www.sqlite.org/pragma.html#pragma_foreign_key_check
//
// PRAGMA foreign_key_check does not return an error,
// but the list of violated foreign key constraints.
//
// Let's turn any violation into an SQLITE_CONSTRAINT
// error, and rollback the transaction.
throw DatabaseError(resultCode: .SQLITE_CONSTRAINT, message: "FOREIGN KEY constraint failed")
// Let's turn any violation into an
// SQLITE_CONSTRAINT_FOREIGNKEY error, and rollback
// the transaction.
throw DatabaseError(
resultCode: .SQLITE_CONSTRAINT_FOREIGNKEY,
message: "FOREIGN KEY constraint failed")
}

// > 11. Commit the transaction started in step 2.
return .commit
}
},
finally: {
// > 12. If foreign keys constraints were originally enabled, reenable them now.
// > 12. If foreign keys constraints were originally enabled,
// > reenable them now.
try db.execute(sql: "PRAGMA foreign_keys = ON")
})
}
Expand Down
10 changes: 6 additions & 4 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4921,6 +4921,8 @@ migrator.registerMigration("v2") { db in

**Each migration runs in a separate transaction.** Should one throw an error, its transaction is rollbacked, subsequent migrations do not run, and the error is eventually thrown by `migrator.migrate(dbQueue)`.

**Migrations run with deferred foreign key checks,** starting SQLite 3.7.16+ (iOS 9.0+ / macOS 10.10+ / tvOS 9.0+ / watchOS 2.0+ / [custom SQLite build] / [SQLCipher](#encryption)). This means that eventual foreign key violations are only checked at the end of the migration (and they make the migration fail).

**The memory of applied migrations is stored in the database itself** (in a reserved table).

You migrate the database up to the latest version with the `migrate(_:)` method:
Expand Down Expand Up @@ -4986,11 +4988,11 @@ The `eraseDatabaseOnSchemaChange` option triggers a recreation of the database i

SQLite does not support many schema changes, and won't let you drop a table column with "ALTER TABLE ... DROP COLUMN ...", for example.

Yet any kind of schema change is still possible. The SQLite documentation explains in detail how to do so: https://www.sqlite.org/lang_altertable.html#otheralter. This technique requires the temporary disabling of foreign key checks, and is supported by the `registerMigrationWithDeferredForeignKeyCheck` function:
Yet any kind of schema change is still possible, by recreating tables:

```swift
// Add a NOT NULL constraint on player.name:
migrator.registerMigrationWithDeferredForeignKeyCheck("AddNotNullCheckOnName") { db in
migrator.registerMigration("AddNotNullCheckOnName") { db in
// Add a NOT NULL constraint on player.name:
try db.create(table: "new_player") { t in
t.autoIncrementedPrimaryKey("id")
t.column("name", .text).notNull()
Expand All @@ -5001,7 +5003,7 @@ migrator.registerMigrationWithDeferredForeignKeyCheck("AddNotNullCheckOnName") {
}
```

While your migration code runs with disabled foreign key checks, those are re-enabled and checked at the end of the migration, regardless of eventual errors.
> :point_up: **Note**: This technique requires SQLite 3.7.16+ (iOS 9.0+ / macOS 10.10+ / tvOS 9.0+ / watchOS 2.0+ / [custom SQLite build] / [SQLCipher](#encryption)).


## Joined Queries Support
Expand Down
71 changes: 61 additions & 10 deletions Tests/GRDBTests/DatabaseMigratorTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,13 +141,8 @@ class DatabaseMigratorTests : GRDBTestCase {
}
migrator.registerMigration("foreignKeyError") { db in
try db.execute(sql: "INSERT INTO persons (name) VALUES ('Barbara')")
do {
// triggers immediate foreign key error:
try db.execute(sql: "INSERT INTO pets (masterId, name) VALUES (?, ?)", arguments: [123, "Bobby"])
XCTFail("Expected error")
} catch {
throw error
}
// triggers foreign key error:
try db.execute(sql: "INSERT INTO pets (masterId, name) VALUES (?, ?)", arguments: [123, "Bobby"])
}

let dbQueue = try makeDatabaseQueue()
Expand All @@ -163,8 +158,6 @@ class DatabaseMigratorTests : GRDBTestCase {
XCTAssert((error.resultCode == error.extendedResultCode) || error.extendedResultCode == .SQLITE_CONSTRAINT_FOREIGNKEY)
XCTAssertEqual(error.resultCode, .SQLITE_CONSTRAINT)
XCTAssertEqual(error.message!.lowercased(), "foreign key constraint failed") // lowercased: accept multiple SQLite version
XCTAssertEqual(error.sql!, "INSERT INTO pets (masterId, name) VALUES (?, ?)")
XCTAssertEqual(error.description.lowercased(), "sqlite error 19 with statement `insert into pets (masterid, name) values (?, ?)` arguments [123, \"bobby\"]: foreign key constraint failed")

let names = try dbQueue.inDatabase { db in
try String.fetchAll(db, sql: "SELECT name FROM persons")
Expand All @@ -173,7 +166,65 @@ class DatabaseMigratorTests : GRDBTestCase {
}
}

func testMigrationWithoutForeignKeyChecks() throws {
func testForeignKeyViolation() throws {
#if !GRDBCUSTOMSQLITE && !GRDBCIPHER
guard #available(iOS 8.2, OSX 10.10, *) else {
return
}
#endif
var migrator = DatabaseMigrator()
migrator.registerMigration("createPersons") { db in
try db.execute(sql: "CREATE TABLE persons (id INTEGER PRIMARY KEY, name TEXT, tmp TEXT)")
try db.execute(sql: "CREATE TABLE pets (masterId INTEGER NOT NULL REFERENCES persons(id), name TEXT)")
try db.execute(sql: "INSERT INTO persons (name) VALUES ('Arthur')")
let personId = db.lastInsertedRowID
try db.execute(sql: "INSERT INTO pets (masterId, name) VALUES (?, 'Bobby')", arguments:[personId])
}
migrator.registerMigration("removePersonTmpColumn") { db in
// Test the technique described at https://www.sqlite.org/lang_altertable.html#otheralter
try db.execute(sql: "CREATE TABLE new_persons (id INTEGER PRIMARY KEY, name TEXT)")
try db.execute(sql: "INSERT INTO new_persons SELECT id, name FROM persons")
try db.execute(sql: "DROP TABLE persons")
try db.execute(sql: "ALTER TABLE new_persons RENAME TO persons")
}
migrator.registerMigration("foreignKeyError") { db in
try db.execute(sql: "INSERT INTO persons (name) VALUES ('Barbara')")
// triggers foreign key error:
try db.execute(sql: "INSERT INTO pets (masterId, name) VALUES (?, ?)", arguments: [123, "Bobby"])
}

let dbQueue = try makeDatabaseQueue()
do {
try migrator.migrate(dbQueue)
XCTFail("Expected error")
} catch let error as DatabaseError {
// Migration 1 and 2 should be committed.
// Migration 3 should not be committed.

XCTAssert((error.resultCode == error.extendedResultCode) || error.extendedResultCode == .SQLITE_CONSTRAINT_FOREIGNKEY)
XCTAssertEqual(error.resultCode, .SQLITE_CONSTRAINT)
XCTAssertEqual(error.message!.lowercased(), "foreign key constraint failed") // lowercased: accept multiple SQLite version

try dbQueue.inDatabase { db in
// Arthur inserted (migration 1), Barbara (migration 3) not inserted.
var rows = try Row.fetchAll(db, sql: "SELECT * FROM persons")
XCTAssertEqual(rows.count, 1)
var row = rows.first!
XCTAssertEqual(row["name"] as String, "Arthur")

// persons table has no "tmp" column (migration 2)
XCTAssertEqual(Array(row.columnNames), ["id", "name"])

// Bobby inserted (migration 1), not deleted by migration 2.
rows = try Row.fetchAll(db, sql: "SELECT * FROM pets")
XCTAssertEqual(rows.count, 1)
row = rows.first!
XCTAssertEqual(row["name"] as String, "Bobby")
}
}
}

func testMigrationWithDeferredForeignKeyChecksDeprecated() throws {
#if !GRDBCUSTOMSQLITE && !GRDBCIPHER
guard #available(iOS 8.2, OSX 10.10, *) else {
return
Expand Down

0 comments on commit be53b0d

Please sign in to comment.