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


##########
superset/semantic_layers/models.py:
##########
@@ -786,9 +786,11 @@ def result_cache_discriminator(self) -> tuple[str, str] | 
None:
 
     def get_extra_cache_keys(self, query_obj: QueryObjectDict) -> 
list[Hashable]:
         discriminator: tuple[str, str] | None = self.result_cache_discriminator
-        if discriminator is None:
-            return []
-        return [("semantic-result-version", *discriminator)]
+        # Separate cached pre-normalization results during rolling deployments.
+        keys: list[Hashable] = ["semantic-null-filters-v1"]

Review Comment:
   This marker only changes the semantic view's own cache key. A native chart 
that embeds a chart-backed annotation layer sourced from a semantic chart 
filtered on NULL builds its key in `_annotation_cache_context` 
(`query_context_processor.py`), which folds in only 
`result_cache_discriminator`, not this marker. After the upgrade that parent 
key is unchanged, so cache hits keep serving the pre-fix annotation rows that 
dropped the NULL matches until the entry expires. Should the marker also be 
added to the `semantic_result_versions` context for `SemanticView` annotation 
sources (including views with no discriminator)?



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