8000 fix: Fix virtual properties with PoE by robbie-c · Pull Request #33703 · PostHog/posthog · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix: Fix virtual properties with PoE #33703

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 16, 2025

Conversation

robbie-c
Copy link
Member
@robbie-c robbie-c commented Jun 15, 2025

Important

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

Problem

Virtual properties didn't work with PoE enabled. This doesn't affect local dev but is used a lot in prod.

Changes

Fix this issue, get PoE properties working

Did you write or update any docs for this change?

How did you test this code?

Added a test

@robbie-c robbie-c marked this pull request as ready for review June 15, 2025 19:20
@robbie-c robbie-c enabled auto-merge (squash) June 15, 2025 19:20
Copy link
Contributor
@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Summary

Fixed virtual properties functionality when Persons on Events (PoE) is enabled by adding virtual fields to the PoE table structure.

  • Added $virt_initial_referring_domain_type and $virt_initial_channel_type virtual fields to PoE table in hogql/database/database.py
  • Added parameterized tests in hogql/database/schema/test/test_persons.py to verify virtual property behavior across all PoE modes
  • Improved code clarity by introducing early poe variable definition for VirtualTable to reduce duplicate casts
  • Added test coverage specifically for virtual property behavior in trends with PoE enabled

2 files reviewed, no comments
Edit PR Review Bot Settings | Greptile

@robbie-c robbie-c requested review from a team, lricoy and jabahamondes and removed request for a team June 15, 2025 20:04
@robbie-c robbie-c merged commit 34f7487 into master Jun 16, 2025
105 checks passed
@robbie-c robbie-c deleted the fix/fix-virtual-properties-with-poe branch June 16, 2025 00:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0