From 5973b3e95f24677b68b1ac33048ef4458f0c6f02 Mon Sep 17 00:00:00 2001 From: Michael Matloka Date: Wed, 24 Apr 2024 18:57:35 +0200 Subject: [PATCH] fix(insights): Do NOT calculate non-cacheables when cache-only --- posthog/api/services/query.py | 7 ++++++- posthog/hogql_queries/query_runner.py | 6 +++--- posthog/hogql_queries/test/test_query_runner.py | 6 +++--- .../web_analytics/web_analytics_query_runner.py | 4 ++-- 4 files changed, 14 insertions(+), 9 deletions(-) diff --git a/posthog/api/services/query.py b/posthog/api/services/query.py index 09d33759d0225..8d681940b4da7 100644 --- a/posthog/api/services/query.py +++ b/posthog/api/services/query.py @@ -4,6 +4,7 @@ from pydantic import BaseModel from rest_framework.exceptions import ValidationError +from posthog.caching.fetch_from_cache import NothingInCacheResult from posthog.clickhouse.query_tagging import tag_queries from posthog.hogql.constants import LimitContext from posthog.hogql.context import HogQLContext @@ -86,8 +87,12 @@ def process_query_model( 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 use the caching layer too query_runner = get_query_runner(query, team, limit_context=limit_context) - result = query_runner.calculate() + if execution_mode == ExecutionMode.CACHE_ONLY_NEVER_CALCULATE: + result = NothingInCacheResult(query_runner.get_cache_key()) + else: + result = query_runner.calculate() elif isinstance(query, HogQLAutocomplete): result = get_hogql_autocomplete(query=query, team=team) elif isinstance(query, HogQLMetadata): diff --git a/posthog/hogql_queries/query_runner.py b/posthog/hogql_queries/query_runner.py index bb060e220e3e9..1e3b66d0c2fea 100644 --- a/posthog/hogql_queries/query_runner.py +++ b/posthog/hogql_queries/query_runner.py @@ -329,7 +329,7 @@ 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}" + cache_key = self.get_cache_key() tag_queries(cache_key=cache_key) if execution_mode != ExecutionMode.CALCULATION_ALWAYS: @@ -406,10 +406,10 @@ 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}" + f"query_{self.toJSON()}_{self.__class__.__name__}_{self.team.pk}_{self.team.timezone}_{modifiers}_{self.limit_context or LimitContext.QUERY}" ) @abstractmethod diff --git a/posthog/hogql_queries/test/test_query_runner.py b/posthog/hogql_queries/test/test_query_runner.py index 4905feaa2eae9..a02cf4fb46cdd 100644 --- a/posthog/hogql_queries/test/test_query_runner.py +++ b/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): diff --git a/posthog/hogql_queries/web_analytics/web_analytics_query_runner.py b/posthog/hogql_queries/web_analytics/web_analytics_query_runner.py index ffb758858d151..f91f3c1cff404 100644 --- a/posthog/hogql_queries/web_analytics/web_analytics_query_runner.py +++ b/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}"