Skip to content

fix: UI cache toggle should disable all cache layers - #19774

Open
jtuglu1 wants to merge 1 commit into
apache:masterfrom
jtuglu1:web-console/cache-toggle-full-disable
Open

fix: UI cache toggle should disable all cache layers#19774
jtuglu1 wants to merge 1 commit into
apache:masterfrom
jtuglu1:web-console/cache-toggle-full-disable

Conversation

@jtuglu1

@jtuglu1 jtuglu1 commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Summary

  • The web console "Use cache" toggle in the run panel only set useCache/populateCache in the query context, leaving result-level and forwarded result-level cache settings unaffected.
  • Extends the toggle handler to also set useResultLevelCache, populateResultLevelCache, useForwardedResultLevelCache, and populateForwardedResultLevelCache, so disabling cache from the UI actually disables all cache layers.
  • Adds the new keys to the QueryContext type definition.

Test plan

  • tsc --noEmit passes for web-console
  • Manually verify in the console: toggling "Use cache" off/on in the run panel's settings menu and confirming the resulting query context includes all six cache-related keys set to the same boolean value

The "Use cache" toggle in the run panel only set useCache/populateCache,
leaving result-level and forwarded result-level cache context keys
untouched. Extend the toggle to also set useResultLevelCache,
populateResultLevelCache, useForwardedResultLevelCache, and
populateForwardedResultLevelCache so disabling cache from the UI
actually disables all cache layers.
@jtuglu1 jtuglu1 changed the title Web console: cache toggle should disable all cache layers fix: cache toggle should disable all cache layers Jul 27, 2026
@jtuglu1
jtuglu1 force-pushed the web-console/cache-toggle-full-disable branch from 49bec51 to d71c1a8 Compare July 27, 2026 21:45
@jtuglu1 jtuglu1 changed the title fix: cache toggle should disable all cache layers fix: UI cache toggle should disable all cache layers Jul 27, 2026
@jtuglu1
jtuglu1 requested a review from vogievetsky July 28, 2026 02:14

@FrankChen021 FrankChen021 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity Findings
P0 0
P1 1
P2 1
P3 0
Total 2

Legacy queries saved with only the pre-existing cache flags remain result-cache-enabled, and the newly emitted forwarded-cache keys have no server-side consumer.

Reviewed 2 of 2 changed files.


This is an automated review by Codex GPT-5.6-Sol

...queryContext,
useCache,
populateCache: useCache,
useResultLevelCache: useCache,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Normalize legacy disabled cache contexts

The new flags are written only after an onValueChange event. Queries saved by the previous UI already contain useCache=false/populateCache=false but lack these result-level keys; value={useCache} still displays Disabled, while execution leaves both result-level flags absent. Druid defaults absent useResultLevelCache/populateResultLevelCache values to true, so an enabled broker result cache remains active. Normalize legacy disabled contexts or submit false result-level flags whenever useCache is false, without requiring users to toggle on and off again.

populateCache: useCache,
useResultLevelCache: useCache,
populateResultLevelCache: useCache,
useForwardedResultLevelCache: useCache,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Do not emit unsupported forwarded-cache keys

These two forwarded result-level keys have no server-side consumer: their only repository occurrences are the newly added UI/type fields, while Druid reads only useResultLevelCache and populateResultLevelCache. They therefore disable nothing. Additionally, when query-context authorization is enabled, SQL treats each user-supplied key as a QUERY_CONTEXT resource, so toggling this control can require permissions for meaningless keys and reject otherwise valid queries. Remove them, or add actual server-side support if a separate forwarded cache layer is intended.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants