Repository navigation
feat(hogql): extend native cluster comparisons to typed queries - #116838
vdekrijger wants to merge 149 commits into
Conversation
Co-Authored-By: Claude Opus 5.5 <[email protected]>
…into codex/native-events-comparison
…sses-enums' into codex/native-events-comparison
# Conflicts: # posthog/hogql_queries/query_runner.py
… them with the rollout code - strip_events_route_modifiers() and with_pinned_events_route() return new dicts and copy only the source chain - the pin writes the resolved route into the innermost source, which every worker reads - the route helpers sit under the per-query rollout TODO so they are removed together - _RunMethod moves up to the other TypeVars
# Conflicts: # posthog/hogql_queries/query_runner.py
# Conflicts: # posthog/hogql_queries/query_runner.py
The skip lived in #120299, which is closed. The precompute tables become readable from the events cluster through Distributed tables, so native-routed queries take the same precompute path as legacy ones. Tests that asserted the skip now cover the table and workload choice on the direct-scan path.
|
@mariusandra, following up on the changes requested in the Codex review from Oct 20:
This PR is now based on master, after #120299 was closed. Both inline threads are resolved. Could you take another look when you have a moment? 🤖 Posted by Claude Code |
Each allowed kind now runs through the query API with the native opt-in, and every ClickHouse call it makes must use the native workload and never read the legacy events table. Both native guards share one error message, and the type allowlist sits next to its execution modes.
…ests A multi-series trends breakdown now runs on the native route, and the experiment test checks every ClickHouse call instead of the last one. The query runner keeps its original settings import, the execution modes are a frozenset, and dead native-host settings overrides are gone from the tests.
|
@PostHog review |
|
@vdekrijger PostHog Review is reviewing this pull request. |
There was a problem hiding this comment.
PostHog Review · Standard · reviewed 2d363cc: 2 Should fix.
This is review round 7 on this pull request. When findings keep coming back in the same code, the approach is usually the problem. Before the next fix, check whether a different design would remove the whole class of findings.
| ActorsQuery, | ||
| InsightActorsQuery, | ||
| InsightActorsQueryOptions, | ||
| PropertyValuesQuery, |
There was a problem hiding this comment.
P2 · Bypass the property-values aggregate for native comparisons
With property-values-table enabled, a native event-property lookup takes _calculate_event_from_table(), whose helper reads property_values_distributed through default-cluster sync_execute. It ignores the native route and returns auxiliary-cluster suggestions even when native events contain different values; bypass this fast path for native comparisons.
| PropertyValuesQuery, | ||
| ExperimentQuery, | ||
| ExperimentExposureQuery, | ||
| WebOverviewQuery, |
There was a problem hiding this comment.
P2 · Route live channel boundary scans to the native cluster
For a native WebOverviewQuery with lazy precompute enabled and a $channel_type filter, execute_channel_period_read() compiles live boundary scans with native modifiers but calls sync_execute without a workload. Those scans read the main cluster's events_json, producing incorrect comparisons when datasets differ; pass the native workload to this read.
|
Superseded by #122489, which merged on Oct 26 and routes events_json reads to the events cluster with one rule instead of the per-query opt-in this PR carried. Closing; the branch stays. |
Problem
Eligible callers can compare only raw SQL (
HogQLQuery) on the native events cluster after the pilot in #115591. This layer lets typed insights opt in too, now that their child reads stay on the selected cluster.Stack, bottom to top: #120297 child queries follow the route and #120298 all-time lookups follow the route, both merged → this PR → #115778 async → #119468 user flag → #122031 precompute builds read
events_json. Separate PRs on master: #120295 pool retirement, #120296 batch-export schema.Changes
useNewEventsCluster. Eligibility and per-query opt-in remain required.run()and at enqueue. Async arrives in #115778.eventsuntil #122031 moves the builds. The precompute tables stay on the main cluster and become readable from the events cluster through Distributed tables. HogQL-level preaggregation transforms stay off for native queries, as in the SQL pilot.Note
A native opt-in with
refresh=blockingnow returns results instead of a 400.How did you test this code?
Test rationale: Funnels, retention, paths, paths v2, stickiness, lifecycle, events, property values, Web overview, Web stats, Web goals and Web page URL search run through the query API with the native opt-in, and every ClickHouse call they make must use the native workload and never read the legacy
eventstable. A multi-series trends breakdown (through the threaded series fan-out), experiments, actors and their options run at runner level with the same check. Product kinds are tested in their own products, as the product-isolation invariant requires. Experiments, funnel breakdown options, specialized actor sources, actor source reuse and the Web Vitals async guard have their own cases. The routing helpers they rely on are tested in the layers below.Not run: production validation. Locally, ClickHouse-backed funnel-options tests could not run because the local ClickHouse lacks a funnel UDF. CI covers them.
Release status
Docs update
None.
🤖 Agent context
Autonomy: Human-driven (agent-assisted).
Agent: OpenAI Codex, model
gpt-6.1-sol; review fixes and the stack split by Claude Code, modelclaude-opus-5-5.Skills: posthog-pr-finish, receiving-code-review, writing-tests, writing-code-comments, writing-pr-descriptions, running-ci-preflight, stacking-prs, merging-prs.
#120299, which made native queries skip precompute, is closed. Its changes were reverted from this branch and its skip-specific tests removed or moved to the direct-scan path. Typed support moved out of the SQL pilot into this layer. The routing fixes it needs now live in the three layers below, and the pool and batch-export changes moved to their own PRs. Committed test identities are invented.
Searches for the user-change hook and cached source runner found no open duplicate PR.