Skip to content

Better query condition cache coverage for ORDER BY ... LIMIT k queries#110507

Merged
alexey-milovidov merged 8 commits into
ClickHouse:masterfrom
shankar-iyer:fix_topk_qcc_clickbench
Jul 18, 2026
Merged

Better query condition cache coverage for ORDER BY ... LIMIT k queries#110507
alexey-milovidov merged 8 commits into
ClickHouse:masterfrom
shankar-iyer:fix_topk_qcc_clickbench

Conversation

@shankar-iyer

@shankar-iyer shankar-iyer commented Jul 15, 2026

Copy link
Copy Markdown
Member

This PR makes two improvements to how the query condition cache (QCC) interacts with the TopK dynamic filtering optimization for ORDER BY ... LIMIT k queries.

  1. Enable query condition cache for ORDER BY ... LIMIT n queries #104478 enabled the QCC for queries that go through TopK dynamic filtering, but it did not handle queries that are also rewritten by lazy materialization — the QCC entry was never populated in that case. Lazy materialization replaces the FilterStep that carried the QCC key, so this PR re-runs the updateQueryConditionCache pass after lazy materialization to re-attach the key to the surviving FilterStep.

  2. A query selected for TopK dynamic filtering can now reuse QCC entries written by a non-TopK query that had the same predicate. The TopK query's WHERE condition is and(__topKFilter(...), <predicate>); it now also probes the cache under the bare <predicate> hash, so entries written by a plain SELECT ... WHERE <predicate> are reused. The TopK query still writes its own entries under a key salted with the TopK plan parameters, so a plain query (or a TopK query with a different LIMIT / sort key) never reads a TopK-salted entry.

Related: #104478

Changelog category

  • Improvement

Changelog entry

  • Queries accelerated by TopK dynamic filtering (ORDER BY ... LIMIT k) now make fuller use of the query condition cache: the cache is populated even when lazy materialization is applied to the query, and such queries can reuse entries previously written by an ordinary query that had the same WHERE predicate.
    ...

Version info

  • Merged into: 26.7.1.1131 (included in 26.7 and later)

@clickhouse-gh

clickhouse-gh Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Workflow [PR], commit [8137b02]

Summary:


AI Review

Summary

This PR reattaches query-condition-cache keys after lazy materialization rewrites the surviving storage FilterStep, and lets TopK-filtered ORDER BY ... LIMIT k reads reuse plain-predicate query-condition-cache entries while keeping TopK-salted writes isolated from ordinary queries. I re-read the current diff and prior review discussion, and I did not find any remaining correctness, safety, or regression-test issues in the current code.

Final Verdict

✅ No blocking findings.

@shankar-iyer shankar-iyer changed the title Fix topk qcc clickbench Better query condition cache coverage for ORDER BY ... LIMIT k queries Jul 15, 2026
@clickhouse-gh clickhouse-gh Bot added the pr-improvement Pull request with some product improvements label Jul 15, 2026
Comment thread src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp Outdated
Comment thread src/Processors/QueryPlan/Optimizations/optimizeTree.cpp
Comment thread tests/queries/0_stateless/04539_query_condition_cache_topk_reuse_where.sql Outdated
@shankar-iyer

Copy link
Copy Markdown
Member Author

Waiting for perf run with ClickBench.

@shankar-iyer shankar-iyer added the pr-performance Pull request with some performance improvements label Jul 16, 2026
@clickhouse-gh

clickhouse-gh Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

📊 Cloud Performance Report

🟢 AI verdict: improvement1 query(s) improved out of 37 analysed

This PR fixes query-condition-cache population and reuse for TopK dynamic filtering plus lazy materialization (WHERE + ORDER BY LIMIT reads). Only clickbench Q23 shows a work-backed win: CPU fell from ~1545 to ~504 ms alongside a ×2.2 faster wall time, consistent with the fixed granule-skip path. Q4, Q15 and Q31 improved in wall time only, with flat or rising CPU and no selective WHERE/TopK to exercise the change, so they were kept as no change or downgraded to not sure as co-location/cache variance. The envelope suppression on Q4 looks correct.

clickbench

🟢 1 improved · ⚠️ 2 inconclusive

Flagged queries (3 of 43)
Query Verdict Baseline median (ms) PR median (ms) Change q-value Hint
🟢 23 improvement 164 73 ×2.2 faster <0.0001 cache: This WHERE + ORDER BY LIMIT read is exactly the TopK/lazy-materialization path the PR fixes; CPU work fell from ~1545 to ~504 ms, so ×2.2 faster is real and work-backed.
⚠️ 15 not_sure 243 188 -22.6% <0.0001 aggregation: Aggregation with no selective WHERE, so the QCC/TopK fix can't apply; total CPU is flat (2613 vs 2614 ms) while wall dropped, pointing to co-location cache variance.
⚠️ 31 not_sure 364 325 -10.6% <0.0001 aggregation: Aggregation with no selective WHERE, outside the QCC granule-skip path; CPU rose (3539 to 3639 ms) while wall fell, so the wall gain is co-location variance, not the PR.

Change = percent below ×2; the ratio of medians (×N faster/slower) beyond, where percent understates the scale. q-value = BH-FDR adjusted p; smaller is stronger evidence. MIRAI flags a query when q < fdr_q (default 0.10) — the value the verdict is based on.

tpch_adapted_1_official

🟢 No significant changes

