fix(hogql): convert a missing currency without dividing by zero

[lricoy]

Problem

• A cost or revenue query fails with Division by zero (Code 153) when any source row has no currency, for example an Apple Search Ads local_spend row without one.
• The whole query fails, not just that row. That breaks the marketing cost precompute and live cost reads for such a source.
• convertCurrency already guards a zero rate. A NULL currency makes the rate lookup return NULL rather than the default 0, so NULL = 0 is not true and the divisor stays NULL.
• ClickHouse then evaluates divideDecimal on the zero under the NULL row and throws.
• Separately, an action step with an element filter on $el_text fails to compile with property_to_expr for type element not implemented for key $el_text.
• The action editor saves that key as text, but actions saved through other paths still carry $el_text.

Changes

• A row with no currency now converts to 0, the same as an unknown currency, and the rest of the query runs.
• The printed convertCurrency wraps the source rate lookup in ifNull(…, 0).
• Element property $el_text now compiles the same as text: a match against elements_chain_texts.
• That text covers the clicked element and its ancestors, like text does, not only the $el_text event property.
• Already-saved actions with such a step match in queries after deploy. Their realtime bytecode updates on their next save.
• A cost row with no currency, as in some Apple Search Ads rows, now counts as 0 spend instead of failing the query. Reading it as the base currency would misstate spend whenever the ad account's currency differs from the team's.
• Mechanical: the exact-SQL printer tests change. Query snapshots containing convertCurrency (revenue analytics, trends) change by the same ifNull wrap; CI regenerates them.

How did you test this code?

• New ClickHouse test: convertCurrency over a NULL and a valid source currency returns 0 and the converted value. Without the fix it fails with the same Division by zero seen in production.
• The existing element-property test now also runs with the $el_text key.
• Not regenerated locally: the revenue analytics snapshot files. Locally generated SQL for those tests differs from the committed snapshots in unrelated join shapes, so CI's snapshot job owns them.

Release status

• No feature flag controls this change
• This change is behind a feature flag and is not available to users
• This change makes a previously flagged feature available to everyone

Automatic notifications

• Publish to changelog?

Docs update

None.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: Claude Code, Claude Opus 5.5

• Skills invoked: /writing-tests, /reviewing-with-coderabbit, /writing-pr-descriptions.
• CodeRabbit CLI pass: no findings.
• A multi-agent QA review (general correctness, adversarial, database, data integrity, reliability, compatibility) rated it low risk. The $el_text notes above came from it. Its suggested base-currency fallback was tried and reverted after review, because it misstates spend for accounts in another currency.
• Root cause reproduced against local ClickHouse: a NULL divisor inside if(rate = 0, 1, rate) still throws; ifNull(rate, 0) = 0 does not.

🤖 Generated with Claude Code

[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.

✅ Duplication (Python) — clean

New Python code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

✅ Duplication (TypeScript) — clean

New TypeScript code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

⚠️ Playwright — 1 flaky

🎭 Playwright report · View test results →

⚠️ 1 flaky test:

• Materialize view pane (chromium)

These issues are not necessarily caused by your changes.
Annoyed by this section? Help fix flakies and failures and it will go green!

⚠️ Backend snapshots — 2 updated (2 modified, 0 added, 0 deleted)

Query snapshots: Backend query snapshots updated

Changes: 2 snapshots (2 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 →

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: PostHog/posthog/.coderabbit.yaml

Review profile: QUIET

Plan: Enterprise

Run ID: 6a9fbcd9-05da-4277-bbf1-130a34b4c6c6

📥 Commits

Reviewing files that changed from the base of the PR and between 39a2743 and 634bb91.

📒 Files selected for processing (5)

• products/marketing_analytics/backend/hogql_queries/__snapshots__/test_marketing_analytics_table_query_runner_business.ambr
• products/marketing_analytics/backend/hogql_queries/__snapshots__/test_marketing_analytics_table_query_runner_compare.ambr
• products/product_analytics/backend/hogql_queries/trends/test/__snapshots__/test_math_property_revenue_currency_timestamp.ambr
• products/revenue_analytics/backend/views/test/__snapshots__/test_mrr_views.ambr
• products/revenue_analytics/backend/views/test/__snapshots__/test_mrr_views.new_events_schema.ambr

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

---

📝 Walkthrough

Walkthrough

Currency conversion now treats a NULL source-currency rate lookup as Decimal64 zero. Element text filters accept both text and $el_text keys. Marketing analytics adapter expressions substitute a base currency when currency values are empty or null.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to ad862

The currency and element-filter changes appear consistent with their intended behavior, and no concrete merge-blocking issue was found. Focused snapshot tests were not run in this environment.

Architecture Summary

Architecture risk: 🔵 Low · up to 634bb

The change affects 2 systems.

Changed systems: products, posthog

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

• observed — products (service) was modified; 10 changed files map to changed impact.
• observed — posthog (service) was modified; 6 changed files map to changed impact.

Before / after behavior

• observed — Modified behavior in posthog/hogql/printer/clickhouse.py: The from_rate lookup now converts a NULL dictionary result to toDecimal64(0, scale), in addition to its existing zero default. This lets the existing zero-rate branch handle NULL currency lookups as zero.
• observed — Modified behavior in posthog/hogql/printer/test/test_printer.py: The expected SQL for conversion with an explicit date now treats a NULL dictionary rate as Decimal64 zero in the zero-rate check and denominator fallback.
• observed — Modified behavior in posthog/hogql/printer/test/test_printer.py: The expected SQL for conversion without a date makes the same NULL-to-zero adjustment, using today() for dictionary lookups.
• observed — Modified behavior in posthog/hogql/property.py: The element text-filter branch now accepts "$el_text" as well as "text"; both keys use the existing elements_chain_texts comparison.

🚥 Pre-merge checks | ✅ 1

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description follows the required template. It clearly states the problem, user impact, code changes, tests, snapshot limitation, release status, documentation status, and agent context.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Note

Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.

🟡 Other comments (1)

posthog/hogql/property.py-1553-1553 (1)

1553-1553: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Allow $el_text in the typed element filter model.

ElementPropertyFilter.key uses Key10, which does not include $el_text. A typed element filter with this key is rejected before property_to_expr reaches the new branch. Add $el_text to the source enum or use an action input model that permits it.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository: PostHog/posthog/.coderabbit.yaml

Review profile: QUIET

Plan: Enterprise

Run ID: f1391f38-7451-46ee-ab10-52393b6df6c4

📥 Commits

Reviewing files that changed from the base of the PR and between c006603 and 90d1178.

📒 Files selected for processing (5)

• posthog/hogql/printer/clickhouse.py
• posthog/hogql/printer/test/test_printer.py
• posthog/hogql/property.py
• posthog/hogql/test/test_property.py
• posthog/hogql/test/test_query.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

[greptile-apps]

[Medium risk] Fixes currency conversion to handle missing values without errors.

The PR should not merge until Apple Search Ads spend with a missing row currency is attributed to the correct account currency.

_{Reviews (2) · Last reviewed commit: "fix(marketing-analytics): read cost rows..."}

[trunk-io]

   

Failed Test	Failure Summary	Logs
SQL Editor › Basic flow › Materialize view pane	The test failed because the expected element was not visible.	Logs ↗︎

View Full Report ↗︎ ⋅ Docs

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

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

[lricoy]

/trunk merge

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

[stamphog]

Approved.

This is a small, contained fix to query printing: the currency-conversion change wraps the source rate lookup in ifNull, and the element filter now treats the $el_text key like text. Tests cover both, and a teammate approved an earlier commit. The Greptile concern about a base-currency fallback applies to an approach the author reverted, and the CodeRabbit note about the typed filter enum is minor.

• Author wrote 0% of the modified lines and has 9 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	✓	10L, 3F substantive, 2311L/13F incl. docs/generated/snapshots — within ceiling
tier	✓	T1-agent / T1d-complex (2311L, 13F, two-areas, fix)
stamphog 2.3.0		.stamphog/policy.yml @ ad8625b · reviewed head ad8625b