iframe-proxy

vvo · 2026-03-17T20:22:29Z

Implements finer-grained subquery cache propagation control, as requested in the review of #76252 by @alexey-milovidov.

use_query_cache on the outer query no longer auto-propagates to subqueries. Three rules implemented in shouldUseQueryCacheForSubquery:

No auto-propagation by default — use_query_cache on the outer query does NOT propagate to subqueries
Explicit per-subquery opt-in — SELECT * FROM (SELECT ... SETTINGS use_query_cache = true) enables caching for that specific subquery
Bulk propagation — query_cache_for_subqueries = true enables propagation into all subqueries

Also fixes two issues from #76252:

Config key mismatch: the rename from QueryCache → QueryResultCache changed keys in Context.cpp but not in config files, breaking SYSTEM RELOAD CONFIG for cache settings
All 03381_* tests now explicitly SET enable_analyzer = 1 since Planner-level subquery caching is new-analyzer only

Changelog category (leave one):

Improvement

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

Individual subquery results can now be cached independently using SETTINGS use_query_cache = true on specific subqueries, without caching the entire outer query. A new setting query_cache_for_subqueries = true enables bulk propagation of use_query_cache into all subqueries. Note: use_query_cache on the outer query no longer auto-propagates to subqueries.

Documentation entry for user-facing changes

Documentation is written (mandatory for new features)

A new "Subquery Caching" section has been added to the query cache documentation explaining the three propagation modes with examples.

Version info

Merged into: 26.5.1.844

clickhouse-gh · 2026-03-18T02:20:15Z

alexey-milovidov · 2026-03-18T08:20:05Z

Thanks! This is exactly what we need!
Please check test failures and update accordingly.

vvo · 2026-03-18T16:10:08Z

@alexey-milovidov Here's the status on implementing your feedback for finer-grained subquery
cache propagation.

The three rules from your feedback are implemented in
shouldUseQueryCacheForSubquery:

No auto-propagation by default
Explicit SETTINGS use_query_cache = true on a subquery enables caching for that subquery
query_cache_for_subqueries = true enables bulk propagation

Along the way we also fixed two issues from the base PR (#76252):

The rename from QueryCache → QueryResultCache changed config keys in
Context.cpp from query_cache.* to query_result_cache.*, but config files still use query_cache.*
— so SYSTEM RELOAD CONFIG was silently ignored. This broke test_query_cache_size_is_runtime_configurable.
All 03381_* tests now explicitly SET enable_analyzer = 1 since the Planner-level subquery caching is new-analyzer only.

Does the overall approach look right to you?

vvo · 2026-03-20T00:30:52Z

@rschu1ze @alexey-milovidov I think we're done here, let me know!

vvo · 2026-04-01T14:10:31Z

@rschu1ze @alexey-milovidov do we still want this?

rschu1ze · 2026-04-01T14:28:56Z

Yes. Haven't had time to check yet, sorry.

alexey-milovidov · 2026-04-07T00:22:40Z

The Stress test (arm_msan) failure is fixed by #101239, which should be merged first. After it is merged, please update the branch to include the fix.

alexey-milovidov · 2026-04-07T19:47:30Z

The failures of "Flaky check" in "functions_bad_arguments" will be fixed by #101994.

Address review feedback on PR ClickHouse#99804. The error messages thrown by `checkCanWriteQueryResultCache` for queries containing non-deterministic functions or system tables had the awkward "regardless or to omit caching" phrasing. Reword to "regardless, or omit caching" for clarity. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Address review feedback on PR ClickHouse#99804. `removeTableAliases` used `starts_with("__table")` to strip planner-generated table aliases from the AST before computing the subquery cache key. Any user-visible identifier starting with the same prefix (e.g. `__table_prod`) was incorrectly normalized away, so a query like `SELECT x FROM __table_prod` could collide with `SELECT x FROM prod` in the subquery cache and return wrong results. The analyzer's `createUniqueAliasesIfNecessary` generates aliases of the exact form `__table<N>` where `N` is a non-empty sequence of digits (`__table1`, `__table2`, etc.). Restrict the alias-stripping check to that exact pattern via a new `isPlannerGeneratedTableAlias` helper. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…vior After commit abf0a27 gated `query_cache_for_subqueries` Planner-level caching on `is_subquery`, the top-level query is no longer recorded under the `is_subquery = 1` namespace — only actual subqueries are. The 03381_* test references and SQL comments were still asserting the old "outer + inner" count and so the Fast test was failing on all 10 affected tests. - Update test references to match the new actual outputs. - Update misleading SQL comments that referred to the outer node as a subquery. - Move `query_cache_for_subqueries` from the `26.4` to the `26.5` section of `SettingsChangesHistory.cpp` so the upgrade-from-26.4 check in `02995_new_settings_history` recognises it. CI report: ClickHouse#99804 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

alexey-milovidov · 2026-05-18T01:52:27Z

 }

-IASTHash calculateASTHash(ASTPtr ast, const String & current_database, const Settings & settings)
+IASTHash calculateAstHash(ASTPtr ast, const String & current_database, const Settings & settings, const bool is_subquery, const bool already_cloned = false)


Our code style demands capital letters for every abbreviated word. AST, not Ast.

Renamed in 47aecf5 — calculateAstHash → calculateASTHash and calculateAstHashAndQueryString → calculateASTHashAndQueryString.

alexey-milovidov

A few rough edges, but mostly good.

CLAassistant · 2026-05-19T03:23:28Z

All committers have signed the CLA.

