Skip to content

Fix flaky binary_strings_round_trip - #73

Merged
abonander merged 1 commit into
mainfrom
fix-extra-metadata
Aug 28, 2026
Merged

Fix flaky binary_strings_round_trip#73
abonander merged 1 commit into
mainfrom
fix-extra-metadata

Conversation

@koletzilla

Copy link
Copy Markdown
Contributor

fix: don't assert version-dependent server metadata in binary_strings_round_trip

Problem

CI is red on params::binary_strings_round_trip with a schema mismatch where the data is
byte-identical and the only difference is one metadata key: the expected schema has
"PARQUET:logical_type": "UUID" on the UUID column, but the server response doesn't.

That key is incidental server metadata and varies by server version — the test baked it in
because the clickhouse-server:latest-alpine image emitted it when the test was written. CI
uses the unpinned latest-alpine tag, and current versions no longer emit the key, so the
test broke without any driver change.

Change

Test-only, in tests/it/params.rs:

  • Removed the baked-in PARQUET:logical_type entry from the expected schema (the canonical
    arrow.uuid extension keys stay).
  • The server's response schema is normalized before the comparison by dropping
    PARQUET:logical_type from each field's metadata, so the test passes whether or not the
    server version emits that key. Everything else — data, types, nullability, and all other
    metadata — is still compared strictly.

Verification

  • Reproduced the exact CI failure against a live server; the test passes with the fix.
  • Full integration suite green (12/12), cargo fmt / clippy clean.

Follow-up idea (not in this PR)

CI's unpinned latest-alpine image means server behavior changes land on open PRs as unrelated
failures. Pinning a server version for PR CI (with a scheduled job tracking latest) would keep
PR runs deterministic.

@koletzilla
koletzilla requested a review from abonander as a code owner August 28, 2026 16:50
@abonander
abonander merged commit 8972ba7 into main Aug 28, 2026
6 checks passed
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.

2 participants