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

clickhouse: Fixed get db version query #240

Merged
merged 1 commit into from
Jun 18, 2022

Conversation

chapsuk
Copy link
Contributor

@chapsuk chapsuk commented Feb 7, 2021

2 bugs with current query:

  • reset command resets only last applied migrations , because LIMIT 1;
  • not deterministic result for migrations applied with same tstamp, because ORDER BY tstamp;

@huynguyenh, please take a look)

@nktsitas
Copy link

hello @huynguyenh ,

It is possible that with the introduction of errors on found missing migrations, this breaks migration up functionality for clickhouse driver after the first run.

goose/up.go

Line 222 in fbd200b

rows, err := GetDialect().dbVersionQuery(db)

2022/04/11 17:23:33 goose run: error: found 2 missing migrations:
        version 20220411162903: migrations\clickhouse\20220411162903_create_sites_table.sql
        version 20220411163159: migrations\clickhouse\20220411163159_create_campaigns_table.sql

goose_db_version table:

version_id    |is_applied|date      |tstamp                 |
--------------+----------+----------+-----------------------+
             0|         1|2022-04-11|2022-04-11 13:31:30.000|
20220411162903|         1|2022-04-11|2022-04-11 13:31:40.000|
20220411163159|         1|2022-04-11|2022-04-11 13:32:28.000|

Also, enabling --allow-missing will go into a cycle creating new rows of migrations, since the known are only the last one.

@mfridman mfridman mentioned this pull request Apr 23, 2022
3 tasks
@mfridman
Copy link
Collaborator

This looks good to me. I tested this as part of #338

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

Successfully merging this pull request may close these issues.

3 participants