Skip to content
This repository was archived by the owner on Mar 19, 2021. It is now read-only.

with_transaction #2 #99

Merged
merged 13 commits into from
Aug 11, 2020
Merged

with_transaction #2 #99

merged 13 commits into from
Aug 11, 2020

Conversation

dominicletz
Copy link
Contributor

On the original pull request there is unfortunately not much traction. But here is the grown version that I'm using in the mean time since. It has a couple of improvements on top of the initial with_transaction pull request.

  • Server.with_transaction() becomes reentrancy safe. Instead of passing a pure Sqlitex db instance, it is passing it's own pid as parameters, and it's safe to call all Server.* methods on it without getting stuck in a GenServer self-call

  • This allows faster query() within with_transaction() calls because the StatementCache of the Sqlitex.Server instance is now used.

BEFORE
      iex> Sqlitex.Server.with_transaction(server, fn(db) ->
      ...>   Sqlitex.exec(db, "create table foo(id integer)")
      ...>   Sqlitex.exec(db, "insert into foo (id) values(42)")
      ...> end)
NOW
      iex> Sqlitex.Server.with_transaction(server, fn(pid) ->
      ...>   Sqlitex.Server.query(pid, "create table foo(id integer)")
      ...>   Sqlitex.Server.query(pid, "insert into foo (id) values(42)")
      ...> end)
  • This also allows calling Server.with_transaction() within a transaction -- double wrapping of the statement is avoided. So you can write small functions that execute transactions but reuse them in other transaction creating functions.
      iex> Sqlitex.Server.with_transaction(server, fn(pid) ->
      ...>   Sqlitex.Server.with_transaction(pid, fn(pid2) ->
      ...>     Sqlitex.Server.query(pid2, "create table foo(id integer)")
      ...>     Sqlitex.Server.query(pid2, "insert into foo (id) values(42)")
      ...>   end)
      ...> end)
  • Errors in SQL statements within the Servers with_transaction are rescued as before but then rethrown in the context of the caller for easier debugging and locating of errors.

@ConnorRigby ConnorRigby self-requested a review August 11, 2020 22:50
@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 93.45% when pulling 6e79d79 on diodechain:with_tx into 1d47a6b on elixir-sqlite:master.

Copy link
Member

@ConnorRigby ConnorRigby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A very much welcome addition. Thanks for the hard work!

@dominicletz
Copy link
Contributor Author

Ha, so the nicer debug output actually only compiles with Elixir > 1.7 because of __STACKTRACE__ not sure what to do about that. Is there away to have a compatibility switch?

@ConnorRigby
Copy link
Member

This fails on CI for elixir 1.6 and older. I don't really mind breaking incompatibility with those versions, since i think this will be an API breaking change anyway. When this is released, i will remove CI and make note of the breakage in the changelog.

@ConnorRigby ConnorRigby merged commit dd414b1 into elixir-sqlite:master Aug 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants