findmypast-oss/mssqlex

Support for next version of DBConnection once released

shdblowers opened this issue · 3 comments

Information on what needs to be done from @fishcakez via slack:

fishcakez
[10:28 PM]
elixir-ecto/postgrex#321 … is PR for postgrex
GitHub
Update to latest DBConnection by fishcakez · Pull Request #321 · elixir-ecto/postgrex
WIP: awaiting DBConnection release Adds support for non-stream cursors Adds support for advance transaction handling

[10:30]
IIRC neither supports cursors? So dont need to change cursors/streams

[10:33]
For handle_begin/commit/rollback it is expected to return {:idle | :transaction | :error, state} if the driver can know (based on database responses) that a begin/rollback/commit can not work because of the transaction status. I think :error is only relevant to postgres, so :idle means can not rollback/commit because not in transaction and :transaction means can not begin because in transaction. OFC with savepoints one might been :idle for begin (can not begin savepoint outside transaction). Please also make sure to :disconnect (instead : error) on those callbacks, to be safe.

[10:33]
there is also handle_status/2 callback, which would return {:idle | :transaction | :error, state}. This is for nested transactions so we can check that transaction is open at start and end, because begin/rollback/commit are not called.

Hi @shdblowers if you could look at this shortly, or check that it is feasible that would be very much appreciated. If we need to make changes, would rather do those sooner than later. Ideally it would be possible to get the transaction status as part of each queries response but it doesn't look like that happens.

@fishcakez getting errors when I'm trying out new version of DBConnection because the arity of DBConnection.transaction has changed, opened up potential fix here: elixir-ecto/db_connection#93

Have started work in this branch: dbconnection_update_2