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.

Sign up for GitHub

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

Jump to bottom

Fix a bunch of SQLite-related bugs #2866

Merged
merged 8 commits into from
Oct 10, 2024
Merged

Fix a bunch of SQLite-related bugs #2866

merged 8 commits into from
Oct 10, 2024
+576 −74

Commits on Oct 10, 2024

  1. Fix SQLITE_MISUSE bug when reusing a prepared statement that previous… 

    …ly threw.

`Query`'s destructor didn't run in this case, so the statement was never reset, and trying to use it again would fail with SQLITE_MISUSE.
    @kentonv
    kentonv committed Oct 10, 2024
    Configuration menu
    Copy the full SHA
    d9ee8c3 View commit details
    Browse the repository at this point in the history

  2. Cleanup: Merge SQLite resetRowCounters() into destroy(). 

    We are doing other per-statement cleanup in `destroy()`, makes sense to reset row counters here too, rather than on the next query constructor.
    @kentonv
    kentonv committed Oct 10, 2024
    Configuration menu
    Copy the full SHA
    b74eb80 View commit details
    Browse the repository at this point in the history

  3. SQLite: Refactor: prepareSql() returns StatementAndEffect. 

    This commit has no functional change, but changes prepareSql() to return a struct containing the statement and other information which must be kept with the statement. At present there is no other information, but the next commit will add a description of the statement's effects on the transaction stack.

I split this into a separate commit because it's a noisy mechanical change, which I wanted to keep separate from the actual logic change.
    @kentonv
    kentonv committed Oct 10, 2024
    Configuration menu
    Copy the full SHA
    78b9d98 View commit details
    Browse the repository at this point in the history

  4. SQLite: Add SqliteDatabase::onRollback(). 

    This registers a callback which will be called if the current transaction scope ends up being rolled back. We'll need this to fix bugs elsewhere.

The implementation requires that we keep track of the transaction stack, which is a bit of a pain. We can discover which statements affect the transaction stack via the authorizor callback.
    @kentonv
    kentonv committed Oct 10, 2024
    Configuration menu
    Copy the full SHA
    4e349eb View commit details
    Browse the repository at this point in the history

  5. SQLite: Fix bug when creation of _cf_KV table is rolled back. 

    The state of the `SqliteKv` object could get out-of-sync in the case of rollbacks. Our new `onRollback()` function makes this easy to deal with.
    @kentonv
    kentonv committed Oct 10, 2024
    Configuration menu
    Copy the full SHA
    f8ae843 View commit details
    Browse the repository at this point in the history

  6. SQLite: Fix bug when creation of _cf_METADATA table is rolled back. 

    Also if cached alarm changes are rolled back.

This replaces the previous invalidation mechanism, which did not properly handle `transactionSync()`. The new mechanism handles all rollbacks, so we don't need the old mechanism at all anymore.
    @kentonv
    kentonv committed Oct 10, 2024
    Configuration menu
    Copy the full SHA
    8da4f7b View commit details
    Browse the repository at this point in the history

  7. SQLite: Throw better exception message when people use BEGIN TRANSACT… 

    …ION.

People mistakenly believed that we didn't support transactions!
    @kentonv
    kentonv committed Oct 10, 2024
    Configuration menu
    Copy the full SHA
    c86b551 View commit details
    Browse the repository at this point in the history

  8. SQLite: Fix missing RELEASE of savepoinst in transactionSync(). 

    `ROLLBACK TO` does not release the savepoint, just rolls back to the point immediately after the savepoint was created. We must additionally release it.

This never caused a problem becaues:
1. `transactionSync()` always runs inside an implicit transaction. Committing or rolling back the implicit transaction implicitly releases all the savepoints.
2. It turns out SQLite allows savepoints with the same name as previous ones. The name always refers to the most-recent savepoint by that name which hasn't been released. So, the "leaked" savepoint didn't prevent a new savepoint with the same name from being created later.

Note that the async version of `transaction()` does release savepoints properly.

I also added a TODO to take advantage of the fact that savepoints can have the same name...
    @kentonv
    kentonv committed Oct 10, 2024
    Configuration menu
    Copy the full SHA
    d34205c View commit details
    Browse the repository at this point in the history