-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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(insights): launch funnels as a Clickhouse UDF behind a feature flag #23587
Merged
+11,868
−308
Merged
Changes from 124 commits
Commits
Show all changes
201 commits
Select commit
Hold shift + click to select a range
f351508
udf
aspicer e4a0979
working
aspicer 5fd74bf
merge
aspicer 2322db9
Merge remote-tracking branch 'origin/master' into aspicer/udf
aspicer 2cb95db
funnels
aspicer 69674bc
sorted group array
aspicer e6a4e6d
working query
aspicer 1401ffa
3 level funnels
aspicer a70ffc2
loading conversion window limit
aspicer abbb018
working with array
aspicer d8b319b
timestamps
aspicer fdd66e7
timestamps
aspicer 9bf5dee
working
aspicer 2af6b78
passing breakdown through
aspicer e497417
passing
aspicer c41cd48
breakdowns passing through
aspicer 2bd965b
string breakdown
aspicer 1c45b7b
breakdowns and stesting
aspicer 17ac128
time tracking
aspicer 6288035
need to make timestamps
aspicer cb1962f
returning values
aspicer f27e4b7
timings working
aspicer 4e69b14
median
aspicer 89dceb0
nan
aspicer 3e43a8f
working array breakdowns
aspicer dc9a0f4
hm
aspicer 91d8c4e
test case
aspicer 9e5ce4c
step
aspicer d13e5c8
36 tests failing
aspicer 65de38a
order by
aspicer 10aa1dc
hm
aspicer afa3149
mm
aspicer 7d25857
30
aspicer 7ad5736
mm
aspicer 594ac94
exclusions
aspicer 1a2288c
exclusions
aspicer 2045839
results
aspicer b9558a9
8 failing
aspicer 0aec55c
udf
aspicer 9197277
working
aspicer c30b651
merge
aspicer 2ccbd0f
funnels
aspicer 99199f7
sorted group array
aspicer dd681a7
working query
aspicer aee3589
3 level funnels
aspicer 4d8351d
loading conversion window limit
aspicer 4f85e01
working with array
aspicer 18a0e50
timestamps
aspicer 57da140
timestamps
aspicer 07af8e4
working
aspicer c78a4f7
passing breakdown through
aspicer 44bffbb
passing
aspicer 615b06f
breakdowns passing through
aspicer c796027
string breakdown
aspicer 72d4d56
breakdowns and stesting
aspicer 5bac5f5
time tracking
aspicer 1312dfa
need to make timestamps
aspicer 5ade8a9
returning values
aspicer 80288c3
timings working
aspicer 5e8c2f1
median
aspicer b594037
nan
aspicer 23f0442
working array breakdowns
aspicer df85473
hm
aspicer 8f4789c
test case
aspicer 41bdd2d
step
aspicer 42a5af8
36 tests failing
aspicer f609f6a
order by
aspicer d72f923
hm
aspicer a0db6b5
mm
aspicer 034b79a
30
aspicer 9e6c501
mm
aspicer bd90420
exclusions
aspicer 0535c5e
exclusions
aspicer 8699b7e
results
aspicer 9833c93
8 failing
aspicer 7116889
false
aspicer 4228165
cohorts
aspicer d7784f7
5 failing
aspicer e262538
funnel no step
aspicer 49e9177
prop vals
aspicer cc263e4
3 tests failing
aspicer 21886d2
working
aspicer 63c981c
hm
aspicer 5814d9a
permutations
aspicer 62eafe5
working funnel
aspicer fd1f4ec
tests
aspicer feec377
Update query snapshots
github-actions[bot] 773264b
mypy fixes and ast literal eval
aspicer 5319398
removed outer loop
aspicer a8db5a8
Merge remote-tracking branch 'origin/master' into aspicer/udf
aspicer 79426b4
Merge branch 'aspicer/udf' of github.com:PostHog/posthog into aspicer…
aspicer 84170a8
cleanup
aspicer ec4ff23
Update UI snapshots for `chromium` (2)
github-actions[bot] 4e0ed71
remove extra
aspicer 1b022e5
tests
aspicer 910f37f
Merge branch 'aspicer/udf' of github.com:PostHog/posthog into aspicer…
aspicer eacfee6
Update query snapshots
github-actions[bot] 3d269bc
Update query snapshots
github-actions[bot] ca30e00
making waves
aspicer 2d3ba27
py
aspicer bdf6cc2
Merge branch 'aspicer/udf' of github.com:PostHog/posthog into aspicer…
aspicer 256b423
strict funnels
aspicer e5bc5bb
Update query snapshots
github-actions[bot] 18e5eed
Update query snapshots
github-actions[bot] 10fa2dd
max step
aspicer ad83187
Merge branch 'aspicer/udf' of github.com:PostHog/posthog into aspicer…
aspicer 3bd07a5
typing
aspicer e5f99af
Merge remote-tracking branch 'origin/master' into aspicer/udf
aspicer 91df7b1
Cleanup
aspicer f21eecd
revert celery threads
aspicer b6e1a86
hm
aspicer 88ae18d
clean up
aspicer 81b8ed0
Merge branch 'aspicer/udf' of github.com:PostHog/posthog into aspicer…
aspicer 8525554
merge
aspicer fb650be
Update UI snapshots for `chromium` (1)
github-actions[bot] 08e4f6a
Update UI snapshots for `chromium` (2)
github-actions[bot] e559aac
Merge remote-tracking branch 'origin/master' into aspicer/udf
aspicer ba6a9b3
mm
aspicer 4c9cdf7
Merge remote-tracking branch 'origin/master' into aspicer/udf
aspicer d00c4f2
bad merge
aspicer 92d9203
Merge branch 'aspicer/udf' of github.com:PostHog/posthog into aspicer…
aspicer 4b13a4e
tests
aspicer b671b69
Update query snapshots
github-actions[bot] 50a20e7
docker compose full
aspicer 2054fc4
tests
aspicer 9ad6666
entrypoint
aspicer 031b4fe
Update query snapshots
github-actions[bot] dd8dea0
fix mypy
aspicer 4824c3c
fix mypy
aspicer d7c2c94
Merge branch 'aspicer/udf' of github.com:PostHog/posthog into aspicer…
aspicer de2a2cb
having
aspicer 35f2ad0
Update query snapshots
github-actions[bot] 65f5944
Merge remote-tracking branch 'origin/master' into aspicer/udf
aspicer 519ec76
Merge branch 'aspicer/udf' of github.com:PostHog/posthog into aspicer…
aspicer e7b0ef4
Update query snapshots
github-actions[bot] 0acb3b5
Update UI snapshots for `chromium` (1)
github-actions[bot] fbd5848
Update query snapshots
github-actions[bot] 6e8cc02
Update UI snapshots for `chromium` (1)
github-actions[bot] 47e59ca
trends wip
aspicer a336eb0
udf returning timestamps and success / failure
aspicer d2115ff
start of interval logic
aspicer 2d34776
returning trends
aspicer 64afdd3
funnels query runner
aspicer 4f3c247
max steps ordered
aspicer db59c81
min
aspicer 9f4228a
new test files
aspicer 2f61308
breakdowns, working on join
aspicer 187d37d
breakdowns working
aspicer a97fefd
undo test rig
aspicer aef455c
breakdowns working, 5 tests failing for funnel trends
aspicer 42bed52
Update query snapshots
github-actions[bot] 041ee84
Update query snapshots
github-actions[bot] ce8ca2e
timezone parsing correct
aspicer 89b0371
Merge branch 'aspicer/trends-udf' of github.com:PostHog/posthog into …
aspicer ebadbdf
qq
aspicer f3731be
Update query snapshots
github-actions[bot] cb253ec
testing
aspicer 93d69db
Merge branch 'aspicer/trends-udf' of github.com:PostHog/posthog into …
aspicer 96bde73
Update query snapshots
github-actions[bot] 3057194
switch to json
aspicer 3430f4a
lala
aspicer f97d8f5
Update query snapshots
github-actions[bot] 39b7220
Update query snapshots
github-actions[bot] 2b3dbf6
reduce files
aspicer 112444a
Merge branch 'aspicer/trends-udf' of github.com:PostHog/posthog into …
aspicer b7ac533
timezones working
aspicer 6dfb5aa
Update query snapshots
github-actions[bot] 8c07c63
Update query snapshots
github-actions[bot] a8f250b
merge
aspicer 71f6177
remove pypy
aspicer 775c2e5
fix test
aspicer 03a0617
Update query snapshots
github-actions[bot] d940fc5
i c
aspicer 8dc53dc
Merge branch 'aspicer/trends-udf' of github.com:PostHog/posthog into …
aspicer bcc3cbd
Update query snapshots
github-actions[bot] 52c6056
Update query snapshots
github-actions[bot] 38ce36e
Update query snapshots
github-actions[bot] fbfd7b1
add the ability to toggle a udf
aspicer fb5ee7c
remove test function
aspicer e65617c
Merge branch 'aspicer/udf' of github.com:PostHog/posthog into aspicer…
aspicer 062b660
mypy
aspicer 839274d
udf
aspicer 2e2521d
fix typo
aspicer 18c9ccf
Update query snapshots
github-actions[bot] fbb1407
Merge remote-tracking branch 'origin/master' into aspicer/udf
aspicer 6ec5de8
Merge branch 'aspicer/udf' of github.com:PostHog/posthog into aspicer…
aspicer aad3b15
tmate
aspicer a981ea9
timeout
aspicer 62d07b7
tet
aspicer 0fd6128
bump temporalio
aspicer ded3751
always wait for temporal
aspicer 209a722
Update UI snapshots for `chromium` (2)
github-actions[bot] cd25fa1
Update UI snapshots for `chromium` (2)
github-actions[bot] 5b8313d
Merge remote-tracking branch 'origin/master' into aspicer/udf
aspicer ebf6c56
Merge branch 'aspicer/udf' of github.com:PostHog/posthog into aspicer…
aspicer f245dfc
clean up actions and rename
aspicer a6f0c21
staged
aspicer afacb91
Update query snapshots
github-actions[bot] 329cd4a
Merge remote-tracking branch 'origin/master' into aspicer/udf
aspicer f6fa7ab
Merge branch 'aspicer/udf' of github.com:PostHog/posthog into aspicer…
aspicer 32fd423
Update query snapshots
github-actions[bot] File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
#!/bin/bash | ||
set -e | ||
|
||
apk add python3 | ||
cp -r /idl/* /var/lib/clickhouse/format_schemas/ |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,129 @@ | ||
<functions> | ||
<function> | ||
<type>executable</type> | ||
<name>aggregate_funnel</name> | ||
<return_type>Array(Tuple(Int8, Nullable(String), Array(Float64)))</return_type> | ||
<argument> | ||
<type>UInt8</type> | ||
<name>num_steps</name> | ||
</argument> | ||
<argument> | ||
<type>UInt64</type> | ||
<name>conversion_window_limit</name> | ||
</argument> | ||
<argument> | ||
<type>String</type> | ||
<name>breakdown_attribution_type</name> | ||
</argument> | ||
<argument> | ||
<type>String</type> | ||
<name>funnel_order_type</name> | ||
</argument> | ||
<argument> | ||
<type>Array(Nullable(String))</type> | ||
<name>prop_vals</name> | ||
</argument> | ||
<argument> | ||
<type>Array(Tuple(Nullable(Float64), Nullable(String), Array(Int8)))</type> | ||
<name>value</name> | ||
</argument> | ||
<format>TabSeparated</format> | ||
<command>aggregate_funnel.py</command> | ||
</function> | ||
|
||
<function> | ||
<type>executable</type> | ||
<name>aggregate_funnel_cohort</name> | ||
<return_type>Array(Tuple(Int8, UInt64, Array(Float64)))</return_type> | ||
<argument> | ||
<type>UInt8</type> | ||
<name>num_steps</name> | ||
</argument> | ||
<argument> | ||
<type>UInt64</type> | ||
<name>conversion_window_limit</name> | ||
</argument> | ||
<argument> | ||
<type>String</type> | ||
<name>breakdown_attribution_type</name> | ||
</argument> | ||
<argument> | ||
<type>String</type> | ||
<name>funnel_order_type</name> | ||
</argument> | ||
<argument> | ||
<type>Array(UInt64)</type> | ||
<name>prop_vals</name> | ||
</argument> | ||
<argument> | ||
<type>Array(Tuple(Nullable(Float64), UInt64, Array(Int8)))</type> | ||
<name>value</name> | ||
</argument> | ||
<format>TabSeparated</format> | ||
<command>aggregate_funnel_cohort.py</command> | ||
</function> | ||
|
||
<function> | ||
<type>executable</type> | ||
<name>aggregate_funnel_array</name> | ||
<return_type>Array(Tuple(Int8, Array(String), Array(Float64)))</return_type> | ||
<argument> | ||
<type>UInt8</type> | ||
<name>num_steps</name> | ||
</argument> | ||
<argument> | ||
<type>UInt64</type> | ||
<name>conversion_window_limit</name> | ||
</argument> | ||
<argument> | ||
<type>String</type> | ||
<name>breakdown_attribution_type</name> | ||
</argument> | ||
<argument> | ||
<type>String</type> | ||
<name>funnel_order_type</name> | ||
</argument> | ||
<argument> | ||
<type>Array(Array(String))</type> | ||
<name>prop_vals</name> | ||
</argument> | ||
<argument> | ||
<type>Array(Tuple(Nullable(Float64), Array(String), Array(Int8)))</type> | ||
<name>value</name> | ||
</argument> | ||
<format>TabSeparated</format> | ||
<command>aggregate_funnel_array.py</command> | ||
</function> | ||
|
||
<function> | ||
<type>executable</type> | ||
<name>aggregate_funnel_test</name> | ||
<return_type>String</return_type> | ||
<argument> | ||
<type>UInt8</type> | ||
<name>num_steps</name> | ||
</argument> | ||
<argument> | ||
<type>UInt64</type> | ||
<name>conversion_window_limit</name> | ||
</argument> | ||
<argument> | ||
<type>String</type> | ||
<name>breakdown_attribution_type</name> | ||
</argument> | ||
<argument> | ||
<type>String</type> | ||
<name>funnel_order_type</name> | ||
</argument> | ||
<argument> | ||
<type>Array(Array(String))</type> | ||
<name>prop_vals</name> | ||
</argument> | ||
<argument> | ||
<type>Array(Tuple(Nullable(Float64), Nullable(String), Array(Int8)))</type> | ||
<name>value</name> | ||
</argument> | ||
<format>TabSeparated</format> | ||
<command>test_function.py</command> | ||
</function> | ||
</functions> |
Binary file modified
BIN
+209 Bytes
(100%)
...end/__snapshots__/scenes-app-insights--funnel-top-to-bottom-breakdown--dark.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,6 +33,7 @@ | |
FunnelExclusionActionsNode, | ||
FunnelTimeToConvertResults, | ||
FunnelVizType, | ||
FunnelExclusionEventsNode, | ||
) | ||
from posthog.types import EntityNode, ExclusionEntityNode | ||
|
||
|
@@ -408,6 +409,93 @@ def _get_inner_event_query( | |
|
||
return funnel_events_query | ||
|
||
# This version of the inner event query modifies how exclusions are returned to | ||
# make them behave more like steps. It returns a boolean "exclusion_{0..n}" for each event | ||
def _get_inner_event_query_for_udf( | ||
self, | ||
entities: list[EntityNode] | None = None, | ||
entity_name="events", | ||
skip_entity_filter=False, | ||
skip_step_filter=False, | ||
) -> ast.SelectQuery: | ||
query, funnelsFilter, breakdown, breakdownType, breakdownAttributionType = ( | ||
self.context.query, | ||
self.context.funnelsFilter, | ||
self.context.breakdown, | ||
self.context.breakdownType, | ||
self.context.breakdownAttributionType, | ||
) | ||
entities_to_use = entities or query.series | ||
|
||
extra_fields: list[str] = [] | ||
|
||
for prop in self.context.includeProperties: | ||
extra_fields.append(prop) | ||
|
||
funnel_events_query = FunnelEventQuery( | ||
context=self.context, | ||
extra_fields=[*self._extra_event_fields, *extra_fields], | ||
extra_event_properties=self._extra_event_properties, | ||
).to_query( | ||
skip_entity_filter=skip_entity_filter, | ||
) | ||
# funnel_events_query, params = FunnelEventQuery( | ||
# extra_fields=[*self._extra_event_fields, *extra_fields], | ||
# extra_event_properties=self._extra_event_properties, | ||
# ).get_query(entities_to_use, entity_name, skip_entity_filter=skip_entity_filter) | ||
|
||
all_step_cols: list[ast.Expr] = [] | ||
all_exclusions: list[list[FunnelExclusionEventsNode | FunnelExclusionActionsNode]] = [] | ||
for index, entity in enumerate(entities_to_use): | ||
step_cols = self._get_step_col(entity, index, entity_name) | ||
all_step_cols.extend(step_cols) | ||
all_exclusions.append([]) | ||
|
||
for excluded_entity in funnelsFilter.exclusions or []: | ||
for i in range(excluded_entity.funnelFromStep + 1, excluded_entity.funnelToStep + 1): | ||
all_exclusions[i].append(excluded_entity) | ||
|
||
for index, exclusions in enumerate(all_exclusions): | ||
exclusion_col_expr = self._get_exclusions_col(exclusions, index, entity_name) | ||
all_step_cols.append(exclusion_col_expr) | ||
|
||
breakdown_select_prop = self._get_breakdown_select_prop() | ||
|
||
if breakdown_select_prop: | ||
all_step_cols.extend(breakdown_select_prop) | ||
|
||
funnel_events_query.select = [*funnel_events_query.select, *all_step_cols] | ||
|
||
if breakdown and breakdownType == BreakdownType.COHORT: | ||
if funnel_events_query.select_from is None: | ||
raise ValidationError("Apologies, there was an error adding cohort breakdowns to the query.") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When would this happen? If never, we could do |
||
funnel_events_query.select_from.next_join = self._get_cohort_breakdown_join() | ||
|
||
if not skip_step_filter: | ||
assert isinstance(funnel_events_query.where, ast.Expr) | ||
steps_conditions = self._get_steps_conditions_for_udf(all_exclusions, length=len(entities_to_use)) | ||
funnel_events_query.where = ast.And(exprs=[funnel_events_query.where, steps_conditions]) | ||
|
||
if breakdown and breakdownAttributionType != BreakdownAttributionType.ALL_EVENTS: | ||
# ALL_EVENTS attribution is the old default, which doesn't need the subquery | ||
return self._add_breakdown_attribution_subquery(funnel_events_query) | ||
|
||
return funnel_events_query | ||
|
||
def _get_exclusions_col( | ||
self, | ||
exclusions: list[ExclusionEntityNode], | ||
index: int, | ||
entity_name: str, | ||
) -> ast.Expr: | ||
if not exclusions: | ||
return parse_expr(f"0 as exclusion_{index}") | ||
|
||
conditions = [self._build_step_query(exclusion, index, entity_name, "") for exclusion in exclusions] | ||
return parse_expr( | ||
f"if({{condition}}, 1, 0) as exclusion_{index}", placeholders={"condition": ast.Or(exprs=conditions)} | ||
) | ||
|
||
def _get_cohort_breakdown_join(self) -> ast.JoinExpr: | ||
breakdown = self.context.breakdown | ||
|
||
|
@@ -545,12 +633,23 @@ def _get_steps_conditions(self, length: int) -> ast.Expr: | |
|
||
return ast.Or(exprs=step_conditions) | ||
|
||
def _get_steps_conditions_for_udf(self, exclusions, length: int) -> ast.Expr: | ||
step_conditions: list[ast.Expr] = [] | ||
|
||
for index in range(length): | ||
step_conditions.append(parse_expr(f"step_{index} = 1")) | ||
if exclusions[index]: | ||
step_conditions.append(parse_expr(f"exclusion_{index} = 1")) | ||
|
||
return ast.Or(exprs=step_conditions) | ||
|
||
def _get_step_col( | ||
self, | ||
entity: EntityNode | ExclusionEntityNode, | ||
index: int, | ||
entity_name: str, | ||
step_prefix: str = "", | ||
for_udf: bool = False, | ||
) -> list[ast.Expr]: | ||
# step prefix is used to distinguish actual steps, and exclusion steps | ||
# without the prefix, we get the same parameter binding for both, which borks things up | ||
|
@@ -559,9 +658,10 @@ def _get_step_col( | |
step_cols.append( | ||
parse_expr(f"if({{condition}}, 1, 0) as {step_prefix}step_{index}", placeholders={"condition": condition}) | ||
) | ||
step_cols.append( | ||
parse_expr(f"if({step_prefix}step_{index} = 1, timestamp, null) as {step_prefix}latest_{index}") | ||
) | ||
if not for_udf: | ||
step_cols.append( | ||
parse_expr(f"if({step_prefix}step_{index} = 1, timestamp, null) as {step_prefix}latest_{index}") | ||
) | ||
|
||
for field in self.extra_event_fields_and_properties: | ||
step_cols.append( | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Commented out?