Address review feedback on PR ClickHouse#99804. The error messages thrown by `checkCanWriteQueryResultCache` for queries containing non-deterministic functions or system tables had the awkward "regardless or to omit caching" phrasing. Reword to "regardless, or omit caching" for clarity. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Address review feedback on PR ClickHouse#99804. `removeTableAliases` used `starts_with("__table")` to strip planner-generated table aliases from the AST before computing the subquery cache key. Any user-visible identifier starting with the same prefix (e.g. `__table_prod`) was incorrectly normalized away, so a query like `SELECT x FROM __table_prod` could collide with `SELECT x FROM prod` in the subquery cache and return wrong results. The analyzer's `createUniqueAliasesIfNecessary` generates aliases of the exact form `__table<N>` where `N` is a non-empty sequence of digits (`__table1`, `__table2`, etc.). Restrict the alias-stripping check to that exact pattern via a new `isPlannerGeneratedTableAlias` helper. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Address review feedback on PR ClickHouse#99804. `shouldUseQueryCacheForSubquery` was returning `true` for the top-level query when both `use_query_cache = 1` and `query_cache_for_subqueries = 1` were set. That caused the top-level query to be cached twice: once by `executeQuery` under the `is_subquery = false` key, and once again by the Planner under the `is_subquery = true` key. Gate the `query_cache_for_subqueries` propagation branch on `is_subquery` so only real subqueries enter the Planner-level cache namespace, matching the setting's contract. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…vior After commit abf0a27 gated `query_cache_for_subqueries` Planner-level caching on `is_subquery`, the top-level query is no longer recorded under the `is_subquery = 1` namespace — only actual subqueries are. The 03381_* test references and SQL comments were still asserting the old "outer + inner" count and so the Fast test was failing on all 10 affected tests. - Update test references to match the new actual outputs. - Update misleading SQL comments that referred to the outer node as a subquery. - Move `query_cache_for_subqueries` from the `26.4` to the `26.5` section of `SettingsChangesHistory.cpp` so the upgrade-from-26.4 check in `02995_new_settings_history` recognises it. CI report: ClickHouse#99804 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Implements finer-grained subquery cache propagation control, as requested in the review of ClickHouse#76252. `use_query_cache` on the outer query no longer auto-propagates to subqueries. Three rules implemented in `shouldUseQueryCacheForSubquery`: 1. No auto-propagation by default: `use_query_cache` on the outer query does not propagate to subqueries. 2. Explicit per-subquery opt-in: `SELECT * FROM (SELECT ... SETTINGS use_query_cache = true)` enables caching for that specific subquery. 3. Bulk propagation: `query_cache_for_subqueries = true` enables propagation into all subqueries. Also fixes two issues from ClickHouse#76252: - Config key mismatch: the rename `QueryCache` -> `QueryResultCache` changed keys in `Context.cpp` but not in config files, breaking `SYSTEM RELOAD CONFIG` for cache settings. - All `03381_*` tests now explicitly `SET enable_analyzer = 1` since Planner-level subquery caching is new-analyzer only. Co-authored-by: Баранник Никита Павлович <npbarannik@edu.hse.ru> Co-authored-by: Robert Schulze <robert@clickhouse.com> Co-authored-by: Alexey Milovidov <milovidov@clickhouse.com>

clickhouse-gh · 2026-05-19T04:40:25Z

+    /// When should_cache is true but the outer query didn't set use_query_cache (explicit subquery opt-in),
+    /// skip the context flag check in checkCanWriteQueryResultCache while still respecting safety checks.
+    bool skip_context_check = should_cache && !can_use_query_result_cache;
+    if (should_cache && checkCanWriteQueryResultCache(ast, query_context, skip_context_check))


This write path can cache subquery results even when the outer query has use_query_cache = 0 (explicit per-subquery opt-in uses skip_context_check = true). In that case we bypass the non-throw overflow-mode guard that only exists in executeQuery.

That reintroduces the truncated-result risk from bug 67476 for subqueries: a subquery can run with *_overflow_mode != 'throw', produce partial output, and still be written into the cache.

Could we apply the same overflow-mode precondition here (or inside checkCanWriteQueryResultCache) before adding StreamInQueryResultCacheStep?

clickhouse-gh · 2026-05-19T07:44:24Z

LLVM Coverage Report

Metric	Baseline	Current	Δ
Lines	84.10%	84.20%	+0.10%
Functions	91.40%	91.40%	+0.00%
Branches	76.60%	76.60%	+0.00%

Changed lines: 94.38% (302/320) | lost baseline coverage: 9 line(s) · Uncovered code

Full report · Diff report

clickgapai · 2026-05-21T15:12:40Z

Hi @vvo @alexey-milovidov — while reviewing this PR I found the following:

Bug (P1): Subquery caching missing overflow mode check allows caching truncated results — [CI Proof] Subquery caching missing overflow mode check allows caching truncated results #105549

Happy to discuss — close anything that's wrong or already addressed.

clickgapai · 2026-05-21T20:24:12Z

vvo marked this pull request as ready for review March 17, 2026 20:26

vvo mentioned this pull request Mar 17, 2026

Query result caching for sub-queries #76252

Closed

1 task

alexey-milovidov added the can be tested Allows running workflows for external contributors label Mar 18, 2026

clickhouse-gh Bot added the pr-improvement Pull request with some product improvements label Mar 18, 2026

vvo force-pushed the subquery-cache-propagation branch from d6f0b0a to 107a40c Compare March 18, 2026 10:49