Skip to content

Cherry-pick: Fix parquet reader v3 handling arrays that cross pages#2103

Open
zvonand wants to merge 2 commits into
antalya-25.8from
releasy/port/pr-94125-be5cb0
Open

Cherry-pick: Fix parquet reader v3 handling arrays that cross pages#2103
zvonand wants to merge 2 commits into
antalya-25.8from
releasy/port/pr-94125-be5cb0

Conversation

@zvonand

@zvonand zvonand commented Jul 22, 2026

Copy link
Copy Markdown
Member

Cherry-picked from ClickHouse#94125.

Changelog category (leave one):

  • Bug Fix (user-visible misbehavior in an official stable release)

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Fixed parquet reader v3 incorrectly handling arrays that cross page boundaries. This happens e.g. for files written by Arrow without enabling page statistics or page index. Affects only columns of Array data type. Likely symptom is that one array every ~1 MB of data gets truncated. Before this fix, use this setting as workaround: input_format_parquet_use_native_reader_v3 = 0 (ClickHouse#94125 by @al13n321)

CI/CD Options

Exclude tests:

  • Fast test
  • Integration Tests
  • Stateless tests
  • Stateful tests
  • Performance tests
  • All with ASAN
  • All with TSAN
  • All with MSAN
  • All with UBSAN
  • All with Coverage
  • All with Aarch64
  • All Regression
  • Disable CI Cache

Regression jobs to run:

  • Fast suites (mostly <1h)
  • Aggregate Functions (2h)
  • Alter (1.5h)
  • Benchmark (30m)
  • ClickHouse Keeper (1h)
  • Iceberg (2h)
  • LDAP (1h)
  • Parquet (1.5h)
  • RBAC (1.5h)
  • SSL Server (1h)
  • S3 (2h)
  • S3 Export (2h)
  • Swarms (30m)
  • Tiered Storage (2h)

Fix parquet reader v3 handling arrays that cross pages
@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown

Workflow [PR], commit [3915ad3]

@zvonand

zvonand commented Jul 22, 2026

Copy link
Copy Markdown
Member Author

@blau-ai

@blau-ai

blau-ai commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

CI triage for #2103

Verdict — 4 distinct failures: 1 PR-caused (fixable), 3 not.

Check Class PR-caused?
Stateless tests … 00900_long_parquet_load_2 (fails on ~8 shards) test Yes
Grype Scan (server / keeper / -alpine) infra / image CVE scan No
RegressionTestsRelease / Swarms (1 of 1520 scenarios) flaky / pre-existing No
DCO process (missing sign-off) No (formality)

1. Stateless 00900_long_parquet_load_2PR-caused, needs a fix

This is the only test the PR touches, and it fails with exactly 1 failure on every stateless configuration that ran it (amd_debug parallel, arm_binary parallel, amd_debug distributed, amd_debug AsyncInsert, amd_binary old-analyzer DatabaseReplicated, ParallelReplicas parallel/sequential, …). A single test failing deterministically across unrelated configs is the signature of a bad expected-output, not flakiness.

Root cause — the cherry-pick pulled in test changes that depend on upstream features not present in 25.8. Upstream ClickHouse#94125 did two things beyond the actual bug fix:

  • In 00900_long_parquet_load_2.sh it removed SETTINGS input_format_parquet_enable_json_parsing=0, session_timezone='UTC', with this comment (deleted by the port):
    # TODO [parquet]: Delete input_format_parquet_enable_json_parsing=0 when cityHash64
    #                 supports JSON: https://github.com/ClickHouse/ClickHouse/issues/87734
    # TODO [parquet]: Delete session_timezone='UTC' after .../pull/87872
    
  • It regenerated 00900_long_parquet_load_2.reference accordingly.

With JSON parsing now enabled, the JSON columns in the pre-existing test files render differently and the row hashes change. Compare base antalya-25.8 vs this PR:

multi_column_bf:  {"key":400, "value":"BNHSB"}  ->  {"key":400,"value":"BNHSB"}   (space removed, hash changed)
sample_json:      {"user":{"name":"Alice",...}} ->  {"scores":[10,20],"user":{"age":31,"name":"Alice"}}  (keys reordered, hash changed)

antalya-25.8 does not have upstream ClickHouse#87734 / ClickHouse#87872, so its binary still emits the old spacing / key-order / hashes. The reference now describes upstream-master behavior, so the pre-existing multi_column_bf / sample_json (and an ipv6_bloom_filter whitespace) rows mismatch and the test fails. The actual Reader.cpp/Reader.h fix and the new array_across_pages_* rows are fine; the churn is what breaks CI.

Suggested fix — isolate the port to just the array-across-pages fix:

  1. Keep src/Processors/Formats/Impl/Parquet/Reader.cpp + Reader.h (the real fix) and both new data_parquet/array_across_pages_*.parquet files.

  2. Revert 00900_long_parquet_load_2.sh to the 25.8 version — restore SETTINGS input_format_parquet_enable_json_parsing=0, session_timezone='UTC' and the two TODO comments (drop the upstream .sh hunk entirely).

  3. Restore the 25.8 00900_long_parquet_load_2.reference and add only the two new blocks after line 113 (the \N ... 2 14757089928194662262 line, before === Try load data from array_float.parquet):

    === Try load data from array_across_pages_1.parquet
    0	[1,2]	1	5845551876324623491
    
    \N	[]	1	5845551876324623491
    === Try load data from array_across_pages_2.parquet
    0	[1,2]	1	5845551876324623491
    1	[3]	1	204286521036002596
    
    \N	[]	2	6049838397360626087
    

    i.e. drop the multi_column_bf / sample_json / ipv6_bloom_filter hunks that came from upstream.

The array_across_pages_* files contain only integers/arrays — no JSON and no timestamps — so input_format_parquet_enable_json_parsing / session_timezone don't affect their output, and the hashes above should hold under 25.8's settings. One residual unknown I can't check without a build: those 8 hashes were generated upstream; if CI shows they differ they'd need regenerating locally. Green CI on the reworked PR is the real confirmation.

2. Grype Scan (server / keeper / -alpine) — not PR-caused (infra / image CVE scan)

These scan the built Docker images for CVEs and report "An error occurred" (jobs finished in ~1m). A Parquet-reader C++ change cannot affect base-image vulnerabilities; this is infra or a pre-existing CVE baseline on antalya-25.8. Safe to ignore for this PR / re-run.

3. RegressionTestsRelease / Swarmsnot PR-caused (flaky / pre-existing)

Report: 1520 scenarios (1487 ok, 1 failed, 32 xfail). Swarms is a distributed-cluster feature with no relationship to the Parquet reader. A single scenario out of 1520 is a classic flake/pre-existing; recommend a re-run and comparison against recent antalya-25.8 swarms runs before treating it as real.

4. DCOnot code breakage (process/formality)

The ported commit 83f8777 is the upstream merge commit ("Merge pull request ClickHouse#94125 ...", authored by Alexey Milovidov) and carries no Signed-off-by trailer, which DCO requires. This is expected for a straight cherry-pick of an upstream commit. Resolvable by re-committing the port with a sign-off (-s) or a maintainer DCO override — not something I can do here (I won't amend/force-push others' commits).


Bottom line: only #1 needs code work, and it's a clean scope-down of the cherry-pick (drop the JSON-parsing .sh/reference churn that belongs to upstream features 25.8 lacks; keep the reader fix + array test). #2#4 are infra/flaky/formality.

I can prepare this fix — want me to open a separate blau/* PR with it, or (since you have write access) commit it directly onto releasy/port/pr-94125-be5cb0? I'll wait for your call before pushing anything.

@blau-ai · evidence: run 29899788849, upstream ClickHouse#94125, base antalya-25.8. I can't build/run ClickHouse in this container, so correctness is confirmed by CI on the reworked PR.

Revert 00900_long_parquet_load_2.sh and reference to the antalya-25.8
versions, keeping only the new array_across_pages_* reference blocks.
The upstream changes enabling JSON parsing and dropping session_timezone
depend on features not present in 25.8 and broke the test on all configs.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Andrey Zvonov <32552679+zvonand@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants