Skip to content

Fix incorrect partition pruning for toWeek() function#99542

Merged
alexey-milovidov merged 2 commits intoClickHouse:masterfrom
takumihara:fix-toweek-partition-pruning
Mar 22, 2026
Merged

Fix incorrect partition pruning for toWeek() function#99542
alexey-milovidov merged 2 commits intoClickHouse:masterfrom
takumihara:fix-toweek-partition-pruning

Conversation

@takumihara
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 to CHANGELOG.md):

Fix incorrect partition pruning for toWeek() that caused queries with WHERE toWeek(date, mode) = N to return empty results for weeks 49-52 on tables partitioned by toYYYYMM(date).

Summary

toWeek() incorrectly claimed monotonicity via its FactorTransform (ToStartOfYearImpl), which caused partition pruning to skip partitions containing December dates when filtering by week numbers 49-52.

The root cause is that toWeek() with certain week modes (e.g. mode 3, ISO weeks) can have week numbers that wrap at year boundaries (week 52 → 1 in late December). The ToStartOfYearImpl factor transform does not detect this wrapping because both December 1 and December 31 are in the same calendar year, so it incorrectly reports the function as monotonic. The optimizer then computes an invalid value range (e.g. [49, 1]) and prunes the partition entirely.

No single FactorTransform can correctly handle all week modes because wrapping behavior depends on the runtime week_mode parameter and different modes wrap at different dates.

Fix: Add a hasMonotonicity() static method to each custom week Transform struct. ToWeekImpl returns false; all other transforms return true. IFunctionCustomWeek delegates to Transform::hasMonotonicity() instead of unconditionally returning true, following the same pattern used by hasPreimage() in DateTimeTransforms.h.

Closes #90240

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Mar 16, 2026

CLA assistant check
All committers have signed the CLA.

toWeek() was incorrectly claiming monotonicity via its FactorTransform
(ToStartOfYearImpl), which caused partition pruning to skip partitions
containing December dates when filtering by week numbers 49-52.

The issue was that toWeek() with certain week modes (e.g. mode 3, ISO
weeks) can have week numbers that wrap at year boundaries (week 52 -> 1
in late December). The monotonicity assumption caused the optimizer to
compute an invalid value range for the function output, leading to
entire partitions being incorrectly pruned.

Fix by disabling monotonicity information for ToWeekImpl, since week
numbers are not monotonic due to year-boundary wrapping behavior that
depends on the runtime week_mode parameter.

Closes ClickHouse#90240
@takumihara takumihara force-pushed the fix-toweek-partition-pruning branch from 32b2ac9 to 26f4b80 Compare March 16, 2026 06:29
@takumihara takumihara marked this pull request as ready for review March 16, 2026 06:31
@alexey-milovidov
Copy link
Copy Markdown
Member

Could you please sign the CLA?

@yariks5s yariks5s self-assigned this Mar 16, 2026
@takumihara
Copy link
Copy Markdown
Contributor Author

@alexey-milovidov @yariks5s
Signed the CLA 🙏

@yariks5s yariks5s added the can be tested Allows running workflows for external contributors label Mar 18, 2026
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Mar 18, 2026

Workflow [PR], commit [4037d6a]

Summary:

@clickhouse-gh clickhouse-gh bot added the pr-bugfix Pull request with bugfix, not backported by default label Mar 18, 2026
@alexey-milovidov
Copy link
Copy Markdown
Member

Check the test results.

- Add missing `GROUP BY` to `03732_toweek_partition_pruning` test
- Update `03789_to_year_week_monotonicity_key_condition` to verify
  that `toWeek` can no longer use primary key index (`INDEX_NOT_USED`)
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Mar 19, 2026

LLVM Coverage Report

Metric Baseline Current Δ
Lines 83.80% 83.70% -0.10%
Functions 23.90% 23.90% +0.00%
Branches 76.30% 76.30% +0.00%

PR changed lines: PR changed-lines coverage: 89.47% (17/19, 0 noise lines excluded)
Diff coverage report
Uncovered code

@takumihara
Copy link
Copy Markdown
Contributor Author

@alexey-milovidov Updated the tests.

@alexey-milovidov alexey-milovidov self-assigned this Mar 22, 2026
@alexey-milovidov alexey-milovidov added the pr-must-backport Pull request should be backported intentionally. Use this label with great care! label Mar 22, 2026
@alexey-milovidov alexey-milovidov merged commit aec3e79 into ClickHouse:master Mar 22, 2026
162 of 163 checks passed
@robot-clickhouse-ci-2 robot-clickhouse-ci-2 added the pr-synced-to-cloud The PR is synced to the cloud repo label Mar 22, 2026
@robot-clickhouse robot-clickhouse added the pr-must-backport-synced The `*-must-backport` labels are synced into the cloud Sync PR label Mar 22, 2026
@robot-clickhouse-ci-1 robot-clickhouse-ci-1 added the pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore label Mar 22, 2026
clickhouse-gh bot added a commit that referenced this pull request Mar 22, 2026
Backport #99542 to 26.2: Fix incorrect partition pruning for toWeek() function
clickhouse-gh bot added a commit that referenced this pull request Mar 22, 2026
Backport #99542 to 26.1: Fix incorrect partition pruning for toWeek() function
clickhouse-gh bot added a commit that referenced this pull request Mar 22, 2026
Backport #99542 to 26.3: Fix incorrect partition pruning for toWeek() function
clickhouse-gh bot added a commit that referenced this pull request Mar 22, 2026
Backport #99542 to 25.8: Fix incorrect partition pruning for toWeek() function
alexey-milovidov added a commit that referenced this pull request Mar 23, 2026
Backport #99542 to 25.12: Fix incorrect partition pruning for toWeek() function
alexey-milovidov added a commit that referenced this pull request Mar 23, 2026
Backport #99542 to 25.3: Fix incorrect partition pruning for toWeek() function
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-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore pr-bugfix Pull request with bugfix, not backported by default pr-must-backport Pull request should be backported intentionally. Use this label with great care! pr-must-backport-synced The `*-must-backport` labels are synced into the cloud Sync PR pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Combination of toYYYYMM(date) partitioning and toWeek(date, 3) filtering returns incorrect results

7 participants