Add adapter level read-only transaction support#741
Conversation
- Add adapter-level read_only_transaction callbacks for SQL adapters - Update mix ecto.query to use the transaction read_only opt instead of adapter-specific transaction SQL.
e221a67 to
c31b6eb
Compare
- Log SET TRANSACTION READ ONLY through the transaction logger - Remove the TDS read_only_transaction callback so Ecto.Adapters.SQL raises the unsupported adapter error
|
|
||
| ## Connection helpers | ||
|
|
||
| defp read_only_transaction(adapter_meta, connection, opts, callback) do |
There was a problem hiding this comment.
Hrm.... what happens if we are already inside a transaction, which is not read-only, and we call this? One option is to detect and raise but that means additional bookkeeping. And because the database themselves are very different when it comes to this functionality, the API is awkward.
I am thinking we don't add this functionality as part of the official repo or transaction API. We just add this exclusively to the adapter and call the adapter directly in mix ecto.query?
There was a problem hiding this comment.
Good point 🤔, let me adjust to this
- Move read-only query execution out of the Repo.transaction API - Call the SQL adapter hook directly from mix ecto.query.
|
Merging, any other remarks can be deal with later! Thank youy @cdvx!!!! Btw, do you think it is also worth adding a |
|
💚 💙 💜 💛 ❤️ |
3 participants
Adds
repo.transaction(read_only: true)support for SQL adapters by moving the read-only transaction handling into adapter-level connection callbacks.This lets
mix ecto.queryuse the shared transaction option instead of adapter-specific transaction SQL itself.Closes #736.