Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Foreign Key constraint is ignored inside transaction on iOS #1356

Closed
yshrsmz opened this issue Jun 7, 2019 · 7 comments
Closed

Foreign Key constraint is ignored inside transaction on iOS #1356

yshrsmz opened this issue Jun 7, 2019 · 7 comments

Comments

@yshrsmz
Copy link
Contributor

yshrsmz commented Jun 7, 2019

https://github.com/yshrsmz/sqldelight-fktest

I have these two tables.

CREATE TABLE author (
    id INTEGER NOT NULL PRIMARY KEY,
    name TEXT NOT NULL,
    birthday TEXT NOT NULL
);

CREATE TABLE book (
    id INTEGER NOT NULL PRIMARY KEY,
    name TEXT NOT NULL,
    author_id INTEGER NOT NULL,
    FOREIGN KEY (author_id) REFERENCES author(id) ON DELETE CASCADE
);

and I'm trying to write a repository class which takes a list of books and insert those in one transaction.

fun save(books:List<Book>) {
	bookQueries.transaction {
		books.forEach { book ->
			bookQueries.insertOrReplace(...)
		}
	}
}

It should work just fine if corresponding author records exist, and it works as intended on Android. On iOS, above code does not respect Foreign Key constraint.

After some investigation I found out that on iOS, foreign keys are not working inside transaction. Below test case illustrates this.

@Test
fun `should fail as FK constraint is not satisfied - transaction`() {
    database.pragmasQueries.foreignKeysOn() // PRAGMA foreign_keys=1;

    val books = database.bookQueries.count().executeAsOne()
    val authors = database.authorQueries.count().executeAsOne()

    assertEquals(0, books)
    assertEquals(0, authors)

    var failed = false

    try {
        database.transaction {
            database.bookQueries.insert(id = book3.id, name = book3.name, author_id = book3.author_id)
        }
    } catch (e: Throwable) {
        println(e)
        failed = true
    }


    val books2 = database.bookQueries.count().executeAsOne()
    println("books count after insertion attempt: $books2")

    if (!failed) {
        fail("should throw exception")
    }
}

This test case passes on Android, and fails on iOS.
And if I call insert statement without transaction, it passes on both platform.

@yshrsmz
Copy link
Contributor Author

yshrsmz commented Jun 7, 2019

btw naively executing database.pragmasQueries.foreignKeysOn() inside transaction does not work.

@kpgalligan
Copy link
Collaborator

Will take a look

@kpgalligan
Copy link
Collaborator

I know what this is. For that pragma, it needs to be on each connection, and the native driver uses 2 connections internally. The "real" solution will need a design discussion. There should be a way to apply that and possibly other pragmas on connect.

Still poking around with this. One test works with a workaround, but the 'transaction' version of the same test does not, and I'm not sure why yet

@Test
    fun `should fail as FK constraint is not satisfied`() {
        database.pragmasQueries.foreignKeysOn()

        database.transaction {
            database.pragmasQueries.foreignKeysOn()
        }

       val books = database.bookQueries.count().executeAsOne()
        val authors = database.authorQueries.count().executeAsOne()
//etc

Adding that second pragma in the transaction forces the second connection to have the pragma called, but the should fail as FK constraint is not satisfied - transaction test still fails for some reason.

@yshrsmz
Copy link
Contributor Author

yshrsmz commented Jun 14, 2019

yes weird.

when I tried to check foreign_keys status with following extension function,
It prints 1 outside of the transaction, and 0 inside the transaction.

fun SqlDriver.printForeignKeysStatus() {
    val cursor = executeQuery(null, "PRAGMA foreign_keys;", 0)
    var status = ""
    while (cursor.next()) {
        status += cursor.getLong(0)
    }
    println("foreign_keys: $status")
    cursor.close()
}

@Test
fun `should fail as FK constraint is not satisfied - transaction`() {
    database.pragmasQueries.foreignKeysOn()
    database.transaction {
        database.pragmasQueries.foreignKeysOn()
    }

    sqlDriver.printForeignKeysStatus()  // foreign_keys: 1

    val books = database.bookQueries.count().executeAsOne()
    val authors = database.authorQueries.count().executeAsOne()

    assertEquals(0, books)
    assertEquals(0, authors)

    var failed = false

    try {
        database.transaction(true) {
            sqlDriver.printForeignKeysStatus() // foreign_keys: 0

            database.bookQueries.insert(id = book3.id, name = book3.name, author_id = book3.author_id)
        }
    } catch (e: Throwable) {
        println(e)
        failed = true
    }


    val books2 = database.bookQueries.count().executeAsOne()
    println("books count after insertion attempt: $books2")

    if (!failed) {
        fail("should throw exception")
    }
}

@kpgalligan
Copy link
Collaborator

Some more info. Future versions of sqliter will have a db config flag for this. I also think I understand why fk pragma wasn't working in transactions: https://stackoverflow.com/a/7360240/227313

@yshrsmz
Copy link
Contributor Author

yshrsmz commented Jul 12, 2019

Confirmed it's working on v1.1.4 with the code below.
Thanks!

// iOS
val dbConfig = DatabaseConfiguration(
        name = "database.db",
        version = Schema.version,
        foreignKeyConstraints = true,
        create = { connection ->
            wrapConnection(connection) { Schema.create(it) }
        },
        upgrade = { connection, oldVersion, newVersion ->
            wrapConnection(connection) { Schema.migrate(it, oldVersion, newVersion) }
        }
    )

val driver = NativeSqlDriver(dbConfig)
// Android
val driver = AndroidSqliteDriver(
        schema = Database.Schema,
        context = context,
        name = "database.db",
        callback = object : AndroidSqliteDriver.Callback(Database.Schema) {
            override fun onConfigure(db: SupportSQLiteDatabase) {
                super.onConfigure(db)
                db.setForeignKeyConstraintsEnabled(true)
            }
        }
    )

@svenjacobs
Copy link
Contributor

This is still a lot of boiler plate code compared to

NativeSqliteDriver(Database.Schema, "database.db")

if I just want to enable foreign keys, for example. I was wondering if the secondary constructor could provide an optional callback where we can hook into the configuration? For example:

constructor(
    schema: SqlDriver.Schema,
    name: String,
    maxReaderConnections: Int = 1,
    onConfiguration: (DatabaseConfiguration) -> DatabaseConfiguration = { it },
) : this(
    configuration = DatabaseConfiguration(
        name = name,
        version = schema.version,
        create = { connection ->
            wrapConnection(connection) { schema.create(it) }
        },
        upgrade = { connection, oldVersion, newVersion ->
            wrapConnection(connection) { schema.migrate(it, oldVersion, newVersion) }
        }
    ).let(onConfiguration),
    maxReaderConnections = maxReaderConnections
)

I can create a PR if this is a desired change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants
@kpgalligan @svenjacobs @yshrsmz and others