Support clickhouse v2 #1261
Support clickhouse v2 #1261
Conversation
|
@kokizzu There are linter failures. Could you fix them? |
| @@ -9,6 +9,7 @@ import ( | |||
| "strings" | |||
| "time" | |||
|
|
|||
| "github.com/ClickHouse/clickhouse-go/v2" | |||
There was a problem hiding this comment.
Can you add a v2 db driver similar to how we use pgxv4 and pgxv5? That way users can decide when to migrate themselves.
There was a problem hiding this comment.
i don't think it's possible, clickhouse v2 autoregister as "clickhouse" same as v1 (that already deprecated)
while pgx it using different name "pgx5"
There was a problem hiding this comment.
I've confirmed this behavior in the code:
| @@ -33,12 +33,12 @@ var ( | |||
| func clickhouseConnectionString(host, port, engine string) string { | |||
| if engine != "" { | |||
| return fmt.Sprintf( | |||
| "clickhouse://%v:%v?username=user&password=password&database=db&x-multi-statement=true&x-migrations-table-engine=%v&debug=false", | |||
| "clickhouse://%v:%v/db?username=user&password=password&x-multi-statement=true&x-migrations-table-engine=%v&debug=false", | |||
There was a problem hiding this comment.
this one needed at least until this one merged:
ClickHouse/clickhouse-go#1541
ref: ClickHouse/clickhouse-go#757
other related issue: #380
| @@ -64,7 +63,7 @@ type DockerContainer struct { | |||
| Cmd []string | |||
| ContainerId string | |||
| ContainerName string | |||
| ContainerJSON dockertypes.ContainerJSON | |||
| ContainerJSON dockercontainer.InspectResponse | |||
There was a problem hiding this comment.
Can you back out these changes? The testing package is deprecated in favor of dktest. This is only kept in case users were still using it.
There was a problem hiding this comment.
You can use //nolint:staticcheck to ignore these false positives for now. I'm not sure what's different in CI since I'm not getting these warnings locally.
EDIT: It's probably because of the docker version upgrade from v27 to v28. I'd back out that change in go.mod first
There was a problem hiding this comment.
downgrading clickhouse-go/v2 version then, because it was needed by clickhouse/v2
There was a problem hiding this comment.
not possible, all related to clickhouse broken if downgrading
There was a problem hiding this comment.
@kokizzu Use clickhouse v2.32.2 to stay on docker v27
There was a problem hiding this comment.
@dhui cannot, all related to clickhouse broken if downgrading to that version
fixes #942
context: clickhouse-go v1 already deprecated, there wont be any new features of clickhouse will be supported, only critical fix, clickhouse/v2 meanwhile always updated with latest clickhouse new features.
from official documentation: v1 of the driver is deprecated and will not reach feature updates or support for new ClickHouse types. Users should migrate to v2, which offers superior performance.
previous PR not sure why the pull bot cleaned my commit #1245
@dhui