Skip to content

ci: extend ClickHouse version matrix in test-local - #462

Open
polyglotAI-bot wants to merge 3 commits into
mainfrom
polyglot/ci-ch-version-matrix
Open

ci: extend ClickHouse version matrix in test-local#462
polyglotAI-bot wants to merge 3 commits into
mainfrom
polyglot/ci-ch-version-matrix

Conversation

@polyglotAI-bot

Copy link
Copy Markdown
Collaborator

Description

CI/maintenance chore: bring the ClickHouse version coverage of the test-local job onto the support policy (the 3 latest series + the last 2 LTS series + head).

  • Added: 26.7, 26.6, 26.5 (latest series), 26.3, 25.8 (LTS)
  • Removed: nothing. latest is kept, but it floats, so it does not count as coverage of any series.
  • head was already in the matrix.

Changes

.github/workflows/ci.ymltest-local matrix:

clickhouse: [ latest, head ]
->
clickhouse: [ '25.8', '26.3', '26.5', '26.6', '26.7', latest, head ]

Versions are quoted so YAML keeps them as strings (an unquoted 26.30-style value would lose its trailing zero).

No other change is needed: docker-compose.yml already passes CLICKHOUSE_VERSION to the server image tag.

Test

  • .github/workflows/ci.yml parses; the test-local matrix resolves to the 7 entries above.
  • The include: block still marks only latest with coverage: true, so the Codecov report/upload steps still run for exactly one matrix entry.
  • Every new tag exists on Docker Hub (docker manifest inspect clickhouse/clickhouse-server:<version> for 25.8, 26.3, 26.5, 26.6, 26.7).

Notes

Version coverage only — no client behavior changes, so no CHANGELOG entry or version bump. Other matrix dimensions are untouched.

Cover the supported ClickHouse series (3 latest + 2 LTS) in addition to
the floating `latest` tag and `head`:

- Latest: 26.7, 26.6, 26.5
- LTS:    26.3, 25.8

`docker-compose.yml` already reads `CLICKHOUSE_VERSION`, so no compose
change is needed. Coverage upload stays on the single `latest` entry.
@codecov

codecov Bot commented Aug 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Extending the `test-local` matrix to the supported server versions showed
that several tests need a server newer than the oldest supported release:

- `insert_native::mixed_types_*` and the `Nullable(Tuple(...))` case of
  `fetch_native::empty_arrays` need 26.6, which added the type.
- `native_types::test_bool` relied on the `Values(...)` table function
  inferring `Bool`; servers before 26.0 infer `UInt8`.
- `insert::insert_unescaped` and `insert::insert_unvalidated` relied on the
  server supplying a primary key for a `MergeTree` table.

Add a `require_server_version!` skip macro, next to the existing
`check_cloud_test_env!`, and declare the requirement where it is real. Make
the other two tests version-independent instead: the bool test now states the
SQL type, and both DDLs now state the engine and the primary key.
@polyglotAI-bot

Copy link
Copy Markdown
Collaborator Author

Pushed a40b085 to make the suite version-aware. The new matrix legs were red because several tests silently require a server newer than the oldest supported release — the matrix only exposed it, since CI previously ran latest and head only.

Two tests were simply version-dependent by accident, and are now version-independent:

  • native_types::test_bool — relied on the Values(...) table function inferring Bool for the false / true literals. Servers before 26.0 infer UInt8. The test now states the SQL type, which the test_type! macro already supports.
  • insert::insert_unescaped and insert::insert_unvalidated — relied on the server supplying a primary key for a MergeTree table. Both DDLs now state ENGINE = MergeTree PRIMARY KEY(bar), matching the other DDLs in that file.

Three genuinely need ClickHouse 26.6, which added Nullable(Tuple(...)). For these there is a new require_server_version!(major, minor) skip macro next to the existing check_cloud_test_env!:

  • insert_native::mixed_types_* — sets enable_nullable_tuple_type, which older servers reject as an unknown setting.
  • the Array(Nullable(Tuple(...))) case of fetch_native::empty_arrays — split into empty_arrays_of_nullable_tuple so the other 11 element types keep running on every version.

Verified in Docker on both sides of the boundary:

  • 25.8 — the whole integration suite passes (197 passed, 0 failed); the three 26.6 tests print the skip line.
  • 26.7 — the same tests all run and pass; nothing is skipped, so the gate does not hide the coverage on current servers.

cargo fmt -- --check and cargo clippy --all-targets are clean.

One failure in the previous run is not addressed here and is not caused by this PR: opentelemetry::error_sets_span_status failed on the 26.6 leg only, and passed on 25.8, 26.3, 26.5, 26.7, latest, and head in that same run. It reads the exported span from a thread_local!, while the tracer is process-wide and the harness is multi-threaded, so the span can end on another thread. That is a pre-existing race and belongs in its own change.

… key

`insert_with_opentelemetry` created its table without an ENGINE clause, so
it depended on the server supplying a default engine and a default primary
key. Servers in the supported range reject this DDL with
`ORDER BY or PRIMARY KEY clause is missing` (Code: 42), which made the new
25.8 matrix leg fail.

State the engine and primary key like the rest of the suite does
(tests/it/insert.rs), which makes the test version-independent.
@polyglotAI-bot

Copy link
Copy Markdown
Collaborator Author

Pushed 4a044b5.

Fixed — test-local (25.8): opentelemetry::insert_with_opentelemetry.
The DDL in tests/it/opentelemetry.rs created its table without an ENGINE
clause, so it relied on the server supplying a default engine and a default
primary key. Servers in the supported range reject it with
ORDER BY or PRIMARY KEY clause is missing (Code: 42). It now states
ENGINE = MergeTree PRIMARY KEY(bar), like the rest of the suite
(tests/it/insert.rs). This is the same DDL family as the previous iteration;
this occurrence was in a different file and was only reached after the earlier
failures stopped aborting the run.

Verified in Docker with cargo test --all-features --workspace:

  • server 25.8.28.1 — full suite green (241 integration tests passed, 0 failed).
  • server latest — the three opentelemetry tests pass.

Not fixed — test-local (26.7): opentelemetry::error_sets_span_status.
This is a pre-existing flake, not related to this diff. It failed on the 26.6
leg of the previous run and on the 26.7 leg of this one, while passing on every
other leg of both runs; it also passes locally on 25.8 and on latest. The test
reads the exported span from a thread_local! while the tracer provider is
process-wide and the test harness is multi-threaded, so a concurrently running
opentelemetry test can consume the span it expects. It is tracked separately
so it is not bundled into this CI-coverage change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant