refactor(hogql): redesign events predicate pushdown on the lowering architecture

[aspicer]

Problem

Events predicate pushdown is a query optimization. When a query reads events with a JOIN and a LIMIT, ClickHouse otherwise scans the whole events table, fans it out across the join, and applies the LIMIT last. Pushdown wraps the events scan in a subquery that filters and LIMITs first, so the join only ever sees the small pre-filtered slice:

FROM events JOIN sessions WHERE properties.x = 'y' LIMIT 100
-- becomes
FROM (SELECT <needed cols> FROM events WHERE properties.x = 'y' LIMIT 100) AS events JOIN sessions

The original implementation predated the lowering rearchitecture: it decided which physical column the subquery had to expose by predicting the printer's materialized-column resolution — duplicating logic that now lives in the ClickHouse physical pass. This rewrites it on the new architecture so it never predicts physical columns. Stacked on the clean cutover (#61943).

Changes

Pushdown now runs between logical lowering and the ClickHouse physical pass, so it works on the dialect-neutral form — a properties.x read is a JSONFieldAccess, not yet a materialized column. It does two things:

1. Pushes the events-only filter predicates into the subquery's WHERE, verbatim. They already reference the real events table, so the physical pass optimizes them (materialized columns, skip indexes) in place.
2. Hoists everything else the outer query still needs from events into the subquery's SELECT. Each maximal subexpression that depends only on the events table — a column, a property read, JSONHas(properties, k), match(uuid, …), upper(properties.x) — is projected under a name and the outer reference rewritten to read that one column. One uniform rule: touches only events → project it; touches a joined table → leave it outside. No physical-column inspection, no per-function special-casing — the physical pass resolves each JSONFieldAccess inside the subquery afterward.

It is a pure optimization: every rewrite is result-equivalent, and any unexpected error leaves the query untouched (it just runs flat).

No carve-outs; one printer fix instead. The old version had bespoke join-key/value modes and a property-key resolver, all to work around a subquery column being typed blanket-nullable — which made the printer ifNull-wrap a hoisted join key, so ClickHouse rejected the join ("Cannot determine join keys"). The root cause is fixed in the printer: BasePrinter._is_type_nullable now reads a subquery column's real nullability through the subquery's type instead of defaulting to nullable. So a non-nullable value isn't wrapped, a join key hoists like any other column, and a property key wraps only when genuinely nullable. Every carve-out is deleted; everything hoists through one path.

Removed LazyTypeDetector. This guard bailed pushdown if it found an unresolved lazy join (person.*, session.*). It is dead: pushdown runs after lazy joins resolve; a query whose only "join" is an implicit lazy one isn't eligible (pushdown requires a real JOIN); and the uniform rule already leaves any lazy reference in the outer query (it isn't the events table, so it counts as foreign). Confirmed it never fires across the whole suite before removing it.

How did you test this code?

I'm an agent (Claude Code). Automated tests I ran:

• The full pushdown suite (test_events_predicate_pushdown.py) — 135 pass, including the ClickHouse execution-equivalence tests that create events, run the query with pushdown on and off, and assert identical results (plain / materialized / dmat / property-group columns, plus self-join, fan-out, outer-JSONHas, and a materialized-property join key).
• Confirmed LazyTypeDetector never fires in the full pipeline before removing it.

Automatic notifications

• Publish to changelog?
• Alert Sales and Marketing teams?

Docs update

No user-facing change — this is an internal query-compilation optimization, so no docs update is needed.

🤖 Agent context

