From d6acc19e94838ed62f9c07e2bd95211ecb67ea52 Mon Sep 17 00:00:00 2001 From: George Munyoro Date: Fri, 28 Aug 2026 17:00:44 +0200 Subject: [PATCH 1/8] perf(hogql): reuse shared database in runners that bypassed it HogQLQuery, EventsQuery, SessionsQuery, GroupsQuery, ActorsQuery, InsightActorsQuery, SessionsTimelineQuery, and PropertyValuesQuery called execute_hogql_query without a context, so the executor built its own HogQL database instead of reusing the runner's shared one. Thread context=build_hogql_context() through those call sites so one build serves every call in a run. Generated-By: PostHog Desktop Task-Id: 298f3504-9b69-4cb1-bb9d-908e0b4610d5 --- posthog/hogql_queries/actors_query_runner.py | 4 +-- posthog/hogql_queries/events_query_runner.py | 2 ++ .../groups/groups_query_runner.py | 3 +- posthog/hogql_queries/hogql_query_runner.py | 6 ++++ .../insights/insight_actors_query_runner.py | 1 + .../property_values_query_runner.py | 9 ++--- .../hogql_queries/sessions_query_runner.py | 1 + .../sessions_timeline_query_runner.py | 7 +--- .../test/test_hogql_query_runner.py | 15 +++++++++ .../hogql_queries/test/test_query_runner.py | 33 ++++++++++++++++++- 10 files changed, 63 insertions(+), 18 deletions(-) diff --git a/posthog/hogql_queries/actors_query_runner.py b/posthog/hogql_queries/actors_query_runner.py index cb3c6032d5a0..7680b19a826f 100644 --- a/posthog/hogql_queries/actors_query_runner.py +++ b/posthog/hogql_queries/actors_query_runner.py @@ -18,7 +18,6 @@ from posthog.hogql import ast from posthog.hogql.constants import HogQLQuerySettings, LimitContext -from posthog.hogql.context import HogQLContext from posthog.hogql.parser import parse_expr, parse_order_expr from posthog.hogql.printer import prepare_and_print_ast from posthog.hogql.property import has_aggregation @@ -202,6 +201,7 @@ def _calculate_internal(self) -> ActorsQueryResponse: user=self.user, timings=self.timings, modifiers=self.modifiers, + context=self.build_hogql_context(), ) input_columns = self.input_columns() missing_actors_count = None @@ -501,7 +501,7 @@ def to_query(self) -> ast.SelectQuery | ast.SelectSetQuery: try: prepare_and_print_ast( select_query, - context=HogQLContext( + context=self.build_hogql_context( team=self.team, enable_select_queries=True, timings=self.timings, diff --git a/posthog/hogql_queries/events_query_runner.py b/posthog/hogql_queries/events_query_runner.py index 5461c90caa4d..cc19d6cba0c4 100644 --- a/posthog/hogql_queries/events_query_runner.py +++ b/posthog/hogql_queries/events_query_runner.py @@ -488,6 +488,7 @@ def _calculate(self) -> EventsQueryResponse: modifiers=self.modifiers, limit_context=self.limit_context, user=self.user, + context=self.build_hogql_context(), ) # Convert star field from tuple to dict in each result @@ -696,6 +697,7 @@ def batch_check_session_recordings(self) -> set[str]: query_type="EventsQuerySessionRecordingsCheck", timings=self.timings, modifiers=self.modifiers, + context=self.build_hogql_context(), ) # Return set of session IDs that exist diff --git a/posthog/hogql_queries/groups/groups_query_runner.py b/posthog/hogql_queries/groups/groups_query_runner.py index 35666c5def97..a1e12ccdc017 100644 --- a/posthog/hogql_queries/groups/groups_query_runner.py +++ b/posthog/hogql_queries/groups/groups_query_runner.py @@ -1,7 +1,6 @@ from posthog.schema import CachedGroupsQueryResponse, GroupsQuery, GroupsQueryResponse from posthog.hogql import ast -from posthog.hogql.context import HogQLContext from posthog.hogql.parser import parse_expr, parse_order_expr from posthog.hogql.property import has_aggregation, property_to_expr @@ -152,7 +151,7 @@ def _calculate(self) -> GroupsQueryResponse: timings=self.timings, modifiers=self.modifiers, # :HACKY: posthog/hogql/transforms/property_types.py needs access to the group_id in order to know the property type - context=HogQLContext(team_id=self.team.pk, globals={"group_id": self.query.group_type_index}), + context=self.build_hogql_context(globals={"group_id": self.query.group_type_index}), ) results = response.results[: self.paginator.limit] if self.paginator.limit is not None else response.results diff --git a/posthog/hogql_queries/hogql_query_runner.py b/posthog/hogql_queries/hogql_query_runner.py index c74a72c67388..a5df03d06091 100644 --- a/posthog/hogql_queries/hogql_query_runner.py +++ b/posthog/hogql_queries/hogql_query_runner.py @@ -209,6 +209,11 @@ def _calculate(self) -> HogQLQueryResponse: execute_hogql_query if paginator is None else paginator.execute_hogql_query, ) + context_kwargs: dict[str, Any] = {} + if self.query.connectionId is None: + # With a connection id the executor builds its own connection-scoped database, + # so the shared one would be built for nothing. + context_kwargs["context"] = self.build_hogql_context() response = func( query_type="HogQLQuery", query=query, @@ -224,6 +229,7 @@ def _calculate(self) -> HogQLQueryResponse: workload=self.workload, ch_user=self.ch_user, settings=self.settings, + **context_kwargs, ) if paginator: response = response.model_copy(update={**paginator.response_params(), "results": paginator.results}) diff --git a/posthog/hogql_queries/insights/insight_actors_query_runner.py b/posthog/hogql_queries/insights/insight_actors_query_runner.py index 027806d3bebd..ad961c4c76df 100644 --- a/posthog/hogql_queries/insights/insight_actors_query_runner.py +++ b/posthog/hogql_queries/insights/insight_actors_query_runner.py @@ -211,4 +211,5 @@ def _calculate(self) -> HogQLQueryResponse: timings=self.timings, modifiers=self.modifiers, limit_context=self.limit_context, + context=self.build_hogql_context(), ) diff --git a/posthog/hogql_queries/property_values_query_runner.py b/posthog/hogql_queries/property_values_query_runner.py index 6c53b1c7b3e2..43e828829a21 100644 --- a/posthog/hogql_queries/property_values_query_runner.py +++ b/posthog/hogql_queries/property_values_query_runner.py @@ -17,7 +17,6 @@ ) from posthog.hogql import ast -from posthog.hogql.context import HogQLContext from posthog.hogql.parser import parse_expr, parse_select from posthog.hogql.query import execute_hogql_query @@ -74,11 +73,7 @@ def _calculate_event(self) -> PropertyValuesQueryResponse: self._event_query(), team=self.team, user=self.user, - context=HogQLContext( - team_id=self.team.pk, - user=self.user, - use_new_events_schema=self._use_new_events_schema, - ), + context=self.build_hogql_context(use_new_events_schema=self._use_new_events_schema), timings=self.timings, modifiers=self.modifiers, limit_context=self.limit_context, @@ -163,7 +158,7 @@ def _calculate_person(self) -> PropertyValuesQueryResponse: query_type=query_type, team=self.team, user=self.user, - context=HogQLContext(team_id=self.team.pk, user=self.user), + context=self.build_hogql_context(), timings=self.timings, modifiers=self.modifiers, limit_context=self.limit_context, diff --git a/posthog/hogql_queries/sessions_query_runner.py b/posthog/hogql_queries/sessions_query_runner.py index 61bfbe475cac..5f66d9c8e5f1 100644 --- a/posthog/hogql_queries/sessions_query_runner.py +++ b/posthog/hogql_queries/sessions_query_runner.py @@ -625,6 +625,7 @@ def _calculate(self) -> SessionsQueryResponse: timings=self.timings, modifiers=self.modifiers, limit_context=self.limit_context, + context=self.build_hogql_context(), ) # Convert star field from tuple to dict in each result diff --git a/posthog/hogql_queries/sessions_timeline_query_runner.py b/posthog/hogql_queries/sessions_timeline_query_runner.py index ed27bea625ac..0500941764ed 100644 --- a/posthog/hogql_queries/sessions_timeline_query_runner.py +++ b/posthog/hogql_queries/sessions_timeline_query_runner.py @@ -11,7 +11,6 @@ ) from posthog.hogql import ast -from posthog.hogql.context import HogQLContext from posthog.hogql.parser import parse_select from posthog.hogql.query import execute_hogql_query @@ -205,11 +204,7 @@ def _calculate(self) -> SessionsTimelineQueryResponse: query=self.to_query(), team=self.team, user=self.user, - context=HogQLContext( - team_id=self.team.pk, - user=self.user, - use_new_events_schema=self._use_new_events_schema, - ), + context=self.build_hogql_context(use_new_events_schema=self._use_new_events_schema), query_type="SessionsTimelineQuery", timings=self.timings, modifiers=self.modifiers, diff --git a/posthog/hogql_queries/test/test_hogql_query_runner.py b/posthog/hogql_queries/test/test_hogql_query_runner.py index 09a1938bb224..c4f5fb34d5d3 100644 --- a/posthog/hogql_queries/test/test_hogql_query_runner.py +++ b/posthog/hogql_queries/test/test_hogql_query_runner.py @@ -11,13 +11,16 @@ HogQLFilters, HogQLPropertyFilter, HogQLQuery, + HogQLQueryModifiers, HogQLQueryResponse, HogQLVariable, QueryStatus, + SessionTableVersion, ) from posthog.hogql import ast from posthog.hogql.errors import ExposedHogQLError, QueryError +from posthog.hogql.query import execute_hogql_query from posthog.hogql.user_query_validator import HOGQL_PERSONAL_API_KEY_OFFSET_ALLOWED_FLAG, OFFSET_NOT_ALLOWED_MESSAGE from posthog.hogql.visitor import clear_locations @@ -65,6 +68,18 @@ def setUp(self): super().setUp() self.random_uuid = self._create_random_persons() + def test_calculate_with_query_modifiers_matches_unthreaded_executor_sql(self): + # The runner hands the executor a context wired to its shared database, which is + # built from the runner's resolved modifiers. Query-level modifiers reshape that + # database (e.g. which sessions table backs `sessions`), so the printed SQL must + # match what the executor produces when it builds the database itself. + query = "select count() from sessions limit 1" + modifiers = HogQLQueryModifiers(sessionTableVersion=SessionTableVersion.V1) + runner = self._create_runner(HogQLQuery(query=query, modifiers=modifiers)) + threaded = runner.calculate() + baseline = execute_hogql_query(query=query, team=self.team, modifiers=modifiers) + assert threaded.clickhouse == baseline.clickhouse + def test_default_hogql_query(self): runner = self._create_runner(HogQLQuery(query="select count(event) from events")) query = runner.to_query() diff --git a/posthog/hogql_queries/test/test_query_runner.py b/posthog/hogql_queries/test/test_query_runner.py index 75dd99ffdfc3..2ce5860e24ca 100644 --- a/posthog/hogql_queries/test/test_query_runner.py +++ b/posthog/hogql_queries/test/test_query_runner.py @@ -6,7 +6,7 @@ import pytest from freezegun import freeze_time -from posthog.test.base import BaseTest +from posthog.test.base import APIBaseTest, BaseTest, ClickhouseTestMixin from unittest import mock from django.conf import settings @@ -20,12 +20,15 @@ from rest_framework.exceptions import ValidationError from posthog.schema import ( + ActorsQuery, BounceRatePageViewMode, CacheMissResponse, CurrencyCode, DataTableNode, DataVisualizationNode, EventsNode, + EventsQuery, + GroupsQuery, HogQLQuery, HogQLQueryModifiers, InCohortVia, @@ -35,7 +38,11 @@ MaterializationMode, PersonsArgMaxVersion, PersonsOnEventsMode, + PropertyType, + PropertyValuesQuery, QueryLogTags, + SessionsQuery, + SessionsTimelineQuery, SessionsV2JoinMode, SessionTableVersion, TestBasicQueryResponse as TheTestBasicQueryResponse, @@ -2004,3 +2011,27 @@ def test_memory_limit_breaker_forbids_extended_budget(self): runner.run(execution_mode=ExecutionMode.RECENT_CACHE_CALCULATE_ASYNC_IF_STALE) mock_enqueue.assert_not_called() assert getattr(ctx.exception, "served_from_query_failure_cache", False) + + +class TestRunnersBuildDatabaseOnce(ClickhouseTestMixin, APIBaseTest): + # Guards the context threading in each runner's _calculate: the one Database build + # must go through shared_database (visible as the build_shared_database timing), + # so every execute_hogql_query call in the same run reuses it instead of the + # executor building its own. + @parameterized.expand( + [ + ("hogql", HogQLQuery(query="select count() from events")), + ("events", EventsQuery(select=["event"])), + ("sessions", SessionsQuery(select=["session_id"])), + ("actors", ActorsQuery(select=["person"])), + ("sessions_timeline", SessionsTimelineQuery()), + ("groups", GroupsQuery(group_type_index=0)), + ("property_values", PropertyValuesQuery(property_key="email", property_type=PropertyType.PERSON)), + ] + ) + def test_calculate_builds_database_once_via_shared_path(self, _name: str, query: BaseModel) -> None: + runner = get_query_runner(query=query, team=self.team, user=self.user) + with mock.patch.object(Database, "create_for", wraps=Database.create_for) as create_for: + runner.calculate() + assert create_for.call_count == 1 + assert any("build_shared_database" in key for key in runner.timings.to_dict()) From be97c48dbc37569a268735a1421b0e54fd6ff6fa Mon Sep 17 00:00:00 2001 From: George Munyoro Date: Fri, 28 Aug 2026 17:10:40 +0200 Subject: [PATCH 2/8] perf(hogql): attribute database builds to their call path hogql_database_build_duration_seconds counts builds but cannot say which call path requested them, so redundant executor-side builds are invisible in aggregate. Add hogql_database_build_total{trigger} and label the query-execution seams (shared, executor, printer, runner_context, shared_kill_switch); all other callers count as direct. Generated-By: PostHog Desktop Task-Id: 298f3504-9b69-4cb1-bb9d-908e0b4610d5 --- posthog/hogql/database/database.py | 4 +++- posthog/hogql/editor_assist_metrics.py | 9 ++++++++- posthog/hogql/printer/utils.py | 1 + posthog/hogql/query.py | 2 ++ posthog/hogql_queries/query_runner.py | 7 +++++-- 5 files changed, 19 insertions(+), 4 deletions(-) diff --git a/posthog/hogql/database/database.py b/posthog/hogql/database/database.py index 13e7903e5239..e8a60357dca0 100644 --- a/posthog/hogql/database/database.py +++ b/posthog/hogql/database/database.py @@ -150,7 +150,7 @@ from posthog.hogql.database.schema.web_vitals_paths_preaggregated import WebVitalsPathsPreaggregatedTable from posthog.hogql.database.utils import get_join_field_chain, qualify_join_key_expr from posthog.hogql.database.warehouse_join_resolvers import data_warehouse_resolver_params -from posthog.hogql.editor_assist_metrics import HOGQL_DATABASE_BUILD_DURATION_SECONDS +from posthog.hogql.editor_assist_metrics import HOGQL_DATABASE_BUILD_DURATION_SECONDS, HOGQL_DATABASE_BUILD_TOTAL from posthog.hogql.errors import QueryError, ResolutionError, TableAccessDeniedError from posthog.hogql.modifiers import create_default_modifiers_for_team from posthog.hogql.parser import parse_expr @@ -1372,10 +1372,12 @@ def create_for( connection_id: str | None = None, bypass_warehouse_access_control: bool = False, build_postgres_foreign_keys: bool = True, + trigger: str = "direct", ) -> Database: if timings is None: timings = HogQLTimings() + HOGQL_DATABASE_BUILD_TOTAL.labels(trigger=trigger).inc() with HOGQL_DATABASE_BUILD_DURATION_SECONDS.labels(phase="fetch_sources").time(): sources = Database._fetch_sources( team_id, diff --git a/posthog/hogql/editor_assist_metrics.py b/posthog/hogql/editor_assist_metrics.py index 3dd24e9d2417..cace5cfee42f 100644 --- a/posthog/hogql/editor_assist_metrics.py +++ b/posthog/hogql/editor_assist_metrics.py @@ -1,4 +1,4 @@ -from prometheus_client import Histogram +from prometheus_client import Counter, Histogram EDITOR_ASSIST_DURATION_SECONDS = Histogram( "hogql_editor_assist_duration_seconds", @@ -13,3 +13,10 @@ labelnames=["phase"], buckets=(0.005, 0.01, 0.025, 0.05, 0.1, 0.25, 0.5, 1.0, 2.5), ) + +HOGQL_DATABASE_BUILD_TOTAL = Counter( + "hogql_database_build", + "HogQL database builds by the call path that requested them. Redundant builds show up as " + "executor/printer builds that a shared build should have covered.", + labelnames=["trigger"], +) diff --git a/posthog/hogql/printer/utils.py b/posthog/hogql/printer/utils.py index fb42c4ad428f..61d9571c54e9 100644 --- a/posthog/hogql/printer/utils.py +++ b/posthog/hogql/printer/utils.py @@ -148,6 +148,7 @@ def prepare_ast_for_printing( user=context.user, timings=context.timings, bypass_warehouse_access_control=context.bypass_warehouse_access_control, + trigger="printer", ) if context.direct_postgres_connection_metadata is None and context.database is not None: context.direct_postgres_connection_metadata = getattr(context.database, "_direct_connection_metadata", None) diff --git a/posthog/hogql/query.py b/posthog/hogql/query.py index 11fec561a34b..a8a044afce64 100644 --- a/posthog/hogql/query.py +++ b/posthog/hogql/query.py @@ -191,6 +191,7 @@ def _process_placeholders(self): modifiers=self.query_modifiers, timings=self.timings, bypass_warehouse_access_control=self.context.bypass_warehouse_access_control, + trigger="executor", ) self.select_query = replace_filters( self.select_query, self.filters, self.team, database=self.context.database @@ -255,6 +256,7 @@ def _generate_hogql(self): timings=self.timings, connection_id=self.connection_id, bypass_warehouse_access_control=self.context.bypass_warehouse_access_control, + trigger="executor", ) # Reset between executions: the resolver/printer append per query, and dataclasses.replace diff --git a/posthog/hogql_queries/query_runner.py b/posthog/hogql_queries/query_runner.py index 8df1e64a630f..a70ee8705658 100644 --- a/posthog/hogql_queries/query_runner.py +++ b/posthog/hogql_queries/query_runner.py @@ -1714,7 +1714,9 @@ def shared_database(self) -> Database: # Kill switch: build per access so query threads never share schema state. No timings # measure here because concurrent threads reach this path and HogQLTimings is not # thread-safe. - return Database.create_for(team=self.team, user=self.user, modifiers=self.modifiers) + return Database.create_for( + team=self.team, user=self.user, modifiers=self.modifiers, trigger="shared_kill_switch" + ) if self._shared_database is None: # Concurrent query threads (funnels compare mode) can first-touch this property at the # same time. The lock makes the build run once, and keeps the measure on the single @@ -1727,6 +1729,7 @@ def shared_database(self) -> Database: user=self.user, modifiers=self.modifiers, timings=self.timings, + trigger="shared", ) return self._shared_database @@ -3076,7 +3079,7 @@ def __init__(self, *args, **kwargs): self._build_hogql_context_for_user(self.user) def _build_hogql_context_for_user(self, user: Optional[User]) -> None: - self.database = Database.create_for(team=self.team, user=user) + self.database = Database.create_for(team=self.team, user=user, trigger="runner_context") self.hogql_context = HogQLContext(team_id=self.team.pk, database=self.database, user=user) def _on_user_changed(self) -> None: From b233cc72254426b06fdd01f9b97bc82946d5318b Mon Sep 17 00:00:00 2001 From: George Munyoro Date: Fri, 28 Aug 2026 17:42:33 +0200 Subject: [PATCH 3/8] chore(persons): count the instance-setting read in pagination test The persons list now touches the shared-database kill switch, whose instance setting costs one cold-cache query in tests. Generated-By: PostHog Desktop Task-Id: 298f3504-9b69-4cb1-bb9d-908e0b4610d5 --- posthog/api/test/test_person.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/posthog/api/test/test_person.py b/posthog/api/test/test_person.py index a8cf22a09db3..cf858241fb60 100644 --- a/posthog/api/test/test_person.py +++ b/posthog/api/test/test_person.py @@ -1897,8 +1897,9 @@ def test_pagination_limit(self): returned_ids = [] # The property-access-control feature check reuses the request's already-loaded team, # so listing persons no longer pays a per-request Team lookup (was 16). +1 for the - # saved-expressions fetch in the HogQL database build. - with self.assertNumQueries(16): + # saved-expressions fetch in the HogQL database build. +1 for the shared-database + # kill-switch instance setting, cold-cache here but TTL-cached per worker in production. + with self.assertNumQueries(17): response = self.client.get("/api/person/?limit=10").json() self.assertEqual(len(response["results"]), 9) returned_ids += [x["distinct_ids"][0] for x in response["results"]] From 7b8003770ce21be6680e66c3abb07496df7a39b4 Mon Sep 17 00:00:00 2001 From: "posthog[bot]" <206114724+posthog[bot]@users.noreply.github.com> Date: Fri, 28 Aug 2026 17:26:30 +0000 Subject: [PATCH 4/8] fix(hogql): align HogQL runner printer modifiers with shared database The shared per-run database is built from self.modifiers, but the executor was still handed self.query.modifiers or self.modifiers. When a caller sets both constructor and query modifiers, the schema is built from one set while the SQL is printed and reported for another. Pass self.modifiers so the printer and shared database always agree. Add a regression test that catches the reported modifiers diverging from the schema the SQL was built against. Generated-By: PostHog Desktop Task-Id: d8a05cb6-9f8f-4e00-8ce3-37018a0bbb98 --- posthog/hogql_queries/hogql_query_runner.py | 5 ++++- .../test/test_hogql_query_runner.py | 20 +++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/posthog/hogql_queries/hogql_query_runner.py b/posthog/hogql_queries/hogql_query_runner.py index a5df03d06091..61cc4d15e73b 100644 --- a/posthog/hogql_queries/hogql_query_runner.py +++ b/posthog/hogql_queries/hogql_query_runner.py @@ -218,7 +218,10 @@ def _calculate(self) -> HogQLQueryResponse: query_type="HogQLQuery", query=query, filters=self.query.filters, - modifiers=self.query.modifiers or self.modifiers, + # Print with the same modifiers the shared database was built from (self.modifiers). + # The shared database uses self.modifiers, so passing self.query.modifiers here would let + # a caller that sets both build the schema from one modifier set and print SQL for another. + modifiers=self.modifiers, team=self.team, user=self.user, user_access_control=self.user_access_control, diff --git a/posthog/hogql_queries/test/test_hogql_query_runner.py b/posthog/hogql_queries/test/test_hogql_query_runner.py index c4f5fb34d5d3..27e375ec1a85 100644 --- a/posthog/hogql_queries/test/test_hogql_query_runner.py +++ b/posthog/hogql_queries/test/test_hogql_query_runner.py @@ -80,6 +80,26 @@ def test_calculate_with_query_modifiers_matches_unthreaded_executor_sql(self): baseline = execute_hogql_query(query=query, team=self.team, modifiers=modifiers) assert threaded.clickhouse == baseline.clickhouse + def test_calculate_reports_modifiers_matching_its_shared_database(self): + # The shared database is built from the runner's resolved modifiers, which prefer the + # constructor modifiers over the query's own. The executor must run and report those same + # modifiers, or a caller that sets both gets SQL built for one schema and a response that + # describes another. sessionTableVersion picks which table backs `sessions`. + query = "select count() from sessions limit 1" + runner = HogQLQueryRunner( + team=self.team, + query=HogQLQuery(query=query, modifiers=HogQLQueryModifiers(sessionTableVersion=SessionTableVersion.V2)), + modifiers=HogQLQueryModifiers(sessionTableVersion=SessionTableVersion.V1), + ) + response = runner.calculate() + baseline = execute_hogql_query( + query=query, team=self.team, modifiers=HogQLQueryModifiers(sessionTableVersion=SessionTableVersion.V1) + ) + # SQL is built from the shared database's modifiers (V1), and the reported modifiers agree. + assert response.clickhouse == baseline.clickhouse + assert response.modifiers is not None + assert response.modifiers.sessionTableVersion == SessionTableVersion.V1 + def test_default_hogql_query(self): runner = self._create_runner(HogQLQuery(query="select count(event) from events")) query = runner.to_query() From 946f1f175adb11fe47bb63508f1a2261685c76fe Mon Sep 17 00:00:00 2001 From: "posthog[bot]" <206114724+posthog[bot]@users.noreply.github.com> Date: Fri, 28 Aug 2026 18:00:04 +0000 Subject: [PATCH 5/8] perf(hogql): reuse shared database when resolving {filters} to_query() called replace_filters() without a database, so a HogQL query carrying a {filters} placeholder plus dashboard/insight filters built one throwaway database there (the "direct" trigger) and then the shared database in _calculate. That is two full schema builds for a common SQL-insight shape, the exact redundancy this PR removes elsewhere. Pass the shared database into replace_filters when there is no connection id, so the filtered path builds the schema once. replace_filters uses the database only to classify the query's FROM tables (events/sessions/logs/traces/groups/ persons), which is invariant to user and modifiers for those core tables, so the generated SQL is unchanged. Connection-id queries keep the per-call build, since their schema is the external connection's. Add a filtered-query case to the build-count test, which asserts one Database.create_for call; it fails without the threading. Generated-By: PostHog Desktop Task-Id: ed4c0bb6-29a7-4129-bd13-0a8ecdc8dffb --- posthog/hogql_queries/hogql_query_runner.py | 6 +++++- posthog/hogql_queries/test/test_query_runner.py | 9 +++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/posthog/hogql_queries/hogql_query_runner.py b/posthog/hogql_queries/hogql_query_runner.py index 61cc4d15e73b..fd75e80f3a64 100644 --- a/posthog/hogql_queries/hogql_query_runner.py +++ b/posthog/hogql_queries/hogql_query_runner.py @@ -144,7 +144,11 @@ def to_query(self) -> ast.SelectQuery | ast.SelectSetQuery: finder = find_placeholders(parsed_select) with self.timings.measure("filters"): if self.query.filters and finder.has_filters: - parsed_select = replace_filters(parsed_select, self.query.filters, self.team) + # Resolve {filters} against the shared database so a filtered query builds the schema + # once, instead of replace_filters building a throwaway one. With a connection id the + # schema is the external connection's, so keep the per-call build there. + database = self.shared_database if self.query.connectionId is None else None + parsed_select = replace_filters(parsed_select, self.query.filters, self.team, database) if self.query.variables: with self.timings.measure("replace_variables"): parsed_select = replace_variables(parsed_select, list(self.query.variables.values()), self.team) diff --git a/posthog/hogql_queries/test/test_query_runner.py b/posthog/hogql_queries/test/test_query_runner.py index 2ce5860e24ca..40d6ee1b54a4 100644 --- a/posthog/hogql_queries/test/test_query_runner.py +++ b/posthog/hogql_queries/test/test_query_runner.py @@ -26,9 +26,11 @@ CurrencyCode, DataTableNode, DataVisualizationNode, + DateRange, EventsNode, EventsQuery, GroupsQuery, + HogQLFilters, HogQLQuery, HogQLQueryModifiers, InCohortVia, @@ -2021,6 +2023,13 @@ class TestRunnersBuildDatabaseOnce(ClickhouseTestMixin, APIBaseTest): @parameterized.expand( [ ("hogql", HogQLQuery(query="select count() from events")), + ( + "hogql_filtered", + HogQLQuery( + query="select count() from events where {filters}", + filters=HogQLFilters(dateRange=DateRange(date_from="-7d")), + ), + ), ("events", EventsQuery(select=["event"])), ("sessions", SessionsQuery(select=["session_id"])), ("actors", ActorsQuery(select=["person"])), From aa36e222c7069c31af64cde9634cb464c54b6a62 Mon Sep 17 00:00:00 2001 From: George Munyoro Date: Mon, 31 Aug 2026 11:39:17 +0200 Subject: [PATCH 6/8] perf(hogql): reuse the runner access-control snapshot in shared builds The shared database built its own UserAccessControl inside the system-table access check, re-loading the RBAC rows the runner had already fetched for its cache fingerprint. Pass the runner's snapshot into both shared_database build paths so one instance serves the fingerprint and the schema, as the executor's own build already did. Generated-By: PostHog Desktop Task-Id: 298f3504-9b69-4cb1-bb9d-908e0b4610d5 --- posthog/hogql_queries/query_runner.py | 14 +++++++++++++- posthog/hogql_queries/test/test_query_runner.py | 6 ++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/posthog/hogql_queries/query_runner.py b/posthog/hogql_queries/query_runner.py index 7799bc355d13..e10c00716d5d 100644 --- a/posthog/hogql_queries/query_runner.py +++ b/posthog/hogql_queries/query_runner.py @@ -1717,6 +1717,13 @@ def _on_user_changed(self) -> None: a cached HogQLContext / Database) must call super()._on_user_changed().""" self._shared_database = None + @property + def user_access_control(self) -> Optional[UserAccessControl]: + """Access-control snapshot the shared database is built with. None here; overridden by + AnalyticsQueryRunner with a lazily built, per-run instance so the cache fingerprint and + the database resolve access from the same rows.""" + return None + @property def shared_database(self) -> Database: """One Database for every query this runner executes and for the response SQL printer. @@ -1729,7 +1736,11 @@ def shared_database(self) -> Database: # measure here because concurrent threads reach this path and HogQLTimings is not # thread-safe. return Database.create_for( - team=self.team, user=self.user, modifiers=self.modifiers, trigger="shared_kill_switch" + team=self.team, + user=self.user, + user_access_control=self.user_access_control, + modifiers=self.modifiers, + trigger="shared_kill_switch", ) if self._shared_database is None: # Concurrent query threads (funnels compare mode) can first-touch this property at the @@ -1741,6 +1752,7 @@ def shared_database(self) -> Database: self._shared_database = Database.create_for( team=self.team, user=self.user, + user_access_control=self.user_access_control, modifiers=self.modifiers, timings=self.timings, trigger="shared", diff --git a/posthog/hogql_queries/test/test_query_runner.py b/posthog/hogql_queries/test/test_query_runner.py index 40d6ee1b54a4..04d9421ec15c 100644 --- a/posthog/hogql_queries/test/test_query_runner.py +++ b/posthog/hogql_queries/test/test_query_runner.py @@ -261,6 +261,12 @@ def first_touch(index: int) -> None: timing_keys = [key for key in runner.timings.to_dict() if "build_shared_database" in key] assert timing_keys == ["./build_shared_database"] + def test_shared_database_reuses_runner_access_control_snapshot(self): + runner = HogQLQueryRunner(query=HogQLQuery(query="select 1"), team=self.team, user=self.user) + snapshot = runner.user_access_control + assert snapshot is not None + assert runner.shared_database.user_access_control is snapshot + def test_shared_database_kill_switch_disables_sharing(self): TestQueryRunner = self.setup_test_query_runner_class() runner = TestQueryRunner(query={"some_attr": "bla"}, team=self.team) From a4ccd8a3f46201d28fcd158a84608dc168b119c4 Mon Sep 17 00:00:00 2001 From: George Munyoro Date: Mon, 31 Aug 2026 12:27:21 +0200 Subject: [PATCH 7/8] fix(hogql): invalidate actors runner shared state on user change ActorsQueryRunner.run() assigned self.user before delegating to the base run(), so the base saw no identity change and never dropped the shared database or access-control snapshot. A second run as a different user reused the first user's access-controlled schema. Perform the transition with invalidation instead of a bare assignment, keeping this runner's take-the-run-user-verbatim semantics including None. Generated-By: PostHog Desktop Task-Id: 298f3504-9b69-4cb1-bb9d-908e0b4610d5 --- posthog/hogql_queries/actors_query_runner.py | 7 ++++++- .../hogql_queries/test/test_query_runner.py | 21 +++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/posthog/hogql_queries/actors_query_runner.py b/posthog/hogql_queries/actors_query_runner.py index 7680b19a826f..f4a357b058e5 100644 --- a/posthog/hogql_queries/actors_query_runner.py +++ b/posthog/hogql_queries/actors_query_runner.py @@ -77,7 +77,12 @@ def run( cache_age_seconds: Optional[int] = None, analytics_props: Optional["AnalyticsProps"] = None, ): - self.user = user + # This runner takes the run() user verbatim, including None. A bare assignment would + # hide the change from the base run(), leaving the shared database and access-control + # snapshot scoped to the previous user. + if user is not self.user: + self.user = user + self._on_user_changed() return super().run( execution_mode, user, diff --git a/posthog/hogql_queries/test/test_query_runner.py b/posthog/hogql_queries/test/test_query_runner.py index 04d9421ec15c..8df8254cb745 100644 --- a/posthog/hogql_queries/test/test_query_runner.py +++ b/posthog/hogql_queries/test/test_query_runner.py @@ -21,6 +21,7 @@ from posthog.schema import ( ActorsQuery, + ActorsQueryResponse, BounceRatePageViewMode, CacheMissResponse, CurrencyCode, @@ -61,6 +62,7 @@ from posthog.constants import AvailableFeature from posthog.errors import ExposedCHQueryError from posthog.exceptions import ClickHouseQueryMemoryLimitExceeded, ClickHouseQuerySizeExceeded, ClickHouseQueryTimeOut +from posthog.hogql_queries.actors_query_runner import ActorsQueryRunner from posthog.hogql_queries.hogql_query_runner import HogQLQueryRunner from posthog.hogql_queries.insights.trends.trends_query_runner import TrendsQueryRunner from posthog.hogql_queries.query_failure_handling import classify_failure @@ -79,6 +81,7 @@ from posthog.models.sharing_configuration import SharingConfiguration from posthog.models.team.team import Team, WeekStartDay from posthog.models.team.team_revenue_analytics_config import TeamRevenueAnalyticsConfig +from posthog.models.user import User from posthog.query_cache.failures import ( BASE_BACKOFF, BUDGET_EXTENDED, @@ -267,6 +270,24 @@ def test_shared_database_reuses_runner_access_control_snapshot(self): assert snapshot is not None assert runner.shared_database.user_access_control is snapshot + def test_actors_run_rebuilds_shared_state_on_user_change(self): + other_user = User.objects.create_and_join(self.organization, "other-user@example.com", None) + runner = ActorsQueryRunner(query=ActorsQuery(select=["properties.email"]), team=self.team) + seen: list[tuple[Any, Any]] = [] + + def capture(self_runner: ActorsQueryRunner) -> ActorsQueryResponse: + seen.append((self_runner.shared_database, self_runner.user_access_control)) + return ActorsQueryResponse(results=[], columns=[], hogql="", limit=100, offset=0) + + with mock.patch.object(ActorsQueryRunner, "_calculate", autospec=True, side_effect=capture): + runner.run(execution_mode=ExecutionMode.CALCULATE_BLOCKING_ALWAYS, user=self.user) + runner.run(execution_mode=ExecutionMode.CALCULATE_BLOCKING_ALWAYS, user=other_user) + + (first_database, first_snapshot), (second_database, second_snapshot) = seen + assert first_database is not second_database + assert first_snapshot is not second_snapshot + assert second_database.user_access_control is second_snapshot + def test_shared_database_kill_switch_disables_sharing(self): TestQueryRunner = self.setup_test_query_runner_class() runner = TestQueryRunner(query={"some_attr": "bla"}, team=self.team) From 7a4793117c74d7b4806f13c93f4c697332d7ec59 Mon Sep 17 00:00:00 2001 From: Marius Andra Date: Mon, 31 Aug 2026 12:57:00 +0200 Subject: [PATCH 8/8] fix(hogql): preserve shared runner cache context --- posthog/hogql_queries/hogql_query_runner.py | 4 ++++ posthog/hogql_queries/query_runner.py | 7 +++++++ posthog/hogql_queries/test/test_hogql_query_runner.py | 1 + posthog/hogql_queries/test/test_query_runner.py | 11 ++++++++++- 4 files changed, 22 insertions(+), 1 deletion(-) diff --git a/posthog/hogql_queries/hogql_query_runner.py b/posthog/hogql_queries/hogql_query_runner.py index fd75e80f3a64..dcbfd27e1f47 100644 --- a/posthog/hogql_queries/hogql_query_runner.py +++ b/posthog/hogql_queries/hogql_query_runner.py @@ -85,6 +85,10 @@ def get_cache_payload(self) -> dict: payload = super().get_cache_payload() if self._managed_warehouse_sql_mode == ManagedWarehouseSQLMode.BUILT_IN: payload["managed_warehouse_sql_mode"] = self._managed_warehouse_sql_mode.value + if self._modifiers_override_provided and self.query.modifiers is not None: + # Old workers preferred query modifiers while new workers prefer the constructor override. + # Keep their cached results apart during a rolling deploy. + payload["hogql_modifier_precedence"] = "runner" return payload def query_status_labels(self) -> list[str] | None: diff --git a/posthog/hogql_queries/query_runner.py b/posthog/hogql_queries/query_runner.py index e10c00716d5d..78c7bd16b8c8 100644 --- a/posthog/hogql_queries/query_runner.py +++ b/posthog/hogql_queries/query_runner.py @@ -1682,6 +1682,7 @@ def __init__( self.query_id = query_id self.workload = workload self.ch_user = ch_user + self._modifiers_override_provided = modifiers is not None if not self.is_query_node(query): if isinstance(self.query_type, UnionType): @@ -2976,6 +2977,12 @@ def calculate(self) -> AR: def _on_user_changed(self) -> None: super()._on_user_changed() + if ( + self._user_access_control is not None + and isinstance(self.user, User) + and self._user_access_control.user.pk == self.user.pk + ): + return self._user_access_control = None @property diff --git a/posthog/hogql_queries/test/test_hogql_query_runner.py b/posthog/hogql_queries/test/test_hogql_query_runner.py index 27e375ec1a85..05374fc31202 100644 --- a/posthog/hogql_queries/test/test_hogql_query_runner.py +++ b/posthog/hogql_queries/test/test_hogql_query_runner.py @@ -99,6 +99,7 @@ def test_calculate_reports_modifiers_matching_its_shared_database(self): assert response.clickhouse == baseline.clickhouse assert response.modifiers is not None assert response.modifiers.sessionTableVersion == SessionTableVersion.V1 + assert runner.get_cache_payload()["hogql_modifier_precedence"] == "runner" def test_default_hogql_query(self): runner = self._create_runner(HogQLQuery(query="select count(event) from events")) diff --git a/posthog/hogql_queries/test/test_query_runner.py b/posthog/hogql_queries/test/test_query_runner.py index 8df8254cb745..3daa5bba73a7 100644 --- a/posthog/hogql_queries/test/test_query_runner.py +++ b/posthog/hogql_queries/test/test_query_runner.py @@ -73,6 +73,7 @@ QueryRunner, QueryRunnerWithHogQLContext, get_query_runner, + get_query_runner_or_none, shared_insights_execution_mode, ) from posthog.hogql_queries.utils.query_date_range import QueryDateRange @@ -272,7 +273,14 @@ def test_shared_database_reuses_runner_access_control_snapshot(self): def test_actors_run_rebuilds_shared_state_on_user_change(self): other_user = User.objects.create_and_join(self.organization, "other-user@example.com", None) - runner = ActorsQueryRunner(query=ActorsQuery(select=["properties.email"]), team=self.team) + preloaded_snapshot = UserAccessControl(user=self.user, team=self.team) + runner = get_query_runner_or_none( + query=ActorsQuery(select=["properties.email"]), + team=self.team, + user=self.user, + user_access_control=preloaded_snapshot, + ) + assert isinstance(runner, ActorsQueryRunner) seen: list[tuple[Any, Any]] = [] def capture(self_runner: ActorsQueryRunner) -> ActorsQueryResponse: @@ -285,6 +293,7 @@ def capture(self_runner: ActorsQueryRunner) -> ActorsQueryResponse: (first_database, first_snapshot), (second_database, second_snapshot) = seen assert first_database is not second_database + assert first_snapshot is preloaded_snapshot assert first_snapshot is not second_snapshot assert second_database.user_access_control is second_snapshot