fix: Fixes the failure againt HEAD of CH server by kavirajk · Pull Request #1752 · ClickHouse/clickhouse-go

kavirajk · 2026-01-14T16:22:24Z

Summary

Two changes:

We cannot use async_insert and quorum_insert settings together in latest CH
Then version check for supportsFlatDynamicJSON was buggy. Causing it to return false for version26.1 (major >=25, but minor <6) (Thanks @Avogar for catching the bug 🙇 )

Related PRs on core

https://github.com/ClickHouse/ClickHouse/pull/89140/changes#diff-ff8ad4aed4cf07fe29cb5344d2ab79bc24efd97d054aa112c3a05b5ab853cc44R1558-R1562

Should fix the periodic tests running against current CH head (after 25.12)

Before

$ CLICKHOUSE_VERSION=head go test ./tests/... -run TestDynamicExceededTypes -count=1
using random seed 1768407595883362685 for native tests
Using Docker for integration tests
ClickHouse 26.1.1 ready: Container=62d1298a6bf1 Host=127.0.0.1 TCP=33119 HTTP=33117 SSL=33120 HTTPS=33118
--- FAIL: TestDynamicExceededTypes (0.04s)
    --- FAIL: TestDynamicExceededTypes/UInt8_bounds (0.01s)
        dynamic_test.go:224:
            	Error Trace:	/home/kavi/src/clickhouse-go/tests/dynamic_test.go:224
            	Error:      	Received unexpected error:
            	            	code: 36, message: Variant type with more than 255 nested types is not allowed
            	Test:       	TestDynamicExceededTypes/UInt8_bounds
    --- FAIL: TestDynamicExceededTypes/UInt16_range (0.01s)
        dynamic_test.go:224:
            	Error Trace:	/home/kavi/src/clickhouse-go/tests/dynamic_test.go:224
            	Error:      	Received unexpected error:
            	            	code: 36, message: Variant type with more than 255 nested types is not allowed
            	Test:       	TestDynamicExceededTypes/UInt16_range
FAIL
FAIL	github.com/ClickHouse/clickhouse-go/v2/tests	6.456s
ok  	github.com/ClickHouse/clickhouse-go/v2/tests/issues	6.319s [no tests to run]
?   	github.com/ClickHouse/clickhouse-go/v2/tests/issues/209	[no test files]
?   	github.com/ClickHouse/clickhouse-go/v2/tests/issues/360	[no test files]
?   	github.com/ClickHouse/clickhouse-go/v2/tests/issues/470	[no test files]
?   	github.com/ClickHouse/clickhouse-go/v2/tests/issues/476	[no test files]
?   	github.com/ClickHouse/clickhouse-go/v2/tests/issues/484	[no test files]
?   	github.com/ClickHouse/clickhouse-go/v2/tests/issues/485	[no test files]
ok  	github.com/ClickHouse/clickhouse-go/v2/tests/std	6.178s [no tests to run]
?   	github.com/ClickHouse/clickhouse-go/v2/tests/stress	[no test files]
FAIL
$ CLICKHOUSE_VERSION=head go test ./examples/... -run TestAsyncInsert -count=1
using random seed 1768407605293614881 for examples_clickhouse_api tests
Using Docker for integration tests
ClickHouse 26.1.1 ready: Container=811cae9b564b Host=127.0.0.1 TCP=33132 HTTP=33130 SSL=33133 HTTPS=33131
--- FAIL: TestAsyncInsert (0.01s)
    main_test.go:59:
        	Error Trace:	/home/kavi/src/clickhouse-go/examples/clickhouse_api/main_test.go:59
        	Error:      	Received unexpected error:
        	            	code: 344, message: Insert quorum cannot be used together with async inserts. Please disable either `insert_quorum` or `async_insert` setting.
        	Test:       	TestAsyncInsert
FAIL
FAIL	github.com/ClickHouse/clickhouse-go/v2/examples/clickhouse_api	1.370s
ok  	github.com/ClickHouse/clickhouse-go/v2/examples/std	1.260s [no tests to run]
FAIL
$

After

$ CLICKHOUSE_VERSION=head go test ./tests/... -run TestDynamicExceededTypes -count=1
ok  	github.com/ClickHouse/clickhouse-go/v2/tests	6.507s
ok  	github.com/ClickHouse/clickhouse-go/v2/tests/issues	6.336s [no tests to run]
?   	github.com/ClickHouse/clickhouse-go/v2/tests/issues/209	[no test files]
?   	github.com/ClickHouse/clickhouse-go/v2/tests/issues/360	[no test files]
?   	github.com/ClickHouse/clickhouse-go/v2/tests/issues/470	[no test files]
?   	github.com/ClickHouse/clickhouse-go/v2/tests/issues/476	[no test files]
?   	github.com/ClickHouse/clickhouse-go/v2/tests/issues/484	[no test files]
?   	github.com/ClickHouse/clickhouse-go/v2/tests/issues/485	[no test files]
ok  	github.com/ClickHouse/clickhouse-go/v2/tests/std	6.189s [no tests to run]
?   	github.com/ClickHouse/clickhouse-go/v2/tests/stress	[no test files]
$ CLICKHOUSE_VERSION=head go test ./examples/... -run TestAsyncInsert -count=1
ok  	github.com/ClickHouse/clickhouse-go/v2/examples/clickhouse_api	1.562s
ok  	github.com/ClickHouse/clickhouse-go/v2/examples/std	1.323s [no tests to run]
$

Checklist

Delete items not relevant to your PR:

Unit and integration tests covering the common scenarios were added
A human-readable description of the changes was provided to include in CHANGELOG
For significant changes, documentation in https://github.com/ClickHouse/clickhouse-docs was updated with further explanations or tutorials

chernser · 2026-01-14T17:29:32Z

docker-compose.yml

      - clickhouse
    container_name: clickhouse
-    image: clickhouse/clickhouse-server:25.11
+    image: 'clickhouse/clickhouse-server:${CLICKHOUSE_VERSION-25.12-alpine}'


name of the variable contains version. It should be just ${CLICKHOUSE_VERSION} , right?

+1 on this, what does this env value resolve to?

t's not a typo. It's a way to set the default value if the ENV is not set (same as in bash)
https://docs.docker.com/compose/how-tos/environment-variables/variable-interpolation/#interpolation-syntax

I used just - instead of :- (like in bash) because it may be bit "surprise" for the user.

e.g:

I by mistake set CLICKHOUSE_VERSION="" (empty). But when I run docker-compose up it always picks the default value. I may get confuse why it's not picking the value I set.

Where as with current approach, it will take the "empty" value and try to pull the image -> which will fail -> immediate feedback that user made a mistake of setting "wrong" empty value.

I prefer to fail early and fast :)

Two changes: 1. We cannot use async_insert and quorum_insert settings together in latest CH 2. The Variant validation is more strict on Native format compared to 25.12 Related PRs on core 1. https://github.com/ClickHouse/ClickHouse/pull/89140/changes#diff-ff8ad4aed4cf07fe29cb5344d2ab79bc24efd97d054aa112c3a05b5ab853cc44R1558-R1562 2. https://github.com/ClickHouse/ClickHouse/pull/92843/changes Signed-off-by: Kaviraj <[email protected]>

That was original cause for Variant type exceeding nested types error. The check failed for 26.1 (major version >=25 but minor version less than 6) Thanks Paval from core team for catching this. Signed-off-by: Kaviraj <[email protected]>

Avogar · 2026-01-15T14:20:06Z

lib/column/dynamic.go

 func supportsFlatDynamicJSON(sc *ServerContext) bool {
-	return sc.VersionMajor >= 25 && sc.VersionMinor >= 6
+	// Any CH version more than 25.6
+	return sc.VersionMajor >= 25 || (sc.VersionMajor == 25 && sc.VersionMinor >= 6)


Suggested change

return sc.VersionMajor >= 25 || (sc.VersionMajor == 25 && sc.VersionMinor >= 6)

return sc.VersionMajor > 25 || (sc.VersionMajor == 25 && sc.VersionMinor >= 6)

Otherwise any 25 major version will pass the check

Signed-off-by: Kaviraj <[email protected]>

kavirajk requested review from SpencerTorres, chernser and jkaflik January 14, 2026 16:22

chernser reviewed Jan 14, 2026

View reviewed changes

SpencerTorres approved these changes Jan 15, 2026

View reviewed changes

kavirajk marked this pull request as draft January 15, 2026 14:02

kavirajk added 2 commits January 15, 2026 15:03

fix the JSONMajorVersion bug

77375b8

That was original cause for Variant type exceeding nested types error. The check failed for 26.1 (major version >=25 but minor version less than 6) Thanks Paval from core team for catching this. Signed-off-by: Kaviraj <[email protected]>

kavirajk force-pushed the kavirajk/make-head-test-pass branch from 781840f to 77375b8 Compare January 15, 2026 14:10

kavirajk marked this pull request as ready for review January 15, 2026 14:13

Avogar reviewed Jan 15, 2026

View reviewed changes

remove duplicate equality check for major version 25.

240bdd2

Signed-off-by: Kaviraj <[email protected]>

kavirajk merged commit e99ef45 into main Jan 15, 2026
13 of 14 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Fixes the failure againt HEAD of CH server#1752

fix: Fixes the failure againt HEAD of CH server#1752
kavirajk merged 3 commits intomainfrom
kavirajk/make-head-test-pass

kavirajk commented Jan 14, 2026 •

edited

Loading

Uh oh!

chernser Jan 14, 2026

Uh oh!

SpencerTorres Jan 15, 2026

Uh oh!

kavirajk Jan 15, 2026

Uh oh!

Avogar Jan 15, 2026 •

edited

Loading

Uh oh!

Avogar Jan 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	return sc.VersionMajor >= 25 \|\| (sc.VersionMajor == 25 && sc.VersionMinor >= 6)
	return sc.VersionMajor > 25 \|\| (sc.VersionMajor == 25 && sc.VersionMinor >= 6)

Conversation

kavirajk commented Jan 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Before

After

Checklist

Uh oh!

chernser Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

SpencerTorres Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

kavirajk Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

Avogar Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Avogar Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kavirajk commented Jan 14, 2026 •

edited

Loading

Avogar Jan 15, 2026 •

edited

Loading