Authored by Claude Code (Opus), driven by Sandy. Branch 16 of the HogQL lowering rearchitecture stack, stacked on the clean cutover (#61943).

Key decisions: converged on the single maximal events-only subexpression rule after noticing that JSONHas and match() are no different from any pure function over events columns — they should hoist by one rule, not per-function cases. The nullable-join-key fix went through a couple of iterations (explicit join-key modes, then a property-key resolver) before landing on the printer's nullability check, which let every carve-out be deleted. The LazyTypeDetector removal came from checking the guard against the pass's actual pipeline position: it protects against a precondition that can no longer occur once pushdown runs after lazy resolution.

_build_subquery still hand-builds the subquery's container type rather than calling resolve_types, because the resolver runs before lowering and would null out the lowered JSONFieldAccess types. This is documented in the module docstring.

[github-actions]

Hey @aspicer! 👋

It looks like your git author email on this PR isn't your @posthog.com address (aspicer@gmail.com). Since you're on the PostHog team, it's worth pointing your local git author email at your @posthog.com address. Why it matters:

• Consistent work identity in git history — internal tooling that attributes commits to team members keys off your @posthog.com address.
• Keeps team contributions easy to tell apart from external community ones when scanning history.

You can fix it for this repo with:

git config user.email "you@posthog.com"

Or set it globally with git config --global user.email "you@posthog.com". No need to redo this PR — just a nudge for next time. 🙂

[greptile-apps]

Comments Outside Diff (1)

1. posthog/hogql/transforms/test/test_events_predicate_pushdown.py, line 856-900 (link)

   Several adjacent tests could be parameterized

   The team prefers parameterised tests. test_property_is_projected_under_its_dunder_name, test_nested_property_uses_dunder_joined_name, and test_direct_columns_are_projected / test_direct_column_is_projected_under_its_database_name all follow the same _run → assert X in hoister.projections shape with different inputs. A single @parameterized.expand table would cover these four without duplicating the assertion logic.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: posthog/hogql/transforms/test/test_events_predicate_pushdown.py
   Line: 856-900
   
   Comment:
   **Several adjacent tests could be parameterized**
   
   The team prefers parameterised tests. `test_property_is_projected_under_its_dunder_name`, `test_nested_property_uses_dunder_joined_name`, and `test_direct_columns_are_projected` / `test_direct_column_is_projected_under_its_database_name` all follow the same `_run` → `assert X in hoister.projections` shape with different inputs. A single `@parameterized.expand` table would cover these four without duplicating the assertion logic.
   
   How can I resolve this? If you propose a fix, please make it concise.

   Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix All With AI

Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
posthog/hogql/transforms/events_predicate_pushdown.py:264-270
**Projection name collision between `properties.x` and `poe.properties.x`**

`_preferred_name` for `JSONFieldAccess` uses only `node.keys`, discarding `node.expr` (the source column). After lowering, `properties.$browser` becomes `JSONFieldAccess(expr=Field("properties"), keys=["$browser"])` and `poe.properties.$browser` becomes `JSONFieldAccess(expr=Field("person_properties"), keys=["$browser"])`. Both resolve to the same name `"$browser"`, so the second to be interned silently reuses the first's projection. Any query that selects both `properties.x` and `poe.properties.x` under the same key, with pushdown applied, will read the wrong blob for one of them.

The old `EventsFieldCollector` collected `properties` and `person_properties` as distinct entries. The fix is to qualify the name with the source column: e.g., join over `node.expr.chain + node.keys` when `node.expr` is a `Field`, producing `"properties__$browser"` vs `"person_properties__$browser"`. There is no test covering `SELECT properties.x, poe.properties.x FROM events WHERE … LIMIT n` with a session join — the existing `test_exec_poe_person_property_filter` only pushes the person-property predicate into WHERE, so the hoisted-SELECT collision path is never exercised.

### Issue 2 of 3
posthog/hogql/transforms/test/test_events_predicate_pushdown.py:826-834
**POE property access no longer tested in the unit hoister suite**

The removed `TestEventsFieldCollector` included dedicated tests for `poe.properties`, `poe.id`, and `poe.created_at`. `TestEventsSubexprHoister` has no equivalent cases, which leaves the projection-name collision (same key via `properties.*` and `poe.properties.*`) untested. Adding at minimum a `test_poe_property_and_event_property_same_key_project_separately` case to `_run` would catch this before a regression reaches production.

### Issue 3 of 3
posthog/hogql/transforms/test/test_events_predicate_pushdown.py:856-900
**Several adjacent tests could be parameterized**

The team prefers parameterised tests. `test_property_is_projected_under_its_dunder_name`, `test_nested_property_uses_dunder_joined_name`, and `test_direct_columns_are_projected` / `test_direct_column_is_projected_under_its_database_name` all follow the same `_run` → `assert X in hoister.projections` shape with different inputs. A single `@parameterized.expand` table would cover these four without duplicating the assertion logic.

_{Reviews (1): Last reviewed commit: "refactor(hogql): redesign events predica..." | Re-trigger Greptile}

[tests-posthog]

Query snapshots: Backend query snapshots updated

Changes: 30 snapshots (30 modified, 0 added, 0 deleted)

What this means:

• Query snapshots have been automatically updated to match current output
• These changes reflect modifications to database queries or schema

Next steps:

• Review the query changes to ensure they're intentional
• If unexpected, investigate what caused the query to change

Review snapshot changes →

[tests-posthog]

⏭️ Skipped snapshot commit because branch advanced to b83dfa5 while workflow was testing df893d8.

The new commit will trigger its own snapshot update workflow.

If you expected this workflow to succeed: This can happen due to concurrent commits. To get a fresh workflow run, either:

• Merge master into your branch, or
• Push an empty commit: git commit --allow-empty -m 'trigger CI' && git push

[tests-posthog]

⏭️ Skipped snapshot commit because branch advanced to 8da411e while workflow was testing b83dfa5.

The new commit will trigger its own snapshot update workflow.

If you expected this workflow to succeed: This can happen due to concurrent commits. To get a fresh workflow run, either:

• Merge master into your branch, or
• Push an empty commit: git commit --allow-empty -m 'trigger CI' && git push

[tests-posthog]

Query snapshots: Backend query snapshots updated

Changes: 1 snapshots (1 modified, 0 added, 0 deleted)

What this means:

• Query snapshots have been automatically updated to match current output
• These changes reflect modifications to database queries or schema

Next steps:

• Review the query changes to ensure they're intentional
• If unexpected, investigate what caused the query to change

Review snapshot changes →

[tests-posthog]

Query snapshots: Backend query snapshots updated

Changes: 39 snapshots (39 modified, 0 added, 0 deleted)

What this means:

• Query snapshots have been automatically updated to match current output
• These changes reflect modifications to database queries or schema

Next steps:

• Review the query changes to ensure they're intentional
• If unexpected, investigate what caused the query to change

Review snapshot changes →

[tests-posthog]

Query snapshots: Backend query snapshots updated

Changes: 6 snapshots (6 modified, 0 added, 0 deleted)

What this means:

• Query snapshots have been automatically updated to match current output
• These changes reflect modifications to database queries or schema

Next steps:

• Review the query changes to ensure they're intentional
• If unexpected, investigate what caused the query to change

Review snapshot changes →