dbmate icon indicating copy to clipboard operation
dbmate copied to clipboard

Having multiple statements in migration throws error on ClickHouse

Open Rio6 opened this issue 3 years ago • 6 comments

I have a simple migration to test on ClickHouse like this

-- migrate:up
select 1;
select 2;


-- migrate:down

Running the above migration throws this error

Applying: 20210708033543_test.sql
Error: code: 62, message: Syntax error (Multi-statements are not allowed): failed at position 23 (end of query) (line 2, col 9): ;
select 2;


.

I'm guess either there's an option needed to set when connecting to the DB, or maybe ClickHouse doesn't support multi-statement query. Perhaps dbmate can split the query up and send them one by one?

Rio6 avatar Jul 08 '21 03:07 Rio6

I am also stacked with this problem. It seems that clickhouse http client has multiquery support https://clickhouse.tech/docs/en/interfaces/cli/#command-line-options but CH go client still doesn't support multiqueries. My issue in clickhouse go client repo is opened without any answers for several month https://github.com/ClickHouse/clickhouse-go/issues/351 Maybe it is possible to add implicit split in dbmate?

upd: I did a little research - it seems that CH multiquery support is made on cli client side (https://github.com/ClickHouse/ClickHouse/blob/master/programs/client/Client.cpp#L1099) and there is no server-side multiquery support. So the only way is to split query on tokens in dbmate or in go driver directly.

ybogo avatar Jul 19 '21 09:07 ybogo

It sounds like the easiest way to solve this is probably to fix it in the ClickHouse go client, since our other drivers support multi statements natively.

It would be possible (but not trivial) to add a SQL parser and send statements one at a time (but not sure that is a good idea).

amacneil avatar Jul 19 '21 16:07 amacneil

Yeah it seems like clickhouse-go is the better place for this as it requires parsing the SQL.

Rio6 avatar Jul 23 '21 22:07 Rio6

Yeah it seems like clickhouse-go is the better place for this as it requires parsing the SQL.

Unfortunately there will be no multiquery support on the clickhouse-go driver side https://github.com/ClickHouse/clickhouse-go/issues/351#issuecomment-1013864963

ybogo avatar Jan 16 '22 12:01 ybogo

In that case, it might be a showstopper. I'm not very excited about adding SQL parsing logic to dbmate, it would be a lot of work to maintain over time, with inconsistencies between database plugins, etc. So unless there is upstream support for multiquery in the sql driver, we probably can't support it in dbmate.

amacneil avatar Jan 18 '22 05:01 amacneil

I have not written a lot of golang but I like the approach taken by golang-migrate https://github.com/golang-migrate/migrate/blob/master/database/clickhouse/clickhouse.go#L139

It simply splits the statements on the ; character.

This is not as robust as adding proper SQL parsing logic but it mentions that in the notes. https://github.com/golang-migrate/migrate/tree/master/database/clickhouse#notes

  • The Clickhouse driver does not natively support executing multiple statements in a single query. To allow for multiple statements in a single migration, you can use the x-multi-statement param. There are two important caveats:
    • This mode splits the migration text into separately-executed statements by a semi-colon ;. Thus x-multi-statement cannot be used when a statement in the migration contains a string with a semi-colon.
    • The queries are not executed in any sort of transaction/batch, meaning you are responsible for fixing partial migrations.

In my case, multiple statement support is a dealbreaker. Would the approach taken by golang-migrate be a good middle ground here?

AntonFriberg avatar Sep 06 '22 08:09 AntonFriberg

Goose uses a special comment for multiple statements that cannot be split with ;:

By default, SQL statements are delimited by semicolons - in fact, query statements must end with a semicolon to be properly recognized by goose.

More complex statements (PL/pgSQL) that have semicolons within them must be annotated with -- +goose StatementBegin and -- +goose StatementEnd to be properly recognized.

https://github.com/pressly/goose#sql-migrations

hkrutzer avatar Nov 28 '22 11:11 hkrutzer