Execute single-statement migrations without batch_execute
50U10FCA7 opened this issue · 6 comments
Setup
Versions
- Rust: 1.76.0
- Diesel: 1.4.1
- Database: CockroachDB v23.2 (postgres compatible)
- Operating System: MacOS
Feature Flags
- diesel: postgres
Problem Description
Single-statement migrations executing as batch_execute which makes some statements result with error.
SET CLUSTER SETTING CockroachDB-specific statement requires to be run as single statement (not batch).
Executing single statement diesel migrations will fail anyway because of batch_execute.
What are you trying to accomplish?
Setup cluster settings with diesel migrations.
What is the expected output?
Migrations run successfully.
What is the actual output?
Executing migration script migrations/cockroachdb/schema/00000000000008_read_committed_isolation/up.sql
Failed with: SET CLUSTER SETTING cannot be used inside a multi-statement transaction
make[1]: *** [migration.cockroachdb] Error 1
make: *** [migration.cockroachdb] Error 2
Are you seeing any additional errors?
Nope.
Steps to reproduce
-
Start a docker container with CockroachDB v23.2 (https://hub.docker.com/r/cockroachdb/cockroach)
-
Setup diesel migration:
my_migrations/
00000000000000_my_migration/
up.sql
down.sql
up.sql
SET CLUSTER SETTING sql.txn.read_committed_isolation.enabled = 'true';down.sql
SET CLUSTER SETTING sql.txn.read_committed_isolation.enabled = 'false';- Run diesel migration.
diesel migration --migration-dir="my_migrations" run
Checklist
- I have already looked over the issue tracker and the discussion forum for similar possible closed issues.
- This issue can be reproduced on Rust's stable channel. (Your issue will be
closed if this is not the case) - This issue can be reproduced without requiring a third party crate
Thanks for filling this PR. Can you clarify if that's a CockroachDB specific feature or if that's something that could happen with other backends as well? In the first case it's just a matter of using a not officially supported database which does not support all required features, which is something that should be fixed in that database system or by providing a third party backend/connection implementation.
@weiznich SET CLUSTER SETTING is CockroachDB specific feature, but the case of executing a single statement as batch in diesel migrations is misleading imho.
Well, as a migration can and will contain several statements in general it's the only meaningful solution to use batch_execute for this, as otherwise we would need to have a full sql parsers to detect if a migration is a single or multiple statements. That's clearly out of scope for this project, especially if that is related to an feature of a not supported backend.
@weiznich The problem is also related to supported backends I think:
For example if I turn off multi-statement execution in MySQL (MYSQL_OPTION_MULTI_STATEMENTS_OFF) and try to run migrations contains single statements only I will get an error.
as otherwise we would need to have a full sql parsers to detect if a migration is a single or multiple statements.
Multiple statements separates with ; in most cases, isn't it enough to detect multi-statement queries?
p.s. Or maybe just run a query as single-statement, and if error occurs - run as batch?
Multiple statements separates with ; in most cases, isn't it enough to detect multi-statement queries?
That will not work for queries like UPDATE some_table SET foo = ';'
For example if I turn off multi-statement execution in MySQL (MYSQL_OPTION_MULTI_STATEMENTS_OFF) and try to run migrations contains single statements only I will get an error.
Given that this is not the default I do not see that as critical problem. We likely would accept a PR that improves the handling of such situations by just enforcing that this option is not set.