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


##########
superset/common/query_context_processor.py:
##########
@@ -241,6 +241,12 @@ def get_df_payload_result(
             self._resolve_forced_query(query_obj, cache_key)
             or timeout == CACHE_DISABLED_TIMEOUT
         )
+        if query_obj:
+            # Datasources that run their own caching (semantic containment)
+            # must honor the same resolved chart/custom timeout and force
+            # decision as the result cache, not the datasource-level default.
+            query_obj.force_query = force_query

Review Comment:
   Contribution totals bypass `get_df_payload_result` and dispatch directly 
from `ensure_totals_available`, so this resolved force/timeout state is never 
applied to their query. A contribution chart with `cache_timeout=-1` can 
therefore use a cached totals denominator while its main query bypasses 
caching. Could the same resolved values be applied before the totals dispatch?



##########
superset/charts/schemas.py:
##########
@@ -1806,6 +1807,17 @@ class ChartDataResponseResult(Schema):
         required=True,
         allow_none=None,
     )
+    semantic_cache_status: fields.String = fields.String(

Review Comment:
   This unconditional response field is absent from 
`docs/static/resources/openapi.json`. The required OpenAPI drift workflow 
regenerates that file and will detect the change, leaving generated clients 
without the new response property. Could the generated spec be refreshed with 
this schema change?



##########
superset/semantic_layers/mapper.py:
##########
@@ -147,8 +191,24 @@ def get_results(query_object: QueryObject) -> QueryResult:
 
     # Step 2: Execute the main query (first in the list)
     main_query = queries[0]
-    main_result = dispatcher(main_query)
+    main_outcome: SemanticCacheOutcome = _dispatch_semantic_query(
+        query_object.datasource,
+        dispatcher,
+        main_query,
+        force=query_object.force_query,
+        # Row-count results share the query's logical identity but not its
+        # shape: a cached table result would satisfy a row-count lookup (and
+        # vice versa) and break server-side pagination. Containment reuse is
+        # only defined over tabular results, so row-count dispatches bypass
+        # the cache entirely.
+        cacheable=not query_object.is_rowcount,

Review Comment:
   Only row-count queries bypass storage here; server-paginated data pages with 
a nonzero `row_offset` are still stored. `select_reuse` rejects every 
descriptor whose query or entry offset is nonzero, so those pages can never be 
reused and repeated pages evict usable zero-offset descriptors from the 
128-entry bucket. Could offset pages bypass containment as well?



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