migrate
migrate copied to clipboard
Upgrade ClickHouse to modern v2 driver and make minor improvements
Note: this PR is based on some of the problems/solutions and PR review feedback in #723 and #697
The main change – v2 driver upgrade:
- Upgrades the CLI build to use the recommended v2 driver.
- Adds a newer ClickHouse server version to the associated tests.
- Also splits out test coverage for the v1 driver, mutually exclusive from the v2 driver test using a build tag
Some improvements:
- Validation of database specification (fail for DSN mismatch)
- Escape table and database names in all queries
Open questions: How would the maintainers recommend going about executing the legacy v1 driver test for regression protection without introducing a nasty one-off in the top-level Makefile? If people are still using migrate in applications (and doing so with the v1 driver), it would be nice for CI to catch regressions before merge. On the other hand, I won't be very upset if we just categorize this as a breaking change for v1, remove all the v1 stuff, and point to ClickHouse's own recommendation to use the v2 driver.
Coverage decreased (-0.3%) to 57.47% when pulling 979c9e33465b10b1fa5f36a2e7defe3bebf9ce27 on zcross:zcross/clickhouse-improvements into 03613f14ac4f975eb0070a23958123c5d84e6b87 on golang-migrate:master.
I see now that the lint failure is unrelated to any of my changed sources, so I'll just fix them in a separate commit, in a moment