-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
feat(hogql): add funnel correlation queries #20581
Conversation
Size Change: 0 B Total Size: 813 kB ℹ️ View Unchanged
|
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
1299894
to
00a2cdc
Compare
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated2 snapshot changes in total. 0 added, 2 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated2 snapshot changes in total. 0 added, 2 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated3 snapshot changes in total. 0 added, 3 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
@@ -578,6 +579,7 @@ class HogQLFunctionMeta: | |||
"count": HogQLFunctionMeta("count", 0, 1, aggregate=True), | |||
"COUNT": HogQLFunctionMeta("count", 0, 1, aggregate=True), | |||
"countIf": HogQLFunctionMeta("countIf", 1, 2, aggregate=True), | |||
"countDistinctIf": HogQLFunctionMeta("countIf", 1, 2, aggregate=True), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For consistency we should probably support all -Distinct functions #20662.
Problem
Funnels need to be converted to HogQL (see the funnels section in PostHog/meta#130).
Changes
This PR converts the funnel correlation queries (events, events with properties and properties) to HogQL. It isn't perfect and notably I needed to comment out some materialized column tests (it does always extract the json) and duplicate a CTE (HogQL doesn't support CTEs with union queries). Given that this is a low-volume query, I recon getting it in & finishing up the HogQL rewrite take priority.
How did you test this code?
Converted existing tests