fix(insights): Do NOT calculate non-cacheables when cache-only

posthog/api/services/query.py

@@ -11,7 +11,7 @@
 from posthog.hogql.autocomplete import get_hogql_autocomplete
 from posthog.hogql.metadata import get_hogql_metadata
 from posthog.hogql.modifiers import create_default_modifiers_for_team
-from posthog.hogql_queries.query_runner import ExecutionMode, get_query_runner
+from posthog.hogql_queries.query_runner import CacheMissResponse, ExecutionMode, get_query_runner
 from posthog.models import Team
 from posthog.queries.time_to_see_data.serializers import SessionEventsQuerySerializer, SessionsQuerySerializer
 from posthog.queries.time_to_see_data.sessions import get_session_events, get_sessions
@@ -41,7 +41,7 @@
 
 logger = structlog.get_logger(__name__)
 
-QUERY_WITH_RUNNER = (
+QUERY_WITH_RUNNER_USING_CACHE = (
     TrendsQuery
     | FunnelsQuery
     | RetentionQuery
@@ -82,10 +82,17 @@ def process_query_model(
 ) -> dict:
     result: dict | BaseModel
 
-    if isinstance(query, QUERY_WITH_RUNNER):  # type: ignore
+    if execution_mode == ExecutionMode.CACHE_ONLY_NEVER_CALCULATE and not isinstance(
+        query,
+        QUERY_WITH_RUNNER_USING_CACHE,  # type: ignore
+    ):
+        result = CacheMissResponse(cache_key=None)
+
+    if isinstance(query, QUERY_WITH_RUNNER_USING_CACHE):  # type: ignore
         query_runner = get_query_runner(query, team, limit_context=limit_context)
         result = query_runner.run(execution_mode=execution_mode)
     elif isinstance(query, QUERY_WITH_RUNNER_NO_CACHE):  # type: ignore
+        # TODO: These queries should be using the QueryRunner caching layer too
         query_runner = get_query_runner(query, team, limit_context=limit_context)
         result = query_runner.calculate()
     elif isinstance(query, HogQLAutocomplete):

posthog/caching/calculate_results.py

@@ -1,6 +1,5 @@
 from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple, Union
 
-from posthog.api.services.query import ExecutionMode
 import structlog
 from sentry_sdk import capture_exception
 
@@ -110,7 +109,7 @@ def get_cache_type(cacheable: Optional[FilterType] | Optional[Dict]) -> CacheTyp
 def calculate_for_query_based_insight(
     insight: Insight, *, dashboard: Optional[Dashboard] = None, refresh_requested: bool
 ) -> "InsightResult":
-    from posthog.api.services.query import process_query
+    from posthog.api.services.query import process_query, ExecutionMode
     from posthog.caching.fetch_from_cache import InsightResult, NothingInCacheResult
 
     tag_queries(team_id=insight.team_id, insight_id=insight.pk)

posthog/hogql_queries/query_runner.py

@@ -102,7 +102,7 @@ class CacheMissResponse(BaseModel):
     model_config = ConfigDict(
         extra="forbid",
     )
-    cache_key: str
+    cache_key: Optional[str]
 
 
 RunnableQueryNode = Union[
@@ -329,7 +329,8 @@ def calculate(self) -> BaseModel:
     def run(
         self, execution_mode: ExecutionMode = ExecutionMode.RECENT_CACHE_CALCULATE_IF_STALE
     ) -> CachedQueryResponse | CacheMissResponse:
-        cache_key = f"{self._cache_key()}_{self.limit_context or LimitContext.QUERY}"
+        # TODO: `self.limit_context` should probably just be in get_cache_key()
+        cache_key = cache_key = f"{self.get_cache_key()}_{self.limit_context or LimitContext.QUERY}"
         tag_queries(cache_key=cache_key)
 
         if execution_mode != ExecutionMode.CALCULATION_ALWAYS:
@@ -406,7 +407,7 @@ def to_hogql(self) -> str:
     def toJSON(self) -> str:
         return self.query.model_dump_json(exclude_defaults=True, exclude_none=True)
 
-    def _cache_key(self) -> str:
+    def get_cache_key(self) -> str:
         modifiers = self.modifiers.model_dump_json(exclude_defaults=True, exclude_none=True)
         return generate_cache_key(
             f"query_{self.toJSON()}_{self.__class__.__name__}_{self.team.pk}_{self.team.timezone}_{modifiers}"

posthog/hogql_queries/test/test_query_runner.py

@@ -94,7 +94,7 @@ def test_cache_key(self):
 
         runner = TestQueryRunner(query={"some_attr": "bla"}, team=team)
 
-        cache_key = runner._cache_key()
+        cache_key = runner.get_cache_key()
         self.assertEqual(cache_key, "cache_b6f14c97c218e0b9c9a8258f7460fd5b")
 
     def test_cache_key_runner_subclass(self):
@@ -108,7 +108,7 @@ class TestSubclassQueryRunner(TestQueryRunner):
 
         runner = TestSubclassQueryRunner(query={"some_attr": "bla"}, team=team)
 
-        cache_key = runner._cache_key()
+        cache_key = runner.get_cache_key()
         self.assertEqual(cache_key, "cache_ec1c2f9715cf9c424b1284b94b1205e6")
 
     def test_cache_key_different_timezone(self):
@@ -119,7 +119,7 @@ def test_cache_key_different_timezone(self):
 
         runner = TestQueryRunner(query={"some_attr": "bla"}, team=team)
 
-        cache_key = runner._cache_key()
+        cache_key = runner.get_cache_key()
         self.assertEqual(cache_key, "cache_a6614c0fb564f9c98b1d7b830928c7a1")
 
     def test_cache_response(self):

posthog/hogql_queries/web_analytics/web_analytics_query_runner.py

@@ -244,8 +244,8 @@ def _unsample(self, n: Optional[int | float]):
             else n / self._sample_rate.numerator
         )
 
-    def _cache_key(self) -> str:
-        original = super()._cache_key()
+    def get_cache_key(self) -> str:
+        original = super().get_cache_key()
         return f"{original}_{self.team.path_cleaning_filters}"