migrate icon indicating copy to clipboard operation
migrate copied to clipboard

Published 20 hours ago • verified publisher icon golang-migrate

Fix Clickhouse driver to work correctly with v2

Open lo00l opened this issue 2 years ago • 10 comments

This PR solves a problem mentioned in https://github.com/golang-migrate/migrate/issues/704.

The reason migrating stopped working with Golang Clickhouse driver of 2nd version (github.com/ClickHouse/clickhouse-go/v2) is that it's now required to use prepared statements when inserting values to DB. Currently, this package (github.com/golang-migrate/migrate/v4) doesn't use prepared statements when migrationg Clickhouse. I just added stmt, err := tx.Prepare(query) and now everything works as expected both in 1st and 2nd version of Clickhouse driver.

You can see tests in my repo: https://github.com/lo00l/migrate-test/actions/runs/2072749495

Also this PR introduces similar changes as https://github.com/golang-migrate/migrate/pull/697, but in this case tests are not modified.

lo00l avatar Mar 31 '22 18:03 lo00l

Coverage Status

Coverage decreased (-0.02%) to 57.784% when pulling 2237947f72536eb1b528876f63f28b173fe04bb7 on lo00l:master into 331a15d92a86e002ce5043e788cc2ca287ab0cc2 on golang-migrate:master.

coveralls avatar Mar 31 '22 18:03 coveralls

@lo00l Could you please change your commit.

@dhui Could you please review and approve this PR.

Thanks in advance!

johnatannvmd avatar May 13 '22 14:05 johnatannvmd

@dhui could you please review changes?

image

johnatannvmd avatar Jun 09 '22 06:06 johnatannvmd

@dhui Any updates?

I made a fork of this package in my company's private repo, and currently I use replace directive in go.mod file to install correct version of the package. This version is based on my branch. And everything works well, with no issues.

Maybe it's time to merge this PR?

lo00l avatar Jul 06 '22 09:07 lo00l

Join to the @lo00l, forced to do the same thing. Everything is working as expected.

johnatannvmd avatar Jul 09 '22 18:07 johnatannvmd

Bumping out of interest (thanks to the author/reviewers); I can +1 that I've experienced the problem that motivated this PR and that using this fork solved the problem.

zcross avatar Aug 05 '22 15:08 zcross

I can confirm that these changes fixed this problem code: 62, message: Cannot parse expression of type Int64 here: ?, ?, ?): While executing ValuesBlockInputFormat in line 0: INSERT INTO schema_migrations (version, dirty, sequence) VALUES (?, ?, ?)

Merge this into master ASAP

artur-shafikov avatar Nov 15 '22 10:11 artur-shafikov

@dhui Please review this PR

Thanks 🙏

makeavish avatar Jan 17 '23 12:01 makeavish

@dhui Gentle reminder, please review any of PRs related to clickhouse driver v2

makeavish avatar Jun 08 '23 11:06 makeavish

@lo00l please update your PR with master branch 🙏🏿

obalunenko avatar Aug 16 '23 16:08 obalunenko