Debug info
  • StressHouse run: d61a5ede-2aed-454a-906c-da3829b8597e
  • MIRAI run: ee4c50b1-182e-443f-b536-2948093afd3f
  • PR check IDs:
    • clickbench_718772_1784195137
    • clickbench_718785_1784195137
    • clickbench_718791_1784195137
    • tpch_adapted_1_official_718797_1784195137
    • tpch_adapted_1_official_718803_1784195137
    • tpch_adapted_1_official_718810_1784195137

@shankar-iyer

Copy link
Copy Markdown
Member Author

📊 Cloud Performance Report

🟢 AI verdict: improvement1 query(s) improved out of 37 analysed

This PR fixes query-condition-cache population and reuse for TopK dynamic filtering plus lazy materialization (WHERE + ORDER BY LIMIT reads). Only clickbench Q23 shows a work-backed win: CPU fell from ~1545 to ~504 ms alongside a ×2.2 faster wall time, consistent with the fixed granule-skip path. Q4, Q15 and Q31 improved in wall time only, with flat or rising CPU and no selective WHERE/TopK to exercise the change, so they were kept as no change or downgraded to not sure as co-location/cache variance. The envelope suppression on Q4 looks correct.

clickbench

🟢 1 improved · ⚠️ 2 inconclusive

Flagged queries (3 of 43)
Query Verdict Baseline median (ms) PR median (ms) Change q-value Hint
🟢 23 improvement 164 73 ×2.2 faster <0.0001 cache: This WHERE + ORDER BY LIMIT read is exactly the TopK/lazy-materialization path the PR fixes; CPU work fell from ~1545 to ~504 ms, so ×2.2 faster is real and work-backed.
⚠️ 15 not_sure 243 188 -22.6% <0.0001 aggregation: Aggregation with no selective WHERE, so the QCC/TopK fix can't apply; total CPU is flat (2613 vs 2614 ms) while wall dropped, pointing to co-location cache variance.
⚠️ 31 not_sure 364 325 -10.6% <0.0001 aggregation: Aggregation with no selective WHERE, outside the QCC granule-skip path; CPU rose (3539 to 3639 ms) while wall fell, so the wall gain is co-location variance, not the PR.
Change = percent below ×2; the ratio of medians (×N faster/slower) beyond, where percent understates the scale. q-value = BH-FDR adjusted p; smaller is stronger evidence. MIRAI flags a query when q < fdr_q (default 0.10) — the value the verdict is based on.

tpch_adapted_1_official

🟢 No significant changes

Debug info

Q23 was the motivating factor for this PR.

@clickhouse-gh

clickhouse-gh Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

LLVM Coverage Report

Metric Baseline Current Δ
Lines 85.90% 85.90% +0.00%
Functions 92.70% 92.70% +0.00%
Branches 78.10% 78.00% -0.10%

Changed lines: Changed C/C++ lines covered: 88/94 (93.62%) · Uncovered code

Full report · Diff report

@shankar-iyer

Copy link
Copy Markdown
Member Author

Rerunning the failed jobs.

@shankar-iyer

Copy link
Copy Markdown
Member Author

Green CI right now at commit 8137b02

@alexey-milovidov alexey-milovidov self-assigned this Jul 18, 2026
@alexey-milovidov
alexey-milovidov added this pull request to the merge queue Jul 18, 2026
Merged via the queue into ClickHouse:master with commit bd20703 Jul 18, 2026
521 of 525 checks passed
@robot-ch-test-poll4 robot-ch-test-poll4 added the pr-synced-to-cloud The PR is synced to the cloud repo label Jul 18, 2026
alexey-milovidov added a commit that referenced this pull request Jul 22, 2026
Replace the hard-coded disable from the previous commit with a proper
setting, `use_query_condition_cache_for_top_k` (default `false`), that
enables/disables the query condition cache (QCC) for queries going through
the TopK dynamic-filtering plan (`ORDER BY <column> LIMIT n`). The feature
(added in PR #104478 and extended in PR #110507) is thus disabled by default
without removing it or its tests.

When the setting is off, each QCC touch point for TopK reads restores the
pre-#104478 behavior; when it is on, the current master behavior (salted QCC
entries) is used. The gated points are:

- `updateQueryConditionCache`: skip the QCC write for the WHERE filter of a
  TopK read.
- `ReadFromMergeTree::setTopKColumn`: turn off `use_query_condition_cache` in
  the reader settings for the read.
- `ReadFromMergeTree::selectRangesToRead`: do not record index-analysis QCC
  exclusions for TopK reads.
- `MergeTreeDataSelectExecutor::filterPartsByQueryConditionCache`: consult the
  WHERE condition with the plain (unsalted) hash and skip the predicate-only
  reuse path.

The stateless and performance tests for the feature are kept; each now sets
`use_query_condition_cache_for_top_k = 1` so it still exercises the feature.
The new setting is registered in `SettingsChangesHistory` for 26.7.

Related: #104478
Related: #110507

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
alexey-milovidov added a commit that referenced this pull request Jul 24, 2026
…condition-cache behavior

The 26.7 stable release (v26.7.1.1315-stable) shipped #104478/#110507 with
TopK (`ORDER BY ... LIMIT n`) reads participating in the query condition
cache unconditionally, so the `SettingsChangesHistory` entry for
`use_query_condition_cache_for_top_k` must record `previous_value = true`:
`SET compatibility = '26.7'` on 26.8+ then re-enables the gate instead of
silently disabling the behavior for pinned compatibility profiles.

Adds `04631_query_condition_cache_topk_compatibility` asserting the default
is off and that `compatibility = '26.7'` flips it on while
`compatibility = '26.8'` keeps it off.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-improvement Pull request with some product improvements pr-performance Pull request with some performance improvements 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.

3 participants