Fix Clickhouse driver to work correctly with v2 #723
Conversation
database/clickhouse/clickhouse.go
Outdated
| if _, err := tx.Exec(query, version, bool(dirty), time.Now().UnixNano()); err != nil { | ||
| stmt, err := tx.Prepare(query) | ||
| if err != nil { | ||
| tx.Rollback() |
There was a problem hiding this comment.
You could change this to this
if errRollback := tx.Rollback(); errRollback != nil {
return fmt.Errorf("error during prepare statement %w and rollback %s", err, errRollback.Error())
}
return err
|
@dhui could you please review changes?
|
|
@dhui Any updates? I made a fork of this package in my company's private repo, and currently I use Maybe it's time to merge this PR? |
|
Join to the @lo00l, forced to do the same thing. Everything is working as expected. |
|
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. |
|
I can confirm that these changes fixed this problem Merge this into master ASAP |
|
@dhui Please review this PR Thanks 🙏 |
|
@dhui Gentle reminder, please review any of PRs related to clickhouse driver v2 |
|
@lo00l please update your PR with master branch 🙏🏿 |
This PR solves a problem mentioned in #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 addedstmt, 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 #697, but in this case tests are not modified.