mikebridge commented on PR #40221: URL: https://github.com/apache/superset/pull/40221#issuecomment-4898567658
Two more base-branch observations from the #41826 review panel, for this branch's backlog (neither introduced by the stacked PRs, neither urgent): 1. **Forward-looking cache-key design note**: the semantic cache's keys carry no user/RLS/impersonation context — correct today because semantic-view execution is user-agnostic, but it means *every* store path becomes a cross-user leak if semantic-layer engines ever gain per-user execution context (engine impersonation, per-user RLS in the underlying semantic platform). Core Superset handles this by folding RLS ids into cache keys. Worth a comment in `cache.py` documenting the user-agnostic assumption, so the constraint is visible when per-user execution is considered. 2. **Provenance gap on the cheapest hit path**: `_apply_post_processing` early-returns the cached payload as-is for EXACT-mode / no-leftovers / no-limit hits, skipping the `SemanticRequest(type="cache")` note — so `is_cached` reads `False` for exactly the most common cache hit, and the Explore "semantic smart cache" indicator won't show. One-line fix (append the provenance record before the early return). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
