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

sqlite improvements #1965

Merged
merged 1 commit into from Jul 12, 2022
Merged

sqlite improvements #1965

merged 1 commit into from Jul 12, 2022

Conversation

abonander
Copy link
Collaborator

@abonander abonander commented Jul 12, 2022

  • use direct blocking calls for SQLite in sqlx_macros
    • this also ensures the database is closed properly, cleaning up tempfiles
  • don't send PRAGMA journal_mode unless set
    • this previously defaulted to WAL mode which is a permanent setting
      on databases which doesn't necessarily apply to all use-cases
    • changing into or out of WAL mode acquires an exclusive lock on the database
      that can't be waited on by sqlite3_busy_timeout()
    • for consistency, sqlx-cli commands that create databases will still
      create SQLite databases in WAL mode; added a flag to disable this.
  • in general, don't send PRAGMAs unless different than default
    • we were sending a bunch of PRAGMAs with their default values just to enforce
      an execution order on them, but we can also do this by inserting empty slots
      for their keys into the IndexMap
  • add error code to SqliteError printout
  • document why u64 is not supported (this comes up from time to time so might as well fix it now)

@LovecraftianHorror I believe these changes are all backwards-compatible but I wouldn't mind some more eyes on it.

@ipetkov I can no longer repro #1929 with the changes from #1930 and this, although most of the credit goes to #1930

closes #1374

@ipetkov
Copy link
Contributor

ipetkov commented Jul 12, 2022

I can confirm that 4d49d66 fixes the original issue I had in my build 🎉

@abonander abonander force-pushed the ab/sqlite-refactors branch 2 times, most recently from 5eb4766 to b3b6fc2 Compare July 12, 2022 01:33
* use direct blocking calls for SQLite in `sqlx_macros`
    * this also ensures the database is closed properly, cleaning up tempfiles
* don't send `PRAGMA journal_mode` unless set
    * this previously defaulted to WAL mode which is a permanent setting
      on databases which doesn't necessarily apply to all use-cases
    * changing into or out of WAL mode acquires an exclusive lock on the database
      that can't be waited on by `sqlite3_busy_timeout()`
    * for consistency, `sqlx-cli` commands that create databases will still
      create SQLite databases in WAL mode; added a flag to disable this.
* in general, don't send `PRAGMA`s unless different than default
    * we were sending a bunch of `PRAGMA`s with their default values just to enforce
      an execution order on them, but we can also do this by inserting empty slots
      for their keys into the `IndexMap`
* add error code to `SqliteError` printout
* document why `u64` is not supported
@abonander abonander merged commit bc3e705 into main Jul 12, 2022
@abonander abonander deleted the ab/sqlite-refactors branch July 12, 2022 20:59
@penberg
Copy link
Contributor

penberg commented Aug 12, 2022

@abonander So how does one now set WAL mode for SQLite when using AnyPoolOptions? I could execute PRAGMA journal_mode=WAL; in after_connect() hook, but that seems bit clumsy.

@abonander
Copy link
Collaborator Author

If the database already exists it will remember WAL mode and sqlx db create will create the database in WAL mode by default.

To explicitly set WAL mode, see https://docs.rs/sqlx/latest/sqlx/sqlite/struct.SqliteConnectOptions.html#method.journal_mode

@penberg
Copy link
Contributor

penberg commented Aug 12, 2022

@abonander Right, but the problem I have is that I am using AnyPoolOptions, not the SQLite-specific API in sqlx. The after_connect thing seems to work, though.

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

Successfully merging this pull request may close these issues.

Docs claim that u64 is a valid SQLite Encode/Decode type but it is then unimplemented.
3 participants