Grdb.swift: deleteAll without filter: FetchedRecordsController's trackChanges not called

[comaxime]

When performing:
Record.deleteAll(db)
the associated FetchedRecordsController does not notify of the changes (trackChanges not triggered)

but when performing:
Record.filter(1 == 1).deleteAll(db)
it does :)

[groue]

Hello!

It looks like you've being bitten by the truncate optimization which GRDB is supposed to handle correctly since v0.85.0. And indeed I can't reproduce your issue: I wonder whether you are using the latest GRDB version.

[comaxime]

Thanks for the fast answer!
Am actually using GRDB.swift v0.98.0 (Xcode 8.2, simulator iPad Air 2 iOS 10.2, fresh install)

databasePool.write { db in try? Record.deleteAll(db) }

Any more details I could provide?

[groue]

OK, looks like you have found a real bug! Good thing you have a workaround until it is fixed.

Yes you can help, because I'm clueless: would you mind sharing some code, so that I can eventually reproduce the problem? Something like:

let controller = try FetchedRecordsController(dbPool, ...) // or dbQueue
controller.track { ... }
try controller.performFetch()

try dbPool.write { db in
    try Record.deleteAll(db)
}

// wait for changes that never come :'(

[comaxime]

I have tested with this attached file:
TestDeleteAll.swift.zip

A fetched records controller is tracking the changes:
1- Insert 3 records, filter(1==1).deleteAll, the fetched records controller prints the changes
2- Insert 3 records, deleteAll, the fetched records controller does not print anything

Here is what gets printed:
Delete all records with filter(1 == 1).deleteAll(db)
RecordsDidChange triggered: number of records = 0

then, after inserting again 3 records:
Delete all records with deleteAll(db)

recordsDidChange is not triggered

[groue]

OK Maxime, I can reproduce the bug. Thanks for the report!

[comaxime]

OK, let me know if I can help any further (but I fear this is now beyond my competencies ;) )

[groue]

So far it's beyond mine as well ;-)

[groue]

@comaxime OK the bug is found! A fixed release will ship shortly.

[comaxime]

Great, thanks.
Where was the bug hiding?

[groue]

The bug lies here in the StatementCompilationObserver type which tells GRDB what is happening to the database.

According to https://www.sqlite.org/lang_delete.html#truncateopt:

When the WHERE is omitted from a DELETE statement and the table being deleted has no triggers, SQLite uses an optimization to erase the entire table content without having to visit each row of the table individually.

This is what hides deletions from transaction observers.

The truncate optimization can also be disabled at runtime using the sqlite3_set_authorizer() interface. If an authorizer callback returns SQLITE_IGNORE for an SQLITE_DELETE action code, then the DELETE operation will proceed but the truncate optimization will be bypassed and rows will be deleted one by one.

The fix will be to return SQLITE_IGNORE for SQLITE_DELETE there, and to write robust tests!

[groue]

@comaxime GRDB v0.99.0 is out (changelog). Thanks for the report!

[comaxime]

Thanks @groue for the fast action!

[comaxime]

@groue have just tried v0.99.0, I still have the issue ;)

[groue]

Looks like I missed something…

[egrimmius]

I don't know if this is an unintended consequence of the fix for this issue, but this new error I just received with version 0.99 seems to be related. This is within DatabaseMigrator's registerMigrationWithDisabledForeignKeyChecks. Here's the output:

INSERT INTO table_New SELECT * FROM oldTable
DROP TABLE "oldTable"
ROLLBACK TRANSACTION
error occurred in grdb: SQLite error 1 with statement ALTER TABLE "table_New" RENAME TO "oldTable": there is already another table or index with this name: oldTable

This same code was working fine with version 0.97.

[groue]

@egrimmius: your issue is unrelated. Please enable SQL tracing, look at the executed SQL, and if something still looks odd to you, open a new issue.

[groue]

@comaxime I'm dazzled. But not defeated yet!

[groue]

@comaxime SQLite is full of traps, but I think I tackled it this time: I can run your test file and get all needed notifications:

Insert 3 records
RecordsDidChange triggered: number of records = 3

Delete all records with filter(1 == 1).deleteAll(db)
RecordsDidChange triggered: number of records = 0

Insert 3 records
RecordsDidChange triggered: number of records = 3

Delete all records with deleteAll(db)
RecordsDidChange triggered: number of records = 0

Is it possible for you to run your app with commit https://github.com/groue/GRDB.swift/commit/000f1d02c3bf7ed8c46fa25c153d67ba3d9b3738 ? This would prevent me from shipping a half-baked release this time. If not, I'll release a 0.99.1 version, hoping for the best.

When testing, make sure that the fetched records controller is kept alive: as soon as it is deallocated, it stops observing transactions, and may miss the last database update. So keep your instance of TestDeleteAll safe somewhere.

[groue]

@comaxime Version 0.99.1 has shipped.

[comaxime]

Thanks @groue, indeed I confirm this is fixed :)