Skip to content

Use min/max stats for raw parquet INT64 columns read as DateTime - #117017

Open
wudidapaopao wants to merge 6 commits into
ClickHouse:masterfrom
wudidapaopao:parquet-int64-datetime-stats
Open

Use min/max stats for raw parquet INT64 columns read as DateTime#117017
wudidapaopao wants to merge 6 commits into
ClickHouse:masterfrom
wudidapaopao:parquet-int64-datetime-stats

Conversation

@wudidapaopao

@wudidapaopao wudidapaopao commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

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 into CHANGELOG.md):

Use min/max statistics for pruning when a parquet INT64 column is read as DateTime, including raw INT64 columns without a logical type annotation. Also stop using out-of-range INTEGER(64) statistics as DateTime bounds, which could previously prune matching row groups incorrectly.

Previously, predicates on such columns scanned all row groups. Statistics values outside the DateTime range are not used as bounds, because the cast saturates.

IPv4 statistics are additionally restricted to unsigned 32-bit parquet columns — the only input whose cast is supported — so that pruning cannot hide an unsupported-cast error behind an empty result.

Columns of this shape are common in files from older parquet writers, e.g. EventTime, ClientEventTime and LocalEventTime in the ClickBench hits.parquet.


Workflow [PR]
Sync PR [sync-upstream/pr/117017]

Stats outside the `DateTime` range are not used as bounds because the
cast saturates.
@clickhouse-gh

clickhouse-gh Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Workflow [PR], commit [bf68630]


AI Review

Summary

This PR extends parquet min/max pruning to raw physical INT64 columns read as DateTime, drops out-of-range 64-bit bounds that would not survive the internal saturating _CAST, and disables IPv4 stats for integer shapes that could otherwise hide a conversion error or misprune rows. I did not find any remaining code-level correctness or safety issues in the current diff.

PR Metadata

Changelog category is correct, and a Changelog entry is required. The entry is specific overall, but the sentence saying unsigned 32-bit columns are "the only input whose cast is supported" is inaccurate: UInt64 -> IPv4 casts are supported today, they are just intentionally excluded from stats pruning because the cast wraps and does not preserve bounds. Suggested replacement:

IPv4 statistics are additionally restricted to unsigned 32-bit parquet columns, because wider integer inputs do not preserve min/max bounds and could let pruning hide a conversion error or misprune matching rows.

Final Verdict

Non-blocking review. No new inline code findings; only the changelog wording above should be corrected before merge.

@clickhouse-gh clickhouse-gh Bot added the pr-performance Pull request with some performance improvements label Aug 29, 2026
@wudidapaopao wudidapaopao added the can be tested Allows running workflows for external contributors label Aug 29, 2026
Comment thread src/Processors/Formats/Impl/Parquet/SchemaConverter.cpp
@clickhouse-gh

clickhouse-gh Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Build profile diff (arm_release)

Comparing 0ad2c20a7 with master 9adeb11c7 (stripped binary size, per-symbol sizes and ThinLTO time; compile times per translation unit against the most recent warmup build that recompiled it).

✅ No significant changes.

Binary sizes
Binary Master PR Δ
programs/clickhouse-stripped 710.14 MiB 707.08 MiB -3.05 MiB (-0.43%)

Only the stripped binary is compared: the official master build keeps debug symbols while PR builds strip them, so the other binaries differ by construction.

Compile time of recompiled translation units

17 translation units recompiled, 143 s compile time in total, 17 of them have a recent master baseline.

Job report

Only the `UInt32` to `IPv4` cast is supported; stats on other inputs
could hide the unsupported-cast error when all row groups are pruned.
@clickhouse-gh clickhouse-gh Bot added the pr-bugfix Pull request with bugfix, not backported by default label Aug 29, 2026
@clickhouse-gh

clickhouse-gh Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

📊 Cloud Performance Report

✅ AI verdict: no_change — no significant changes across 39 queries analysed

no significant changes detected. K_source=6 K_base=30 flagged=0/65

clickbench

🟢 No significant changes

tpch_adapted_1_official

🟢 No significant changes

Debug info
  • StressHouse run: 367bfa62-eb71-4c61-9071-f77afd9a7815
  • MIRAI run: dec2e15c-aad1-412d-a4ec-5889f563cf7a
  • PR check IDs:
    • clickbench_306597_1788000322
    • clickbench_306603_1788000322
    • clickbench_306609_1788000322
    • tpch_adapted_1_official_306624_1788000322
    • tpch_adapted_1_official_306654_1788000322
    • tpch_adapted_1_official_306668_1788000322

Comment thread src/Processors/Formats/Impl/Parquet/SchemaConverter.cpp
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

can be tested Allows running workflows for external contributors pr-bugfix Pull request with bugfix, not backported by default pr-performance Pull request with some performance improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant