Add adapter level read-only transaction support - #741
Conversation
Sorry, something went wrong.
- 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.
- 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?
Sorry, something went wrong.
There was a problem hiding this comment.
Good point 🤔, let me adjust to this
Sorry, something went wrong.
- 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 --explain flag to mix ecto.query? In this case we don't have anything to inspect, we just print the results on terminal. |
Sorry, something went wrong.
|
💚 💙 💜 💛 ❤️ |
Sorry, something went wrong.
|
Looks great to me too, sorry about the lag |
Sorry, something went wrong.
This sounds good! |
Sorry, something went wrong.
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.query use the shared transaction option instead of adapter-specific transaction SQL itself.
Closes #736.