groue / GRDB.swift

A toolkit for SQLite databases, with a focus on application development
MIT License
6.78k stars 702 forks source link

GRDB does not seem to copy over all rows when inserting within a migration #1530

Closed anlaital-oura closed 5 months ago

anlaital-oura commented 5 months ago

What did you do?

I am trying to do a migration for a table to fix a primary key issue. Since SQLite does not support altering constraints, I am using a migration written like this to achieve it:

var migrator = DatabaseMigrator()

migrator.registerMigration("some_migration") { db in
    try db.rename(table: "some_table", to: "some_table_old")

    try db.create(table: "some_table") { t in
        // column creation using t.column omitted
    }

    try db.execute(sql: "INSERT INTO some_table SELECT * FROM some_table_old")

    try db.drop(table: "some_table_old")
}

What did you expect to happen?

I expected the SQLite database to contain some_table that has exact same dataset as was present in some_table_old. This is not the case, as only about ~240k rows are inserted out of ~400k.

If I run the same SQL statements using sqlite3 then everything is as expected.

What happened instead?

some_table does not contain all rows from some_table_old.

Environment

GRDB flavor(s): GRDB GRDB version: 6.26.0 Installation method: SPM Xcode version: 15.3 Swift version: 5.10 Platform(s) running GRDB: iOS macOS version running Xcode: 14.4.1

Demo Project

No repro case yet, but I can try to isolate the issue and provide one here later.

groue commented 5 months ago

Hello @anlaital-oura,

This is weird indeed, because well those GRDB statements translate into plain ALTER TABLE, CREATE TABLE, etc, without anything fancy. You can activate tracing to see them:

migrator.registerMigration("some_migration") { db in
    #warning("TODO: remove debugging SQL trace")
    db.trace { print("SQL> \($0)") }

    // Proceed with the rest of the migration
    ...
}

My only concern with your code is that it does not precisely follow the SQLite instructions for performing such changes to the database schema. The GRDB documentation translates it into Swift in the Defining the Database Schema from a Migration chapter.

According to those instructions, I would expect to read instead:

migrator.registerMigration("some_migration") { db in
    // Create a new table with a fixed schema.
    try db.create(table: "new_some_table") { t in
        ...
    }

    // Transfer values.
    // ⚠️ This SQL statement assumes that both tables have the same columns.
    try db.execute(sql: "INSERT INTO new_some_table SELECT * FROM some_table")

    // Replace old table with the new one.
    try db.drop(table: "some_table")
    try db.rename(table: "new_some_table", to: "some_table")
}

This will solve some potential problems (described by the linked SQLite documentation), but I do not know if this will fix the weird dataset changes. I hope this is just a glitch in your testing, or a wrong copy statement, and not an SQLite or GRDB bug.

Please report your findings!

anlaital-oura commented 5 months ago

I modified the code to be exactly what you recommended. It did not help and still only 237,485 rows were inserted. Here are the SQL traces:

SQL> CREATE TABLE "new_ringevent" ("timestamp" INTEGER, "event" BLOB, "type" INTEGER, PRIMARY KEY ("timestamp", "type"))
SQL> INSERT INTO new_ringevent SELECT * FROM ringevent
SQL> DROP TABLE "ringevent"
SQL> ALTER TABLE "new_ringevent" RENAME TO "ringevent"
SQL> INSERT INTO grdb_migrations (identifier) VALUES (?)
SQL> PRAGMA foreign_key_check
SQL> COMMIT TRANSACTION
SQL> PRAGMA foreign_keys = ON
SQL> BEGIN DEFERRED TRANSACTION
SQL> DELETE FROM "ringevent" WHERE "timestamp" < ?
SQL> COMMIT TRANSACTION

I have verified that this happens also when no primary keys constraints are used in the new table. It also seems that the number of rows copied varies a little bit, for example just now I got 237,406 rows inserted. The original data set remained the same at 399,958 rows.

anlaital-oura commented 5 months ago

I am closing the issue as I figured out the root cause. GRDB is working flawlessly (as expected) and we had some incorrect code that was deleting these events accidentally when running against the simulator target. I apologize for wasting your time but still thank you for pointing me to the recommended approach to doing such migrations. 😭

groue commented 5 months ago

😅 Don't worry! You had a good idea opening the issue anyway!