findmypast-oss / mssqlex

Microsoft SQL Server Adapter for Elixir
Apache License 2.0
41 stars 22 forks source link

Support for next version of DBConnection once released #15

Open shdblowers opened 7 years ago

shdblowers commented 7 years ago

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

fishcakez [10:28 PM] https://github.com/elixir-ecto/postgrex/pull/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.

fishcakez commented 7 years ago

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.

shdblowers commented 7 years ago

@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: https://github.com/elixir-ecto/db_connection/pull/93

shdblowers commented 7 years ago

Have started work in this branch: dbconnection_update_2