-
-
Notifications
You must be signed in to change notification settings - Fork 731
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
deleteAll without filter: FetchedRecordsController's trackChanges not called #156
Comments
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. |
Thanks for the fast answer!
Any more details I could provide? |
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 :'( |
I have tested with this attached file: A fetched records controller is tracking the changes: Here is what gets printed: then, after inserting again 3 records: recordsDidChange is not triggered |
OK Maxime, I can reproduce the bug. Thanks for the report! |
OK, let me know if I can help any further (but I fear this is now beyond my competencies ;) ) |
So far it's beyond mine as well ;-) |
@comaxime OK the bug is found! A fixed release will ship shortly. |
Great, thanks. |
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:
This is what hides deletions from transaction observers.
The fix will be to return SQLITE_IGNORE for SQLITE_DELETE there, and to write robust tests! |
Thanks @groue for the fast action! |
@groue have just tried v0.99.0, I still have the issue ;) |
Looks like I missed something… |
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 This same code was working fine with version 0.97. |
@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. |
@comaxime I'm dazzled. But not defeated yet! |
@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:
Is it possible for you to run your app with commit 000f1d0 ? 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 |
@comaxime Version 0.99.1 has shipped. |
Thanks @groue, indeed I confirm this is fixed :) |
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 :)
The text was updated successfully, but these errors were encountered: