[georgemunyoro]

Problem

Building the per-team HogQL database is a large share of query compile time, and eight query runners rebuild it inside the executor on every calculation. These runners power SQL insights, the events explorer, person and group lists, session timelines, and property-value dropdowns.

#88183 added a shared per-run database on the runner base class, but only the insight runners (trends, funnels, retention, paths) use it. The existing hogql_database_build_duration_seconds histogram counts builds and their duration, but cannot attribute a build to the call path that requested it, so redundant builds are invisible in aggregate.

Changes

• Each runner passes context=self.build_hogql_context() into execute_hogql_query, so one shared build serves every call in a run.
• Events explorer queries with * columns built the database twice per run (main query plus the recordings check); they now build once.
• Actors queries that validate a trends source query before executing also drop from two builds to one.
• New counter hogql_database_build_total{trigger} attributes every build to its call path: shared, executor, printer, runner_context, shared_kill_switch, or direct (all other callers). After deploy, executor and printer rates drop and shared rises; watch it with sum(rate(hogql_database_build_total[5m])) by (trigger).
• Query results and SQL are unchanged; the response timings payload now records build_shared_database in place of the executor's create_hogql_database entry.
• Direct-connection HogQL queries keep the executor-built database, which is connection-scoped.
• The existing HOGQL_SHARED_INSIGHT_DATABASE_ENABLED instance setting covers these call sites; disabling it restores per-call builds (visible as the shared_kill_switch trigger).
• Runners changed: HogQLQuery, EventsQuery, SessionsQuery, GroupsQuery, ActorsQuery, InsightActorsQuery, SessionsTimelineQuery, PropertyValuesQuery. The trigger parameter threading is mechanical.

How did you test this code?

• New TestRunnersBuildDatabaseOnce (parameterized over 7 runners) asserts one Database.create_for call per calculation, made through the shared path. All 7 cases fail on master and pass on this branch, so it catches any runner losing the threading.
• New test_calculate_with_query_modifiers_matches_unthreaded_executor_sql catches the shared database diverging from the executor's own build when query-level modifiers reshape the schema (e.g. sessionTableVersion).
• The full local suites for every touched runner pass, except 3 snapshot failures in test_insight_actors_query_runner.py that reproduce identically on unmodified master in this environment (the local person-on-events default differs from the snapshot environment) - unrelated to this diff.
• Repo-wide mypy and hogli ci:preflight --fix pass locally.
• Not run locally: the full backend suite; CI covers it.

Automatic notifications

• Publish to changelog?

Docs update

None - no user-facing behavior change.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

• Authored with Claude Code via a PostHog Desktop task. Skills invoked: /writing-tests, /writing-pr-descriptions.
• First PR of a planned series on HogQL database-build performance; follow-ups cover fetch-phase I/O trims, deferred revenue-analytics views, and a possible per-team sources cache.
• The build-count test initially asserted only the call count, which single-call runners also satisfy on master; it now also asserts the build_shared_database timing so it fails without the threading.
• The trigger counter was added after checking the production histogram: build volume and duration are visible there, but call-path attribution was not.

---

Created with PostHog Desktop

[trunk-io]

😎 Merged successfully - details.

[github-actions]

🤖 CI report

⚠️ Trunk lane — backend Python lane

This PR is assigned to the backend Python lane. It runs backend Python tests and may merge in parallel with PRs in other lanes.

✅ Playwright — all passed

All tests passed.

View test results →

ℹ️ ClickHouse migration SQL — 1 migration(s)

ClickHouse migration SQL per cloud environment

• unset
  • all

    ALTER TABLE posthog_test.metric_series1
        ADD COLUMN IF NOT EXISTS `aggregation_temporality` LowCardinality(String) AFTER unit,
        ADD COLUMN IF NOT EXISTS `is_monotonic` Bool DEFAULT false AFTER aggregation_temporality

    ALTER TABLE posthog_test.metric_series1
        MODIFY TTL toDateTime(last_seen) + INTERVAL 90 DAY DELETE

    ALTER TABLE posthog_test.metric_samples1
        ADD COLUMN IF NOT EXISTS `count` UInt64 DEFAULT 1 AFTER value,
        ADD COLUMN IF NOT EXISTS `histogram_bounds` Array(Float64) AFTER count,
        ADD COLUMN IF NOT EXISTS `histogram_counts` Array(UInt64) AFTER histogram_bounds

    ALTER TABLE posthog_test.metric_series
        ADD COLUMN IF NOT EXISTS `aggregation_temporality` LowCardinality(String) AFTER unit,
        ADD COLUMN IF NOT EXISTS `is_monotonic` Bool DEFAULT false AFTER aggregation_temporality

    ALTER TABLE posthog_test.metric_samples
        ADD COLUMN IF NOT EXISTS `count` UInt64 DEFAULT 1 AFTER value,
        ADD COLUMN IF NOT EXISTS `histogram_bounds` Array(Float64) AFTER count,
        ADD COLUMN IF NOT EXISTS `histogram_counts` Array(UInt64) AFTER histogram_bounds

• US, EU, DEV
  • logs

    ALTER TABLE posthog_test.metric_series1
        ADD COLUMN IF NOT EXISTS `aggregation_temporality` LowCardinality(String) AFTER unit,
        ADD COLUMN IF NOT EXISTS `is_monotonic` Bool DEFAULT false AFTER aggregation_temporality

    ALTER TABLE posthog_test.metric_series1
        MODIFY TTL toDateTime(last_seen) + INTERVAL 90 DAY DELETE

    ALTER TABLE posthog_test.metric_samples1
        ADD COLUMN IF NOT EXISTS `count` UInt64 DEFAULT 1 AFTER value,
        ADD COLUMN IF NOT EXISTS `histogram_bounds` Array(Float64) AFTER count,
        ADD COLUMN IF NOT EXISTS `histogram_counts` Array(UInt64) AFTER histogram_bounds

    ALTER TABLE posthog_test.metric_series
        ADD COLUMN IF NOT EXISTS `aggregation_temporality` LowCardinality(String) AFTER unit,
        ADD COLUMN IF NOT EXISTS `is_monotonic` Bool DEFAULT false AFTER aggregation_temporality

    ALTER TABLE posthog_test.metric_samples
        ADD COLUMN IF NOT EXISTS `count` UInt64 DEFAULT 1 AFTER value,
        ADD COLUMN IF NOT EXISTS `histogram_bounds` Array(Float64) AFTER count,
        ADD COLUMN IF NOT EXISTS `histogram_counts` Array(UInt64) AFTER histogram_bounds

[pr-assigner-resolver-posthog]

👀 Auto-assigned reviewers

These soft owners were skipped because they only have minor changes here. Nothing blocks merge, so self-assign if you'd like a look:

• @PostHog/team-analytics-platform (posthog/hogql_queries/owners.yaml)

Soft owners come from each directory's owners.yaml and each product's product.yaml (resolved nearest-file-wins). The locator after each owner is the file that decided it. Generated files and lockfiles are ignored when deciding ownership.

[posthog]

🦔 PostHog Review reviewed this pull request

Found 0 must fix, 2 should fix, 1 consider.

Published 3 findings (view the review).

Resolved comments: 1 fixed, 1 already settled

[greptile-apps]

_{Reviews (1): Last reviewed commit: "perf(hogql): attribute database builds t..." | Re-trigger Greptile}

[Copilot]

Pull request overview

This PR extends the “shared HogQL database per runner run” optimization beyond insight runners to additional HogQL query runners that previously rebuilt the database inside the executor/printer, reducing redundant schema builds during a single calculation. It also adds a Prometheus counter to attribute database builds by the call path that triggered them, making regressions/stragglers visible in aggregate.

Changes:

• Thread a shared HogQLContext (backed by QueryRunner.shared_database) into more runners’ execute_hogql_query calls to avoid executor/printer rebuilding the database within the same run.
• Add hogql_database_build_total{trigger=...} attribution via a new counter incremented in Database.create_for(trigger=...), and thread trigger through executor/printer/shared/runner-context build sites.
• Add regression tests that assert a single Database.create_for per calculation for multiple runners and ensure shared-db SQL matches the executor-built SQL when modifiers reshape schema.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
posthog/hogql/query.py	Adds trigger="executor" when the executor builds a database, improving build attribution.
posthog/hogql/printer/utils.py	Adds trigger="printer" when the printer must build its own database.
posthog/hogql/editor_assist_metrics.py	Introduces HOGQL_DATABASE_BUILD_TOTAL counter (exported as hogql_database_build_total).
posthog/hogql/database/database.py	Adds trigger parameter to Database.create_for and increments the new counter per build.
posthog/hogql_queries/query_runner.py	Attributes shared-db builds (shared, shared_kill_switch) and runner-context builds (runner_context); exposes build_hogql_context() helper.
posthog/hogql_queries/hogql_query_runner.py	Passes shared context when not using a direct connection (connectionId is None).
posthog/hogql_queries/events_query_runner.py	Threads shared context into main events query and session-recordings check to avoid double builds.
posthog/hogql_queries/sessions_query_runner.py	Threads shared context into sessions execution path.
posthog/hogql_queries/sessions_timeline_query_runner.py	Switches manual HogQLContext(...) construction to build_hogql_context(...).
posthog/hogql_queries/property_values_query_runner.py	Switches manual HogQLContext(...) construction to build_hogql_context(...) for event/person paths.
posthog/hogql_queries/groups/groups_query_runner.py	Switches manual context to build_hogql_context(globals=...) while preserving required globals.
posthog/hogql_queries/actors_query_runner.py	Threads shared context into execution and validation/printing path to avoid redundant builds.
posthog/hogql_queries/insights/insight_actors_query_runner.py	Threads shared context into insight actors execution.
posthog/hogql_queries/test/test_query_runner.py	Adds parameterized test asserting one Database.create_for per calculation across multiple runners.
posthog/hogql_queries/test/test_hogql_query_runner.py	Adds test asserting shared-db SQL matches executor-built SQL when query-level modifiers reshape schema.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[trunk-io]

   

View Full Report ↗︎ ⋅ Docs

[posthog]

PostHog Review alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏

[posthog]

PostHog Review

Found 2 should fix, 1 consider.

[thmsobrmlr]

✅ (only checked product analytics related files)

[stamphog]

Approved.

This is an internal query-execution performance optimization (reusing an already-existing shared-database mechanism across more runners), not schema/data-model, API-contract, billing, auth, or CI/deploy territory, so my own read of the diff suffices. The riskiest part — making sure the shared build matches what a per-call build would have produced, including when query-level modifiers reshape the schema — is covered by new targeted tests (build-once assertions per runner, SQL-equivalence test under modifiers), and no reviewer bot or human raised an unresolved must-fix.

• Author wrote 8% of the modified lines and has 84 merged PRs in these paths (familiarity MODERATE).
• 👍 on the PR from greptile-apps[bot].

Gate mechanics and policy version

Gate		Result
prerequisites	✓	all clear
deny-list	✓	no deny categories matched
size	✓	79L, 13F substantive, 167L/16F incl. docs/generated/snapshots — within ceiling
tier	✓	T1-agent / T1d-complex (167L, 16F, single-area, perf)
stamphog 2.0.0b4		.stamphog/policy.yml @ aa36e22 · reviewed head aa36e22

[mariusandra]

Found one blocking authorization-state issue.

ActorsQueryRunner.run() assigns self.user = user before delegating to QueryRunner.run(). The base runner therefore sees no identity change and never calls _on_user_changed(), which clears _shared_database and _user_access_control. Before this PR, the actors execution path built a fresh database per calculation; context=self.build_hogql_context() at posthog/hogql_queries/actors_query_runner.py:204 now makes a second run reuse the first user's access-controlled schema.

Please let the base class own the user transition, or explicitly invalidate before the preassignment, and add a regression test that runs one ActorsQueryRunner as two users and asserts the database/access-control snapshot rebuilds. The new build-count tests otherwise pass locally (8/8).

A new stamphog review started for this PR — the fresh verdict replaces this approval.

[georgemunyoro]

@mariusandra Fixed in a4ccd8a — thanks for catching this.

ActorsQueryRunner.run() now performs the user transition itself instead of a bare pre-assignment: on an actual identity change it sets self.user and calls _on_user_changed(), which drops both _shared_database and _user_access_control. The base run() then sees no further change, so a same-user re-run still keeps a preloaded snapshot, and this runner's take-the-run-user-verbatim semantics (including user=None) are preserved.

Regression test as requested: test_actors_run_rebuilds_shared_state_on_user_change runs one ActorsQueryRunner as two users and asserts the shared database and access-control snapshot are rebuilt (and stay paired) on the second run. It fails on the previous head and passes with the fix. Full test_query_runner.py + test_actors_query_runner.py suites pass locally.

I checked the other runners for the same pattern — ActorsQueryRunner was the only run() override pre-assigning self.user; HogQLQueryRunner.run() forwards to super without touching it.

[stamphog]

Not approved yet — waiting on the conditions below.

Re-add the stamphog label to request another review once you have addressed this.

Gates denied due to an unresolved CHANGES_REQUESTED review from mariusandra identifying a real authorization/access-control bug (reusing a stale user's access-controlled database across users in ActorsQueryRunner), and nothing in later comments shows it was fixed.

• Author wrote 7% of the modified lines and has 84 merged PRs in these paths (familiarity MODERATE).
• 👍 on the PR from greptile-apps[bot].
• Unresolved CHANGES_REQUESTED review from @mariusandra: ActorsQueryRunner sets self.user before delegating to the base runner, so the shared database/access-control cache isn't invalidated on user change, potentially leaking one user's access-controlled schema to another user's query.
• This is security-sensitive (access control) territory with an active blocking review, so it cannot be auto-approved.

Gate mechanics and policy version

Gate		Result
prerequisites	✗	changes requested by: mariusandra
deny-list	✓	no deny categories matched
size	✓	86L, 13F substantive, 195L/16F incl. docs/generated/snapshots — within ceiling
tier	✓	T1-agent / T1d-complex (195L, 16F, single-area, perf)
stamphog 2.0.0b4		.stamphog/policy.yml @ a4ccd8a · reviewed head a4ccd8a

[mariusandra]

Second review complete. The original cross-user invalidation issue is fixed.

I also added two follow-up safeguards in 7a47931: conflicting modifier precedence now gets a rollout-safe cache namespace, and a matching preloaded access-control snapshot survives the Actors runner's first user transition.

Focused regression tests, Ruff, formatting, preflight, and repo-wide mypy all pass. No remaining merge-blocking findings.

[georgemunyoro]

/trunk merge

[deployment-status-posthog]

Deploy status

Environment	Status	Deployed At	Workflow
dev	✅ Deployed	2026-08-31 15:09 UTC	Run
prod-us	✅ Deployed	2026-08-31 15:22 UTC	Run
prod-eu	✅ Deployed	2026-08-31 15:23 UTC	Run