mikebridge commented on code in PR #42760:
URL: https://github.com/apache/superset/pull/42760#discussion_r3876479934


##########
superset/semantic_layers/mapper.py:
##########
@@ -221,6 +284,7 @@ def get_results(query_object: QueryObject) -> QueryResult:
         semantic_result,
         query_object,
         duration,
+        semantic_cache_hit=main_outcome.cache_hit,

Review Comment:
   Confirmed and fixed in `f0e9e32685`. The per-query boolean could not 
represent it, so `semantic_cache_hit` is now `semantic_cache_status: HIT | MISS 
| MIXED` (`SemanticCacheStatus` in `superset/constants.py`), aggregated over 
the main **and every offset** dispatch in `get_results`. The 
`X-Superset-Semantic-Cache` header combines the per-query statuses with the 
same vocabulary: `HIT` only when every query was `HIT`, `MIXED` if any was 
`HIT` or `MIXED`.
   
   The pill renders only on `HIT` — a `MIXED` payload included a provider 
round-trip, so "loaded from semantic cache" would overstate it. Parametrized 
`mapper_test` covers all four hit/miss combinations; the header test covers 
`MIXED` and mixed lists.
   



##########
docs/developer_docs/extensions/contribution-types.md:
##########
@@ -291,3 +291,71 @@ class MySemanticLayer(SemanticLayer[MyConfig, 
MySemanticView]):
 - **Host context**: Original ID used as-is
 
 The decorator registers the class in the semantic layers registry, making it 
available in the UI for users to create connections. The `configuration_class` 
should be a Pydantic model that defines the fields needed to connect 
(credentials, project, database, etc.). Superset uses the model's JSON schema 
to render the configuration form dynamically.
+
+#### Semantic result containment caching
+
+Superset containment caching is experimental and off by default. A provider 
must
+explicitly opt in; the safe defaults leave caching with the provider and scope
+results to an execution context:
+
+```python
+from superset_core.semantic_layers.layer import (
+    SemanticCacheCapabilities,
+    SemanticCacheExecutionContext,
+    SemanticCacheIdentityMaterial,
+    SemanticCacheResponsibility,
+    SemanticCacheScope,
+)
+
+class MySemanticLayer(SemanticLayer[MyConfig, MySemanticView]):
+    semantic_cache_responsibility = SemanticCacheResponsibility.SUPERSET
+    semantic_cache_scope = SemanticCacheScope.EXECUTION_CONTEXT
+    semantic_cache_capabilities = SemanticCacheCapabilities(
+        comparisons=True,
+        membership=True,
+        nulls=True,
+        pattern_escape="\\",
+    )
+
+    def get_semantic_cache_provider_identity(self) -> 
SemanticCacheIdentityMaterial:
+        return SemanticCacheIdentityMaterial(
+            {"provider_version": "v1", "catalog": self.config.catalog}
+        )
+
+    def get_semantic_cache_context_identity(
+        self,
+        context: SemanticCacheExecutionContext,
+    ) -> SemanticCacheIdentityMaterial:
+        return SemanticCacheIdentityMaterial({"tenant": 
self.tenant_id(context)})
+```
+
+Identity material must be secret-free and include every provider setting that 
can
+change results. For execution-context scope, Superset also hashes the 
principal,
+roles, guest-token claims, and row-level-security cache key. Returning `None` 
from
+either identity method bypasses containment. Use `GLOBAL` only when results are
+provably identical across principals and tenants. Declare only filter 
capabilities
+whose provider semantics exactly match Superset's post-processing semantics.
+
+Operators enable both `SEMANTIC_LAYERS` and the development feature flag

Review Comment:
   Confirmed and fixed in `f0e9e32685`, in both directions: the docs now 
require a persistent shared `DATA_CACHE_CONFIG` with a Redis example next to 
the coordination one, and `initialize_semantic_cache` fails closed on a 
`NullCache` data cache (`UNSUPPORTED_BACKEND`, the same path as Sentinel) with 
a startup warning that names `DATA_CACHE_CONFIG`. An operator who enables the 
flag on the default config now sees the misconfiguration at boot instead of a 
0% hit rate.
   



##########
superset/semantic_layers/cache_host.py:
##########
@@ -0,0 +1,165 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Host adaptation for semantic cache identity and provider contracts."""
+
+import hashlib
+from collections.abc import Iterable, Mapping
+from datetime import datetime
+from typing import cast, Protocol
+
+from flask import g, has_request_context
+from superset_core.semantic_layers.layer import (
+    SemanticCacheCapabilities,
+    SemanticCacheExecutionContext,
+    SemanticCacheIdentityMaterial,
+    SemanticCacheResponsibility,
+    SemanticCacheScope,
+)
+
+from superset import security_manager
+from superset.connectors.sqla.models import BaseDatasource
+from superset.semantic_layers.cache_identity import (
+    SemanticCacheIdentityFactory,
+    SemanticViewIdentity,
+)
+from superset.semantic_layers.cache_policy import (
+    ContainmentCapabilities,
+    PatternSemantics,
+)
+from superset.semantic_layers.cache_repository import ViewMeta
+from superset.utils import json
+
+
+class _SemanticCacheProvider(Protocol):
+    semantic_cache_responsibility: SemanticCacheResponsibility
+    semantic_cache_scope: SemanticCacheScope
+    semantic_cache_capabilities: SemanticCacheCapabilities
+
+    def get_semantic_cache_provider_identity(
+        self,
+    ) -> SemanticCacheIdentityMaterial | None: ...  # pragma: no cover
+
+    def get_semantic_cache_context_identity(
+        self,
+        context: SemanticCacheExecutionContext,
+    ) -> SemanticCacheIdentityMaterial | None: ...  # pragma: no cover
+
+
+def _execution_context(
+    datasource: BaseDatasource,
+) -> SemanticCacheExecutionContext | None:
+    if not has_request_context() or not getattr(g, "user", None):
+        return None
+    user: object = g.user
+    principal: object = getattr(user, "id", None) or getattr(user, "username", 
None)
+    if principal is None:
+        return None
+    roles_value: object = getattr(user, "roles", ())
+    roles: Iterable[object] = (
+        roles_value
+        if isinstance(roles_value, Iterable) and not isinstance(roles_value, 
str)
+        else ()
+    )
+    role_ids: tuple[str, ...] = tuple(
+        sorted(
+            str(getattr(role, "id", None) or getattr(role, "name", ""))
+            for role in roles
+        )
+    )
+    guest_token: object = getattr(user, "guest_token", None)
+    rls_cache_key: list[str] = security_manager.get_rls_cache_key(datasource)
+    identity_payload: str = json.dumps(
+        {
+            "guest_token": guest_token,
+            "principal": str(principal),
+            "rls": rls_cache_key,
+            "roles": role_ids,
+        },
+        default=str,
+        separators=(",", ":"),
+        sort_keys=True,
+    )
+    host_identity: str = hashlib.sha256(identity_payload.encode()).hexdigest()
+    return SemanticCacheExecutionContext(str(principal), role_ids, 
host_identity)
+
+
+def build_cache_configuration(
+    datasource: BaseDatasource,
+) -> tuple[ViewMeta, ContainmentCapabilities] | None:
+    """Build safe host metadata when the provider explicitly opts in."""
+    layer: _SemanticCacheProvider = cast(
+        _SemanticCacheProvider,
+        datasource.semantic_layer.implementation,
+    )
+    if (
+        getattr(layer, "semantic_cache_responsibility", None)
+        is not SemanticCacheResponsibility.SUPERSET
+    ):
+        return None
+    provider_material: object = layer.get_semantic_cache_provider_identity()
+    if not isinstance(provider_material, SemanticCacheIdentityMaterial):
+        return None
+    scope: object = getattr(layer, "semantic_cache_scope", None)
+    scope_material: Mapping[str, object]
+    if scope is SemanticCacheScope.GLOBAL:
+        scope_material = {"scope": "global"}
+    elif scope is SemanticCacheScope.EXECUTION_CONTEXT:
+        context: SemanticCacheExecutionContext | None = 
_execution_context(datasource)
+        if context is None:
+            return None
+        context_material: object = 
layer.get_semantic_cache_context_identity(context)
+        if not isinstance(context_material, SemanticCacheIdentityMaterial):
+            return None
+        scope_material = {
+            "host_identity": context.host_identity,
+            "provider_identity": context_material.values,
+        }
+    else:
+        return None
+    provider_capabilities: object = getattr(layer, 
"semantic_cache_capabilities", None)
+    if not isinstance(provider_capabilities, SemanticCacheCapabilities):
+        return None
+    pattern_semantics: PatternSemantics | None = (
+        PatternSemantics.sql_like(escape=provider_capabilities.pattern_escape)
+        if provider_capabilities.pattern_escape is not None
+        else None
+    )
+    capabilities: ContainmentCapabilities = ContainmentCapabilities(
+        comparisons=provider_capabilities.comparisons,
+        membership=provider_capabilities.membership,
+        nulls=provider_capabilities.nulls,
+        pattern_semantics=pattern_semantics,
+    )
+    changed_on: object = getattr(datasource, "changed_on", None)
+    definition_material: dict[str, object] = {
+        "changed_on": changed_on.isoformat()
+        if isinstance(changed_on, datetime)
+        else str(changed_on),
+    }
+    meta: ViewMeta = ViewMeta(
+        view_identity=SemanticViewIdentity(str(datasource.uuid)),
+        definition_identity=SemanticCacheIdentityFactory.definition(
+            definition_material
+        ),
+        provider_identity=SemanticCacheIdentityFactory.provider(
+            provider_material.values
+        ),
+        scope_identity=SemanticCacheIdentityFactory.scope(scope_material),
+        timeout=datasource.cache_timeout,

Review Comment:
   Confirmed and fixed in `f0e9e32685` — and it was slightly worse than 
described: the factory only put the *request* `force` on the `QueryObject`, so 
the processor's resolved `force_query` (which folds in `timeout == -1`) and the 
resolved timeout never reached containment at all. `get_df_payload_result` now 
writes both onto the `QueryObject`, and `build_cache_configuration(..., 
cache_timeout=)` uses the resolved value both for the `-1` bypass (nothing 
read, nothing stored) and as the TTL of stored values, so a chart's own timeout 
bounds reuse. Covered in `cache_host_test`, `cache_integration_test` (store 
untouched by `-1` requests between cacheable ones), and 
`test_query_context_processor_timing`.
   
   One follow-on from a data-systems pass over that change, fixed in 
`7eb97a33a3`: with per-request TTLs, one view's descriptor bucket indexes 
values with different lifetimes, and the bucket was written with the *latest* 
request's timeout — a 60 s chart storing after a 3600 s chart would expire the 
index out from under the longer-lived value (orphaned until its own TTL, extra 
misses; never wrong data, since value keys are content-addressed). Descriptors 
now carry their TTL and the bucket expires no sooner than its longest-lived 
value. `IDENTITY_FORMAT_VERSION` is bumped to v3 because `CachedEntry` is 
pickled.
   



-- 
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]

Reply via email to