Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support multiple queries for clickhouse using up/down-sections #589

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dsagoyaneleven
Copy link

@dsagoyaneleven dsagoyaneleven commented Nov 7, 2024

Currently, ClickHouse does not support executing multiple queries. However with these changes, it becomes possible.
We just put each separate up- or down-query in sections. The sections are optional.

- specially prepared for clickhouse, but might work with any other drivers if needed
@amacneil
Copy link
Owner

This seems like a nice solution that avoids full sql parsing.

Imo we could avoid the separate -- migrate:up-section and just allow multiple -- migrate:up blocks?

Curious to hear what others think.

@dossy
Copy link
Collaborator

dossy commented Nov 14, 2024

Imo we could avoid the separate -- migrate:up-section and just allow multiple -- migrate:up blocks?

+1 on allowing multiple -- migrate:up (and, similarly, -- migrate:down) blocks.

@dsagoyaneleven
Copy link
Author

Imo we could avoid the separate -- migrate:up-section and just allow multiple -- migrate:up blocks?

This might work also.

Maybe we need just keep in mind this detail:

That we have -- migrate:up and -- migrate:down and we can set migration options pro up or pro down.
So the idea was that we define one up block (for example) with a specific migration option, let's say transaction=true, and run multiple -- migrate:up-section queries, if one fails, then you can do rollback.

You can also have multiple -- migrate:up but then it's not very clear how to apply migration options, should every -- migrate:up has it's own, or repeat if it's more than one (same question for --migrate:down).

So it's just a question of implementation for sure.

@dossy
Copy link
Collaborator

dossy commented Nov 16, 2024

You can also have multiple -- migrate:up but then it's not very clear how to apply migration options, should every -- migrate:up has it's own, or repeat if it's more than one (same question for --migrate:down).

Personally, I would expect that migration options specified on a -- migrate:up section only applies to that single section.

@thecodedrift
Copy link

thecodedrift commented Dec 10, 2024

Curious to hear what others think.

Chiming in as a current goose user who couldn't use dbmate earlier because of the multiple clickhouse queries issue.

Multiple --migrate:up and --migrate:down blocks, each with their own settings, is perfectly reasonable. It's also IMO the simplest to explain: you can break multiple statements up by adding additional migrate directives to your file. It's your responsibility to manage a partial migration in this situation, and only recommended when it's unreasonable or impossible to do multiple statements within a single up or down directive.

For me, It turns seven clickhouse migrations into a single file and has zero impact on the postgres migration files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants