Disable trivial LIMIT optimization with row policies/additional_table_filters#102921
Merged
azat merged 10 commits intoMay 4, 2026
Merged
Conversation
Contributor
|
Workflow [PR], commit [2fc5a4d] Summary: ❌
AI ReviewSummaryThis PR makes ClickHouse Rules
Final VerdictStatus: ✅ Approve |
58c8b91 to
5a98719
Compare
5a98719 to
f01b910
Compare
…_filters This caused single-threaded index analysis when row policies were active, even though the row policy filter needs to scan many parts.
Split `buildAdditionalFiltersIfNeeded` into a pure `parseAdditionalFilterAstIfNeeded` (parse + match + assign `additional_filter_ast`) and a side-effectful builder (actions DAG + prewhere column restoration + PreparedSets registration). The parse is now invoked once, early — right after `table_expression_query_info` is prepared — so that `additional_filter_ast` is populated before both the trivial-count and trivial-limit decisions. Previously both checks read a field that was only assigned later inside `buildAdditionalFiltersIfNeeded`, making the per-table `additional_filter_ast` checks effectively dead. As a consequence: - The trivial-limit optimization now correctly treats a per-table `additional_table_filters` match the same way it treats a matching row policy (introduced in 7479c7c). - The pessimistic outer guard that disabled trivial-count whenever *any* `additional_table_filters` entry was present is dropped — the per-table check inside `applyTrivialCountIfPossible` is now live and precise, so the optimization fires again when the filter targets a different table. The existing two call sites of the (now trimmed) `buildAdditionalFiltersIfNeeded` keep their ordering and semantics; they simply read the already-parsed AST. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Repro: 02346_additional_filters_distr
`select * from dist_02346 order by x
settings additional_table_filters={'dist_02346' : 'x > 3 and x < 7'}`
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mirror the check in buildRowPolicyFilterIfNeeded: a row policy that evaluates to always-true adds no predicate, so it should not prevent the trivial LIMIT optimization. Without this, queries where row policy combines to always-true would lose multi-threaded reading unnecessarily. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
a1ad870 to
2a09714
Compare
2a09714 to
adea145
Compare
Share the effective row-policy filter check across applyTrivialCountIfPossible, buildRowPolicyFilterIfNeeded, and the trivial-LIMIT gate so the three sites agree on whether a row policy applies. Previously the early trivial-LIMIT check could drift from the later planning logic, leaving trivial-LIMIT incorrectly firing (or not firing) in edge cases. The helper also guards getDatabaseName with hasDatabase to handle storages with empty database (e.g. Distributed remote queries). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Member
Author
|
Unrelated failures |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
LLVM Coverage Report
Changed lines: 90.41% (66/73) | lost baseline coverage: 1 line(s) · Uncovered code |
robot-ch-test-poll4
added a commit
that referenced
this pull request
May 5, 2026
Cherry pick #102921 to 25.8: Disable trivial LIMIT optimization with row policies/additional_table_filters
robot-clickhouse
added a commit
that referenced
this pull request
May 5, 2026
… policies/additional_table_filters
robot-ch-test-poll4
added a commit
that referenced
this pull request
May 5, 2026
Cherry pick #102921 to 26.2: Disable trivial LIMIT optimization with row policies/additional_table_filters
robot-clickhouse
added a commit
that referenced
this pull request
May 5, 2026
… policies/additional_table_filters
robot-ch-test-poll4
added a commit
that referenced
this pull request
May 5, 2026
Cherry pick #102921 to 26.3: Disable trivial LIMIT optimization with row policies/additional_table_filters
robot-clickhouse
added a commit
that referenced
this pull request
May 5, 2026
… policies/additional_table_filters
robot-ch-test-poll3
added a commit
that referenced
this pull request
May 5, 2026
Cherry pick #102921 to 26.4: Disable trivial LIMIT optimization with row policies/additional_table_filters
robot-clickhouse
added a commit
that referenced
this pull request
May 5, 2026
… policies/additional_table_filters
azat
added a commit
that referenced
this pull request
May 6, 2026
Backport #102921 to 26.2: Disable trivial LIMIT optimization with row policies/additional_table_filters
azat
added a commit
that referenced
this pull request
May 6, 2026
Backport #102921 to 26.4: Disable trivial LIMIT optimization with row policies/additional_table_filters
azat
added a commit
that referenced
this pull request
May 6, 2026
Backport #102921 to 26.3: Disable trivial LIMIT optimization with row policies/additional_table_filters
azat
added a commit
that referenced
this pull request
May 6, 2026
Backport #102921 to 25.8: Disable trivial LIMIT optimization with row policies/additional_table_filters
azat
added a commit
to azat/ClickHouse
that referenced
this pull request
May 7, 2026
…yzer The problem was that in ClickHouse#102921 the check for trivial count optimization has been changed to respect only related tables in additional_table_filters, but this broke queries with parallel replicas and analyzer because analyzer sends query with fully qualified database.table always, and so if you filter contains non-qualified table name, then it will be ignored. Fixes: ClickHouse#104219
azat
added a commit
to azat/ClickHouse
that referenced
this pull request
May 8, 2026
…allel replicas and analyzer The problem was that in ClickHouse#102921 the check for trivial count optimization has been changed to respect only related tables in additional_table_filters, but this changes the behavior for parallel replicas with non default database, since now followers do not match additional_table_filters (because analyze sends qualified table names), and this uncover another bug in trivial count optimization, that simply ignores the fact that this replica is follower and respond with the total count, and then initiator accumulate everything, which is wrong. But most of the time the bug was not reproduced since the cancellation of queries on followers was fast enough to cancel them once all ranges has been served. Fixes: ClickHouse#104219
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):
Disable trivial LIMIT optimization with row policies/additional_table_filters (to allow parallel index analysis)
This caused single-threaded index analysis when row policies were active, even though the row policy filter needs to scan many parts.
Note: marked as
Bug Fixsince it is a regression between old analyzer and new