Skip to content
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

chore: Person ID overrides by default #22811

Merged
merged 62 commits into from
Sep 13, 2024
Merged
Show file tree
Hide file tree
Changes from 44 commits
Commits
Show all changes
62 commits
Select commit Hold shift + click to select a range
667ea9c
Fix `team.person_on_events_querying_enabled`
Twixes Jun 6, 2024
2d2361d
Use project persons on events setting for `team.person_on_events_mode`
Twixes Jun 6, 2024
90a0bbe
Remove long gone `person-on-events-enabled` flag
Twixes Jun 6, 2024
f4dc24f
Add test for modifiers
Twixes Jun 7, 2024
192158c
Don't use `PersonsOnEventsMode.disabled` as fallback
Twixes Jun 7, 2024
a554cd5
Update a couple tests
Twixes Jun 13, 2024
47dffe0
Update query snapshots
github-actions[bot] Aug 13, 2024
a8b0080
Update query snapshots
github-actions[bot] Aug 13, 2024
fd10df7
Update query snapshots
github-actions[bot] Aug 13, 2024
aab9107
Merge branch 'master' into poe-rollout-2024
Twixes Aug 21, 2024
ef1c2bd
Fix comment on `PersonsOnEventsMode`
Twixes Aug 21, 2024
8f30c18
Fix query tagging
Twixes Aug 21, 2024
7292178
Try to squash legacy queries trying to use overrides
Twixes Aug 21, 2024
9b93487
Update query snapshots
github-actions[bot] Aug 21, 2024
7ffc229
Update query snapshots
github-actions[bot] Aug 21, 2024
be22045
Update UI snapshots for `chromium` (1)
github-actions[bot] Aug 21, 2024
c92fd96
Update UI snapshots for `chromium` (1)
github-actions[bot] Aug 21, 2024
341e706
Update UI snapshots for `chromium` (1)
github-actions[bot] Aug 21, 2024
3c08d3a
Update UI snapshots for `chromium` (1)
github-actions[bot] Aug 21, 2024
19d5c69
Update UI snapshots for `chromium` (1)
github-actions[bot] Aug 21, 2024
d6b06b7
Update UI snapshots for `chromium` (1)
github-actions[bot] Aug 21, 2024
1e13011
Merge branch 'master' into poe-rollout-2024
Twixes Sep 4, 2024
4a5975e
Update query snapshots
github-actions[bot] Sep 4, 2024
4c266a8
Update query snapshots
github-actions[bot] Sep 4, 2024
8bbd018
Update mypy-baseline.txt
Twixes Sep 4, 2024
26415dc
Updates tests
Twixes Sep 4, 2024
4975e9d
Update query snapshots
github-actions[bot] Sep 4, 2024
9472432
Adjust `translate_hogql()`
Twixes Sep 4, 2024
eeb4d0f
Fix typing and update snapshots
Twixes Sep 5, 2024
953e8fe
Update recordings list tests
Twixes Sep 5, 2024
a08b4bb
Merge branch 'master' into poe-rollout-2024
Twixes Sep 5, 2024
b06731f
Fix typing
Twixes Sep 5, 2024
514dde0
Update query snapshots
github-actions[bot] Sep 5, 2024
87f1145
Update query snapshots
github-actions[bot] Sep 5, 2024
b4faa85
Fix circular import
Twixes Sep 6, 2024
995800b
Update some more tests
Twixes Sep 6, 2024
6b731b5
Update query snapshots
github-actions[bot] Sep 6, 2024
caf41ca
Update query snapshots
github-actions[bot] Sep 6, 2024
7b4f0f2
Fix legacy demo data generation
Twixes Sep 6, 2024
3daf131
Update query snapshots
github-actions[bot] Sep 6, 2024
72c06b2
Make SQL inline assertions work
Twixes Sep 11, 2024
eaac2ab
Merge branch 'master' into poe-rollout-2024
Twixes Sep 11, 2024
b3c5b7b
Update query snapshots
github-actions[bot] Sep 11, 2024
1e73356
Update query snapshots
github-actions[bot] Sep 11, 2024
448f550
Merge branch 'master' into poe-rollout-2024
Twixes Sep 11, 2024
657b8f1
Update test_session_replay_events.py
Twixes Sep 11, 2024
95b481b
Update query snapshots
github-actions[bot] Sep 11, 2024
5ec6535
Update query snapshots
github-actions[bot] Sep 11, 2024
8d1c9bc
Update query snapshots
github-actions[bot] Sep 11, 2024
0fe68ab
Merge branch 'master' into poe-rollout-2024
Twixes Sep 12, 2024
7399d32
Update query snapshots
github-actions[bot] Sep 12, 2024
4bcbf92
Update query snapshots
github-actions[bot] Sep 12, 2024
dd41130
Merge branch 'master' into poe-rollout-2024
Twixes Sep 13, 2024
afe4300
Update query snapshots
github-actions[bot] Sep 13, 2024
f2f94e9
Update query snapshots
github-actions[bot] Sep 13, 2024
6d20265
Update UI snapshots for `chromium` (1)
github-actions[bot] Sep 13, 2024
a6a1842
Update UI snapshots for `chromium` (1)
github-actions[bot] Sep 13, 2024
47069e2
Fix `test_rolling_retention`
Twixes Sep 13, 2024
dfaebe1
Update test_session_recording_list_from_filters.py
Twixes Sep 13, 2024
8125985
Update query snapshots
github-actions[bot] Sep 13, 2024
1577205
Update query snapshots
github-actions[bot] Sep 13, 2024
fbf8bbe
Fix typo
Twixes Sep 13, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ee/clickhouse/queries/test/__snapshots__/test_lifecycle.ambr
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,7 @@
AND event = '$pageview'
AND timestamp >= toDateTime(dateTrunc('day', toDateTime('2021-04-28 00:00:00', 'UTC'))) - INTERVAL 1 day
AND timestamp < toDateTime(dateTrunc('day', toDateTime('2021-05-05 23:59:59', 'UTC'))) + INTERVAL 1 day
AND (ifNull(like(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person_properties, 'email'), ''), 'null'), '^"|"$', ''), '%test.com'), 0))
AND (ifNull(like(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person_props, 'email'), ''), 'null'), '^"|"$', ''), '%test.com'), 0))
GROUP BY pdi.person_id)
GROUP BY start_of_period,
status)
Expand Down Expand Up @@ -577,7 +577,7 @@
AND event = '$pageview'
AND timestamp >= toDateTime(dateTrunc('day', toDateTime('2021-04-28 00:00:00', 'UTC'))) - INTERVAL 1 day
AND timestamp < toDateTime(dateTrunc('day', toDateTime('2021-05-05 23:59:59', 'UTC'))) + INTERVAL 1 day
AND (ifNull(like(nullIf(nullIf(mat_pp_email, ''), 'null'), '%test.com'), 0))
AND (ifNull(like(nullIf(nullIf(pmat_email, ''), 'null'), '%test.com'), 0))
GROUP BY pdi.person_id)
GROUP BY start_of_period,
status)
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
OFFSET %(offset)s
'''
# ---
# name: TestClickhouseSessionRecordingsListFromSessionReplay.test_effect_of_poe_settings_on_query_generated_1_test_poe_being_unavailable_we_fall_back_to_person_subquery
# name: TestClickhouseSessionRecordingsListFromSessionReplay.test_effect_of_poe_settings_on_query_generated_1_test_poe_being_unavailable_we_fall_back_to_person_id_overrides
'''
-- running in PoE Mode: disabled

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,39 +75,34 @@ def create_event(
False,
False,
PersonsOnEventsMode.PERSON_ID_NO_OVERRIDE_PROPERTIES_ON_EVENTS,
True,
],
[
"test_poe_being_unavailable_we_fall_back_to_person_subquery",
"test_poe_being_unavailable_we_fall_back_to_person_id_overrides",
False,
False,
False,
PersonsOnEventsMode.DISABLED,
True,
PersonsOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_JOINED,
],
[
"test_poe_being_unavailable_we_fall_back_to_person_subquery_but_still_use_mat_props",
False,
False,
False,
PersonsOnEventsMode.DISABLED,
False,
PersonsOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_JOINED,
],
[
"test_allow_denormalised_props_fix_does_not_stop_all_poe_processing",
False,
True,
False,
PersonsOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_ON_EVENTS,
False,
],
[
"test_poe_v2_available_person_properties_are_used_in_replay_listing",
False,
True,
True,
PersonsOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_ON_EVENTS,
False,
],
]
)
Expand All @@ -118,7 +113,6 @@ def test_effect_of_poe_settings_on_query_generated(
poe_v2: bool,
allow_denormalized_props: bool,
expected_poe_mode: PersonsOnEventsMode,
unmaterialized_person_column_used: bool,
) -> None:
with self.settings(
PERSON_ON_EVENTS_OVERRIDE=poe_v1,
Expand Down Expand Up @@ -150,29 +144,19 @@ def test_effect_of_poe_settings_on_query_generated(

person_filtering_expr = self._matching_person_filter_expr_from(hogql_parsed_select)

if poe_v1 or poe_v2:
# when poe is off we will join to events, so we can get person properties directly off them
self._assert_is_events_person_filter(person_filtering_expr)
self._assert_is_events_person_filter(person_filtering_expr)

if poe_v1 or poe_v2:
# Property used directly from event (from materialized column)
assert "ifNull(equals(nullIf(nullIf(mat_pp_rgInternal, ''), 'null')" in printed_query
else:
# when poe is off we join to person_distinct_ids, so we can get persons, so we can query their properties
self._assert_is_pdi_filter(person_filtering_expr)

if unmaterialized_person_column_used:
assert (
"argMax(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person.properties, %(hogql_val_6)s), ''), 'null'), '^\"|\"$', ''), person.version) AS properties___rgInternal"
in printed_query
)
else:
# we should use materialized column
# assert (
# "argMax(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person.properties, %(hogql_val_6)s), ''), 'null'), '^\"|\"$', ''), person.version) AS properties___rgInternal"
# not in printed_query
# )
# TODO frustratingly this doesn't pass - but since we're migrating to PoE maybe we can ignore it
pass

# We get the person property value from the persons JOIN
assert (
"argMax(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person.properties, %(hogql_val_6)s), ''), 'null'), '^\"|\"$', ''), person.version) AS properties___rgInternal"
in printed_query
)
# Then we actually filter on that property value
assert "ifNull(equals(events__person.properties___rgInternal, %(hogql_val_14)s), 0)" in printed_query
self.assertQueryMatchesSnapshot(printed_query)

def _assert_is_pdi_filter(self, person_filtering_expr: list[Expr]) -> None:
Expand Down
1 change: 0 additions & 1 deletion frontend/src/lib/constants.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,6 @@ export const FEATURE_FLAGS = {
FUNNELS_CUE_OPT_OUT: 'funnels-cue-opt-out-7301', // owner: @neilkakkar
KAFKA_INSPECTOR: 'kafka-inspector', // owner: @yakkomajuri
HISTORICAL_EXPORTS_V2: 'historical-exports-v2', // owner @macobo
PERSON_ON_EVENTS_ENABLED: 'person-on-events-enabled', //owner: @EDsCODE
INGESTION_WARNINGS_ENABLED: 'ingestion-warnings-enabled', // owner: @tiina303
SESSION_RESET_ON_LOAD: 'session-reset-on-load', // owner: @benjackwhite
DEBUG_REACT_RENDERS: 'debug-react-renders', // owner: @benjackwhite
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/queries/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ export interface DataNode<R extends Record<string, any> = Record<string, any>> e
/** HogQL Query Options are automatically set per team. However, they can be overriden in the query. */
export interface HogQLQueryModifiers {
personsOnEventsMode?:
| 'disabled'
| 'disabled' // `disabled` is deprecated and set for removal - `person_id_override_properties_joined` is its faster functional equivalent
| 'person_id_no_override_properties_on_events'
| 'person_id_override_properties_on_events'
| 'person_id_override_properties_joined'
Expand Down
2 changes: 0 additions & 2 deletions mypy-baseline.txt
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@ posthog/hogql/database/schema/groups.py:0: note: "Dict" is invariant -- see http
posthog/hogql/database/schema/groups.py:0: note: Consider using "Mapping" instead, which is covariant in the value type
posthog/hogql/database/schema/persons.py:0: error: Incompatible types in assignment (expression has type "Organization | None", variable has type "Organization") [assignment]
posthog/batch_exports/service.py:0: error: Argument 4 to "backfill_export" has incompatible type "datetime | None"; expected "datetime" [arg-type]
posthog/models/filters/base_filter.py:0: error: "HogQLContext" has no attribute "person_on_events_mode" [attr-defined]
posthog/models/team/team.py:0: error: Statement is unreachable [unreachable]
posthog/models/team/team.py:0: error: Statement is unreachable [unreachable]
posthog/models/hog_functions/hog_function.py:0: error: Argument 1 to "get" of "dict" has incompatible type "str | None"; expected "str" [arg-type]
Expand Down Expand Up @@ -357,7 +356,6 @@ posthog/queries/actor_base_query.py:0: error: Incompatible types (expression has
posthog/queries/actor_base_query.py:0: error: Incompatible types (expression has type "datetime", TypedDict item "created_at" has type "str | None") [typeddict-item]
posthog/hogql_queries/insights/funnels/base.py:0: error: Incompatible type for lookup 'pk': (got "str | int | None", expected "str | int") [misc]
posthog/queries/foss_cohort_query.py:0: error: Incompatible type for lookup 'pk': (got "str | int | list[str]", expected "str | int") [misc]
posthog/queries/funnels/base.py:0: error: "HogQLContext" has no attribute "person_on_events_mode" [attr-defined]
posthog/queries/funnels/base.py:0: error: Incompatible types in assignment (expression has type "int | str | None", variable has type "str | None") [assignment]
posthog/queries/funnels/base.py:0: error: Unsupported right operand type for in ("object") [operator]
posthog/queries/funnels/base.py:0: error: "object" has no attribute "append" [attr-defined]
Expand Down
18 changes: 9 additions & 9 deletions posthog/api/test/__snapshots__/test_insight.ambr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
# name: TestInsight.test_insight_funnels_hogql_breakdown
'''
/* user_id:0 request:_snapshot_ */
SELECT array(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person_properties, 'fish'), ''), 'null'), '^"|"$', '')) AS value,
SELECT array(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person_props, 'fish'), ''), 'null'), '^"|"$', '')) AS value,
count(*) as count
FROM events e
INNER JOIN
Expand Down Expand Up @@ -78,7 +78,7 @@
if(step_0 = 1, timestamp, null) as latest_0,
if(event = 'user did things', 1, 0) as step_1,
if(step_1 = 1, timestamp, null) as latest_1,
array(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person_properties, 'fish'), ''), 'null'), '^"|"$', '')) AS prop_basic,
array(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person_props, 'fish'), ''), 'null'), '^"|"$', '')) AS prop_basic,
prop_basic as prop,
argMinIf(prop, timestamp, notEmpty(arrayFilter(x -> notEmpty(x), prop))) over (PARTITION by aggregation_target) as prop_vals
FROM events e
Expand Down Expand Up @@ -183,7 +183,7 @@
AND toTimeZone(timestamp, 'UTC') >= toDateTime('2012-01-08 00:00:00', 'UTC')
AND toTimeZone(timestamp, 'UTC') <= toDateTime('2012-01-15 23:59:59', 'UTC')
AND ((and(ifNull(less(accurateCastOrNull(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(properties, 'int_value'), ''), 'null'), '^"|"$', ''), 'Int64'), 10), 0), 1))
AND (ifNull(like(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person_properties, 'fish'), ''), 'null'), '^"|"$', ''), '%fish%'), 0)))
AND (ifNull(like(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person_props, 'fish'), ''), 'null'), '^"|"$', ''), '%fish%'), 0)))
AND (step_0 = 1
OR step_1 = 1) ))
WHERE step_0 = 1 ))
Expand Down Expand Up @@ -228,11 +228,11 @@
person.person_props as person_props,
if(event = 'user signed up'
AND (and(ifNull(less(accurateCastOrNull(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(properties, 'int_value'), ''), 'null'), '^"|"$', ''), 'Int64'), 10), 0), 1)
AND ifNull(like(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person_properties, 'fish'), ''), 'null'), '^"|"$', ''), '%fish%'), 0)), 1, 0) as step_0,
AND ifNull(like(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person_props, 'fish'), ''), 'null'), '^"|"$', ''), '%fish%'), 0)), 1, 0) as step_0,
if(step_0 = 1, timestamp, null) as latest_0,
if(event = 'user did things'
AND (and(ifNull(less(accurateCastOrNull(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(properties, 'int_value'), ''), 'null'), '^"|"$', ''), 'Int64'), 10), 0), 1)
AND ifNull(like(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person_properties, 'fish'), ''), 'null'), '^"|"$', ''), '%fish%'), 0)), 1, 0) as step_1,
AND ifNull(like(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person_props, 'fish'), ''), 'null'), '^"|"$', ''), '%fish%'), 0)), 1, 0) as step_1,
if(step_1 = 1, timestamp, null) as latest_1
FROM events e
INNER JOIN
Expand Down Expand Up @@ -459,7 +459,7 @@
AND toTimeZone(timestamp, 'UTC') >= toDateTime(toStartOfDay(toDateTime('2012-01-08 00:00:00', 'UTC')), 'UTC')
AND toTimeZone(timestamp, 'UTC') <= toDateTime('2012-01-15 23:59:59', 'UTC')
AND ((and(ifNull(greater(accurateCastOrNull(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(properties, 'int_value'), ''), 'null'), '^"|"$', ''), 'Int64'), 10), 0), 1))
AND (ifNull(like(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person_properties, 'fish'), ''), 'null'), '^"|"$', ''), '%fish%'), 0)))
AND (ifNull(like(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person_props, 'fish'), ''), 'null'), '^"|"$', ''), '%fish%'), 0)))
GROUP BY date)
GROUP BY day_start
ORDER BY day_start)
Expand Down Expand Up @@ -534,7 +534,7 @@
AND toTimeZone(timestamp, 'UTC') >= toDateTime(toStartOfDay(toDateTime('2012-01-08 00:00:00', 'UTC')), 'UTC')
AND toTimeZone(timestamp, 'UTC') <= toDateTime('2012-01-15 23:59:59', 'UTC')
AND ((and(ifNull(greater(accurateCastOrNull(nullIf(nullIf(events.mat_int_value, ''), 'null'), 'Int64'), 10), 0), 1))
AND (ifNull(like(nullIf(nullIf(mat_pp_fish, ''), 'null'), '%fish%'), 0)))
AND (ifNull(like(nullIf(nullIf(pmat_fish, ''), 'null'), '%fish%'), 0)))
GROUP BY date)
GROUP BY day_start
ORDER BY day_start)
Expand Down Expand Up @@ -583,7 +583,7 @@
AND toTimeZone(timestamp, 'UTC') >= toDateTime(toStartOfDay(toDateTime('2012-01-08 00:00:00', 'UTC')), 'UTC')
AND toTimeZone(timestamp, 'UTC') <= toDateTime('2012-01-15 23:59:59', 'UTC')
AND (and(ifNull(less(accurateCastOrNull(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(properties, 'int_value'), ''), 'null'), '^"|"$', ''), 'Int64'), 10), 0), 1)
AND ifNull(like(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person_properties, 'fish'), ''), 'null'), '^"|"$', ''), '%fish%'), 0))
AND ifNull(like(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person_props, 'fish'), ''), 'null'), '^"|"$', ''), '%fish%'), 0))
GROUP BY date)
GROUP BY day_start
ORDER BY day_start)
Expand Down Expand Up @@ -632,7 +632,7 @@
AND toTimeZone(timestamp, 'UTC') >= toDateTime(toStartOfDay(toDateTime('2012-01-08 00:00:00', 'UTC')), 'UTC')
AND toTimeZone(timestamp, 'UTC') <= toDateTime('2012-01-15 23:59:59', 'UTC')
AND (and(ifNull(less(accurateCastOrNull(nullIf(nullIf(events.mat_int_value, ''), 'null'), 'Int64'), 10), 0), 1)
AND ifNull(like(nullIf(nullIf(mat_pp_fish, ''), 'null'), '%fish%'), 0))
AND ifNull(like(nullIf(nullIf(pmat_fish, ''), 'null'), '%fish%'), 0))
GROUP BY date)
GROUP BY day_start
ORDER BY day_start)
Expand Down
Loading
Loading