diff --git a/ee/clickhouse/queries/funnels/funnel_correlation.py b/ee/clickhouse/queries/funnels/funnel_correlation.py index 4dc2b50a39490..ebe78d3b86826 100644 --- a/ee/clickhouse/queries/funnels/funnel_correlation.py +++ b/ee/clickhouse/queries/funnels/funnel_correlation.py @@ -435,7 +435,7 @@ def get_properties_query(self) -> Tuple[str, Dict[str, Any]]: return query, params def _get_aggregation_target_join_query(self) -> str: - if self._team.person_on_events_mode == PersonOnEventsMode.V1_ENABLED: + if self._team.person_on_events_mode == PersonOnEventsMode.PERSON_ID_NO_OVERRIDE_PROPERTIES_ON_EVENTS: aggregation_person_join = f""" JOIN funnel_actors as actors ON event.person_id = actors.actor_id diff --git a/ee/session_recordings/queries/test/test_session_recording_list_from_session_replay.py b/ee/session_recordings/queries/test/test_session_recording_list_from_session_replay.py index a6fe85ebf9702..33fdd6c3e0236 100644 --- a/ee/session_recordings/queries/test/test_session_recording_list_from_session_replay.py +++ b/ee/session_recordings/queries/test/test_session_recording_list_from_session_replay.py @@ -63,7 +63,7 @@ def create_event( True, False, False, - PersonOnEventsMode.V1_ENABLED, + PersonOnEventsMode.PERSON_ID_NO_OVERRIDE_PROPERTIES_ON_EVENTS, { "kperson_filter_pre__0": "rgInternal", "kpersonquery_person_filter_fin__0": "rgInternal", @@ -95,7 +95,7 @@ def create_event( False, True, False, - PersonOnEventsMode.V2_ENABLED, + PersonOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_ON_EVENTS, { "event_names": [], "event_start_time": mock.ANY, @@ -111,7 +111,7 @@ def create_event( False, True, True, - PersonOnEventsMode.V2_ENABLED, + PersonOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_ON_EVENTS, { "event_end_time": mock.ANY, "event_names": [], diff --git a/frontend/__snapshots__/scenes-other-settings--settings-project--dark.png b/frontend/__snapshots__/scenes-other-settings--settings-project--dark.png index cb28d08f5b090..3c3f37e66ec31 100644 Binary files a/frontend/__snapshots__/scenes-other-settings--settings-project--dark.png and b/frontend/__snapshots__/scenes-other-settings--settings-project--dark.png differ diff --git a/frontend/__snapshots__/scenes-other-settings--settings-project--light.png b/frontend/__snapshots__/scenes-other-settings--settings-project--light.png index 31dbd5b34cfcf..015b360886f5b 100644 Binary files a/frontend/__snapshots__/scenes-other-settings--settings-project--light.png and b/frontend/__snapshots__/scenes-other-settings--settings-project--light.png differ diff --git a/frontend/src/queries/schema.json b/frontend/src/queries/schema.json index 21f02f13100f4..d651b09005288 100644 --- a/frontend/src/queries/schema.json +++ b/frontend/src/queries/schema.json @@ -2620,7 +2620,14 @@ "type": "string" }, "personsOnEventsMode": { - "enum": ["disabled", "v1_enabled", "v1_mixed", "v2_enabled", "v3_enabled"], + "enum": [ + "disabled", + "v1_enabled", + "v1_mixed", + "v2_enabled", + "v3_enabled", + "person_id_override_properties_joined" + ], "type": "string" } }, diff --git a/frontend/src/queries/schema.ts b/frontend/src/queries/schema.ts index 831c39241ed83..7a5110c3516dc 100644 --- a/frontend/src/queries/schema.ts +++ b/frontend/src/queries/schema.ts @@ -179,7 +179,13 @@ export interface DataNode extends Node { /** HogQL Query Options are automatically set per team. However, they can be overriden in the query. */ export interface HogQLQueryModifiers { - personsOnEventsMode?: 'disabled' | 'v1_enabled' | 'v1_mixed' | 'v2_enabled' | 'v3_enabled' + personsOnEventsMode?: + | 'disabled' + | 'v1_enabled' + | 'v1_mixed' + | 'v2_enabled' + | 'v3_enabled' + | 'person_id_override_properties_joined' personsArgMaxVersion?: 'auto' | 'v1' | 'v2' inCohortVia?: 'auto' | 'leftjoin' | 'subquery' | 'leftjoin_conjoined' materializationMode?: 'auto' | 'legacy_null_as_string' | 'legacy_null_as_null' | 'disabled' diff --git a/frontend/src/scenes/settings/SettingsMap.tsx b/frontend/src/scenes/settings/SettingsMap.tsx index 3530ad15fe407..571f10320ccb6 100644 --- a/frontend/src/scenes/settings/SettingsMap.tsx +++ b/frontend/src/scenes/settings/SettingsMap.tsx @@ -31,7 +31,6 @@ import { ReplayCostControl, ReplayGeneral, } from './project/SessionRecordingSettings' -import { SettingPersonsOnEvents } from './project/SettingPersonsOnEvents' import { SlackIntegration } from './project/SlackIntegration' import { SurveySettings } from './project/SurveySettings' import { ProjectAccountFiltersSetting } from './project/TestAccountFiltersConfig' @@ -139,11 +138,6 @@ export const SettingsMap: SettingSection[] = [ title: 'Group analytics', component: , }, - { - id: 'persons-on-events', - title: 'Persons on events (beta)', - component: , - }, ], }, diff --git a/frontend/src/scenes/settings/project/SettingPersonsOnEvents.tsx b/frontend/src/scenes/settings/project/SettingPersonsOnEvents.tsx deleted file mode 100644 index 8f17a1b88c6f8..0000000000000 --- a/frontend/src/scenes/settings/project/SettingPersonsOnEvents.tsx +++ /dev/null @@ -1,38 +0,0 @@ -import { LemonSwitch, Link } from '@posthog/lemon-ui' -import { useActions, useValues } from 'kea' -import { teamLogic } from 'scenes/teamLogic' - -export function SettingPersonsOnEvents(): JSX.Element { - const { updateCurrentTeam } = useActions(teamLogic) - const { currentTeam } = useValues(teamLogic) - - return ( - <> -

- We have updated our data model to store person properties directly on events, making queries - significantly faster. This means that person properties will no longer be "timeless", but rather - point-in-time i.e. on filters we'll consider a person's properties at the time of the event, rather than - at present time. This may cause data to change on some of your insights, but will be the default way we - handle person properties going forward. For now, you can control whether you want this on or not, and - should feel free to let us know of any concerns you might have. If you do enable this, you should see - speed improvements of around 3-5x on average on most of your insights. -

-

- Please note, you might need to change the way you send us events after enabling this - feature. Read more here. -

- - { - updateCurrentTeam({ - extra_settings: { ...currentTeam?.extra_settings, ['poe_v2_enabled']: checked }, - }) - }} - label="Enable Person on Events (Beta)" - checked={!!currentTeam?.extra_settings?.['poe_v2_enabled']} - bordered - /> - - ) -} diff --git a/posthog/api/test/__snapshots__/test_insight.ambr b/posthog/api/test/__snapshots__/test_insight.ambr index 6495d532782b2..5bdf7b792790b 100644 --- a/posthog/api/test/__snapshots__/test_insight.ambr +++ b/posthog/api/test/__snapshots__/test_insight.ambr @@ -1616,24 +1616,6 @@ LIMIT 21 /*controller='project_insights-list',route='api/projects/%28%3FP%3Cparent_lookup_team_id%3E%5B%5E/.%5D%2B%29/insights/%3F%24'*/ ''' # --- -# name: TestInsight.test_listing_insights_does_not_nplus1.30 - ''' - SELECT "posthog_taggeditem"."id", - "posthog_taggeditem"."tag_id", - "posthog_taggeditem"."dashboard_id", - "posthog_taggeditem"."insight_id", - "posthog_taggeditem"."event_definition_id", - "posthog_taggeditem"."property_definition_id", - "posthog_taggeditem"."action_id", - "posthog_taggeditem"."feature_flag_id" - FROM "posthog_taggeditem" - WHERE "posthog_taggeditem"."insight_id" IN (1, - 2, - 3, - 4, - 5 /* ... */) /*controller='project_insights-list',route='api/projects/%28%3FP%3Cparent_lookup_team_id%3E%5B%5E/.%5D%2B%29/insights/%3F%24'*/ - ''' -# --- # name: TestInsight.test_listing_insights_does_not_nplus1.4 ''' SELECT "posthog_team"."id", diff --git a/posthog/api/test/test_team.py b/posthog/api/test/test_team.py index 11bc5c664e1dd..d23efe81cf7d8 100644 --- a/posthog/api/test/test_team.py +++ b/posthog/api/test/test_team.py @@ -204,47 +204,6 @@ def test_update_project_timezone(self): ] ) - @freeze_time("2022-02-08") - def test_activity_log_tracks_extra_settings(self): - self._assert_activity_log_is_empty() - - response = self.client.patch("/api/projects/@current/", {"extra_settings": {"poe_v2_enabled": True}}) - self.assertEqual(response.status_code, status.HTTP_200_OK) - - response_data = response.json() - self.assertEqual(response_data["name"], self.team.name) - self.assertEqual(response_data["extra_settings"], {"poe_v2_enabled": True}) - - self._assert_activity_log( - [ - { - "activity": "updated", - "created_at": "2022-02-08T00:00:00Z", - "detail": { - "changes": [ - { - "action": "created", - "after": {"poe_v2_enabled": True}, - "before": None, - "field": "extra_settings", - "type": "Team", - }, - ], - "name": "Default project", - "short_id": None, - "trigger": None, - "type": None, - }, - "item_id": str(self.team.pk), - "scope": "Team", - "user": { - "email": "user1@posthog.com", - "first_name": "", - }, - }, - ] - ) - def test_update_test_filter_default_checked(self): response = self.client.patch("/api/projects/@current/", {"test_account_filters_default_checked": "true"}) self.assertEqual(response.status_code, status.HTTP_200_OK) diff --git a/posthog/hogql/modifiers.py b/posthog/hogql/modifiers.py index 8452016dc1411..3c6fa11a9fc3b 100644 --- a/posthog/hogql/modifiers.py +++ b/posthog/hogql/modifiers.py @@ -1,12 +1,7 @@ from typing import Optional, TYPE_CHECKING -from posthog.schema import ( - HogQLQueryModifiers, - InCohortVia, - MaterializationMode, - PersonsArgMaxVersion, - PersonsOnEventsMode, -) +from posthog.schema import HogQLQueryModifiers, InCohortVia, MaterializationMode, PersonsArgMaxVersion +from posthog.utils import PersonOnEventsMode if TYPE_CHECKING: from posthog.models import Team @@ -21,10 +16,7 @@ def create_default_modifiers_for_team( modifiers = modifiers.model_copy() if modifiers.personsOnEventsMode is None: - if team.person_on_events_v3_querying_enabled: - modifiers.personsOnEventsMode = PersonsOnEventsMode.v3_enabled - else: - modifiers.personsOnEventsMode = team.person_on_events_mode + modifiers.personsOnEventsMode = team.person_on_events_mode or PersonOnEventsMode.DISABLED if modifiers.personsArgMaxVersion is None: modifiers.personsArgMaxVersion = PersonsArgMaxVersion.auto diff --git a/posthog/hogql/test/test_modifiers.py b/posthog/hogql/test/test_modifiers.py index a33d57575e2f4..b2b0ef1e40630 100644 --- a/posthog/hogql/test/test_modifiers.py +++ b/posthog/hogql/test/test_modifiers.py @@ -7,11 +7,7 @@ class TestModifiers(BaseTest): - @override_settings( - PERSON_ON_EVENTS_OVERRIDE=False, - PERSON_ON_EVENTS_V2_OVERRIDE=False, - PERSON_ON_EVENTS_V3_OVERRIDE=False, - ) + @override_settings(PERSON_ON_EVENTS_OVERRIDE=False, PERSON_ON_EVENTS_V2_OVERRIDE=False) def test_create_default_modifiers_for_team_init(self): assert self.team.person_on_events_mode == "disabled" modifiers = create_default_modifiers_for_team(self.team) @@ -27,9 +23,6 @@ def test_create_default_modifiers_for_team_init(self): ) assert modifiers.personsOnEventsMode == PersonsOnEventsMode.v2_enabled - with override_settings(PERSON_ON_EVENTS_V3_OVERRIDE=True): - assert create_default_modifiers_for_team(self.team).personsOnEventsMode == PersonsOnEventsMode.v3_enabled - def test_modifiers_persons_on_events_mode_v1_enabled(self): query = "SELECT event, person_id FROM events" diff --git a/posthog/hogql/test/test_printer.py b/posthog/hogql/test/test_printer.py index ab0c839a0b848..6630ed058340f 100644 --- a/posthog/hogql/test/test_printer.py +++ b/posthog/hogql/test/test_printer.py @@ -156,7 +156,9 @@ def test_fields_and_properties(self): context = HogQLContext( team_id=self.team.pk, within_non_hogql_query=True, - modifiers=HogQLQueryModifiers(personsOnEventsMode=PersonOnEventsMode.V1_ENABLED), + modifiers=HogQLQueryModifiers( + personsOnEventsMode=PersonOnEventsMode.PERSON_ID_NO_OVERRIDE_PROPERTIES_ON_EVENTS + ), ) self.assertEqual( self._expr("person.properties.bla", context), diff --git a/posthog/models/team/team.py b/posthog/models/team/team.py index fd317a9dbb617..bb219fd11f708 100644 --- a/posthog/models/team/team.py +++ b/posthog/models/team/team.py @@ -57,7 +57,7 @@ # keep in sync with posthog/frontend/src/scenes/project/Settings/ExtraTeamSettings.tsx class AvailableExtraSettings: - poe_v2_enabled = "poe_v2_enabled" + pass class TeamManager(models.Manager): @@ -290,18 +290,24 @@ def aggregate_users_by_distinct_id(self) -> bool: @property def person_on_events_mode(self) -> PersonOnEventsMode: - # Persons on Events V2 always takes priority over Persons on Events V1 - if self._person_on_events_v2_querying_enabled: - tag_queries(person_on_events_mode=PersonOnEventsMode.V2_ENABLED) - return PersonOnEventsMode.V2_ENABLED + if self._person_on_events_person_id_override_properties_on_events: + tag_queries(person_on_events_mode=PersonOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_ON_EVENTS) + return PersonOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_ON_EVENTS - if self._person_on_events_querying_enabled: + if self._person_on_events_person_id_no_override_properties_on_events: # also tag person_on_events_enabled for legacy compatibility tag_queries( person_on_events_enabled=True, - person_on_events_mode=PersonOnEventsMode.V1_ENABLED, + person_on_events_mode=PersonOnEventsMode.PERSON_ID_NO_OVERRIDE_PROPERTIES_ON_EVENTS, ) - return PersonOnEventsMode.V1_ENABLED + return PersonOnEventsMode.PERSON_ID_NO_OVERRIDE_PROPERTIES_ON_EVENTS + + if self._person_on_events_person_id_override_properties_joined: + tag_queries( + person_on_events_enabled=True, + person_on_events_mode=PersonOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_JOINED, + ) + return PersonOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_JOINED return PersonOnEventsMode.DISABLED @@ -312,25 +318,17 @@ def person_on_events_querying_enabled(self) -> bool: return self.person_on_events_mode != PersonOnEventsMode.DISABLED @property - def _person_on_events_querying_enabled(self) -> bool: + def _person_on_events_person_id_no_override_properties_on_events(self) -> bool: if settings.PERSON_ON_EVENTS_OVERRIDE is not None: return settings.PERSON_ON_EVENTS_OVERRIDE # on PostHog Cloud, use the feature flag if is_cloud(): - # users can override our feature flag via extra_settings - if self.extra_settings and AvailableExtraSettings.poe_v2_enabled in self.extra_settings: - return self.extra_settings["poe_v2_enabled"] return posthoganalytics.feature_enabled( - "person-on-events-enabled", + "persons-on-events-person-id-no-override-properties-on-events", str(self.uuid), - groups={"organization": str(self.organization_id)}, - group_properties={ - "organization": { - "id": str(self.organization_id), - "created_at": self.organization.created_at, - } - }, + groups={"team": str(self.id)}, + group_properties={"team": {"id": str(self.id), "created_at": self.created_at, "uuid": self.uuid}}, only_evaluate_locally=True, send_feature_flag_events=False, ) @@ -339,7 +337,7 @@ def _person_on_events_querying_enabled(self) -> bool: return get_instance_setting("PERSON_ON_EVENTS_ENABLED") @property - def _person_on_events_v2_querying_enabled(self) -> bool: + def _person_on_events_person_id_override_properties_on_events(self) -> bool: if settings.PERSON_ON_EVENTS_V2_OVERRIDE is not None: return settings.PERSON_ON_EVENTS_V2_OVERRIDE @@ -362,23 +360,24 @@ def _person_on_events_v2_querying_enabled(self) -> bool: return get_instance_setting("PERSON_ON_EVENTS_V2_ENABLED") @property - def person_on_events_v3_querying_enabled(self) -> bool: - if settings.PERSON_ON_EVENTS_V3_OVERRIDE is not None: - return settings.PERSON_ON_EVENTS_V3_OVERRIDE - - return posthoganalytics.feature_enabled( - "persons-on-events-v3-reads-enabled", - str(self.uuid), - groups={"organization": str(self.organization_id)}, - group_properties={ - "organization": { - "id": str(self.organization_id), - "created_at": self.organization.created_at, - } - }, - only_evaluate_locally=True, - send_feature_flag_events=False, - ) + def _person_on_events_person_id_override_properties_joined(self) -> bool: + # on PostHog Cloud, use the feature flag + if is_cloud(): + return posthoganalytics.feature_enabled( + "persons-on-events-person-id-override-properties-joined", + str(self.uuid), + groups={"organization": str(self.organization_id)}, + group_properties={ + "organization": { + "id": str(self.organization_id), + "created_at": self.organization.created_at, + } + }, + only_evaluate_locally=True, + send_feature_flag_events=False, + ) + + return False @property def strict_caching_enabled(self) -> bool: diff --git a/posthog/queries/event_query/event_query.py b/posthog/queries/event_query/event_query.py index 862e6d8f03495..aa292e7ee44e4 100644 --- a/posthog/queries/event_query/event_query.py +++ b/posthog/queries/event_query/event_query.py @@ -120,9 +120,9 @@ def _determine_should_join_distinct_ids(self) -> None: pass def _get_person_id_alias(self, person_on_events_mode) -> str: - if person_on_events_mode == PersonOnEventsMode.V2_ENABLED: + if person_on_events_mode == PersonOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_ON_EVENTS: return f"if(notEmpty({self.PERSON_ID_OVERRIDES_TABLE_ALIAS}.person_id), {self.PERSON_ID_OVERRIDES_TABLE_ALIAS}.person_id, {self.EVENT_TABLE_ALIAS}.person_id)" - elif person_on_events_mode == PersonOnEventsMode.V1_ENABLED: + elif person_on_events_mode == PersonOnEventsMode.PERSON_ID_NO_OVERRIDE_PROPERTIES_ON_EVENTS: return f"{self.EVENT_TABLE_ALIAS}.person_id" return f"{self.DISTINCT_ID_TABLE_ALIAS}.person_id" @@ -131,7 +131,7 @@ def _get_person_ids_query(self, *, relevant_events_conditions: str = "") -> str: if not self._should_join_distinct_ids: return "" - if self._person_on_events_mode == PersonOnEventsMode.V2_ENABLED: + if self._person_on_events_mode == PersonOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_ON_EVENTS: return PERSON_OVERRIDES_JOIN_SQL.format( person_overrides_table_alias=self.PERSON_ID_OVERRIDES_TABLE_ALIAS, event_table_alias=self.EVENT_TABLE_ALIAS, diff --git a/posthog/queries/foss_cohort_query.py b/posthog/queries/foss_cohort_query.py index e6005abab632a..19d6e8282ee93 100644 --- a/posthog/queries/foss_cohort_query.py +++ b/posthog/queries/foss_cohort_query.py @@ -297,7 +297,7 @@ def _build_sources(self, subq: List[Tuple[str, str]]) -> Tuple[str, str]: fields = f"{subq_alias}.person_id" elif prev_alias: # can't join without a previous alias if subq_alias == self.PERSON_TABLE_ALIAS and self.should_pushdown_persons: - if self._person_on_events_mode == PersonOnEventsMode.V1_ENABLED: + if self._person_on_events_mode == PersonOnEventsMode.PERSON_ID_NO_OVERRIDE_PROPERTIES_ON_EVENTS: # when using person-on-events, instead of inner join, we filter inside # the event query itself continue @@ -505,7 +505,9 @@ def get_performed_event_multiple(self, prop: Property, prepend: str, idx: int) - ) def _determine_should_join_distinct_ids(self) -> None: - self._should_join_distinct_ids = self._person_on_events_mode != PersonOnEventsMode.V1_ENABLED + self._should_join_distinct_ids = ( + self._person_on_events_mode != PersonOnEventsMode.PERSON_ID_NO_OVERRIDE_PROPERTIES_ON_EVENTS + ) def _determine_should_join_persons(self) -> None: # :TRICKY: This doesn't apply to joining inside events query, but to the diff --git a/posthog/queries/funnels/funnel_event_query.py b/posthog/queries/funnels/funnel_event_query.py index 01c2f6af00497..d2393f77b5d21 100644 --- a/posthog/queries/funnels/funnel_event_query.py +++ b/posthog/queries/funnels/funnel_event_query.py @@ -131,9 +131,9 @@ def _determine_should_join_distinct_ids(self) -> None: ) is_using_cohort_propertes = self._column_optimizer.is_using_cohort_propertes - if self._person_on_events_mode == PersonOnEventsMode.V2_ENABLED: + if self._person_on_events_mode == PersonOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_ON_EVENTS: self._should_join_distinct_ids = True - elif self._person_on_events_mode == PersonOnEventsMode.V1_ENABLED or ( + elif self._person_on_events_mode == PersonOnEventsMode.PERSON_ID_NO_OVERRIDE_PROPERTIES_ON_EVENTS or ( non_person_id_aggregation and not is_using_cohort_propertes ): self._should_join_distinct_ids = False diff --git a/posthog/queries/paths/paths_event_query.py b/posthog/queries/paths/paths_event_query.py index 134b61f5be66a..5c222558973a2 100644 --- a/posthog/queries/paths/paths_event_query.py +++ b/posthog/queries/paths/paths_event_query.py @@ -141,7 +141,7 @@ def get_query(self) -> Tuple[str, Dict[str, Any]]: return query, self.params def _determine_should_join_distinct_ids(self) -> None: - if self._person_on_events_mode == PersonOnEventsMode.V1_ENABLED: + if self._person_on_events_mode == PersonOnEventsMode.PERSON_ID_NO_OVERRIDE_PROPERTIES_ON_EVENTS: self._should_join_distinct_ids = False else: self._should_join_distinct_ids = True diff --git a/posthog/queries/retention/retention_events_query.py b/posthog/queries/retention/retention_events_query.py index 46ac34df48f61..871d9dbb3e033 100644 --- a/posthog/queries/retention/retention_events_query.py +++ b/posthog/queries/retention/retention_events_query.py @@ -197,7 +197,7 @@ def _determine_should_join_distinct_ids(self) -> None: self._filter.aggregation_group_type_index is not None or self._aggregate_users_by_distinct_id ) is_using_cohort_propertes = self._column_optimizer.is_using_cohort_propertes - if self._person_on_events_mode == PersonOnEventsMode.V1_ENABLED or ( + if self._person_on_events_mode == PersonOnEventsMode.PERSON_ID_NO_OVERRIDE_PROPERTIES_ON_EVENTS or ( non_person_id_aggregation and not is_using_cohort_propertes ): self._should_join_distinct_ids = False diff --git a/posthog/queries/stickiness/stickiness_event_query.py b/posthog/queries/stickiness/stickiness_event_query.py index 755f04a0bc335..ce76b2d17ce5c 100644 --- a/posthog/queries/stickiness/stickiness_event_query.py +++ b/posthog/queries/stickiness/stickiness_event_query.py @@ -82,7 +82,7 @@ def _person_query(self): ) def _determine_should_join_distinct_ids(self) -> None: - if self._person_on_events_mode == PersonOnEventsMode.V1_ENABLED: + if self._person_on_events_mode == PersonOnEventsMode.PERSON_ID_NO_OVERRIDE_PROPERTIES_ON_EVENTS: self._should_join_distinct_ids = False else: self._should_join_distinct_ids = True diff --git a/posthog/queries/trends/breakdown.py b/posthog/queries/trends/breakdown.py index 84b304ff99b28..de8665a549610 100644 --- a/posthog/queries/trends/breakdown.py +++ b/posthog/queries/trends/breakdown.py @@ -111,9 +111,9 @@ def __init__( self.column_optimizer = column_optimizer or ColumnOptimizer(self.filter, self.team_id) self.add_person_urls = add_person_urls self.person_on_events_mode = person_on_events_mode - if person_on_events_mode == PersonOnEventsMode.V2_ENABLED: + if person_on_events_mode == PersonOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_ON_EVENTS: self._person_id_alias = f"if(notEmpty({self.PERSON_ID_OVERRIDES_TABLE_ALIAS}.person_id), {self.PERSON_ID_OVERRIDES_TABLE_ALIAS}.person_id, {self.EVENT_TABLE_ALIAS}.person_id)" - elif person_on_events_mode == PersonOnEventsMode.V1_ENABLED: + elif person_on_events_mode == PersonOnEventsMode.PERSON_ID_NO_OVERRIDE_PROPERTIES_ON_EVENTS: self._person_id_alias = f"{self.EVENT_TABLE_ALIAS}.person_id" else: self._person_id_alias = f"{self.DISTINCT_ID_TABLE_ALIAS}.person_id" @@ -163,7 +163,7 @@ def get_query(self) -> Tuple[str, Dict, Callable]: filter=self.filter, event_table_alias=self.EVENT_TABLE_ALIAS, person_id_alias=f"person_id" - if self.person_on_events_mode == PersonOnEventsMode.V1_ENABLED + if self.person_on_events_mode == PersonOnEventsMode.PERSON_ID_NO_OVERRIDE_PROPERTIES_ON_EVENTS else self._person_id_alias, ) @@ -748,10 +748,10 @@ def _determine_breakdown_label( return str(value) or BREAKDOWN_NULL_DISPLAY def _person_join_condition(self) -> Tuple[str, Dict]: - if self.person_on_events_mode == PersonOnEventsMode.V1_ENABLED: + if self.person_on_events_mode == PersonOnEventsMode.PERSON_ID_NO_OVERRIDE_PROPERTIES_ON_EVENTS: return "", {} - if self.person_on_events_mode == PersonOnEventsMode.V2_ENABLED: + if self.person_on_events_mode == PersonOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_ON_EVENTS: return ( PERSON_OVERRIDES_JOIN_SQL.format( person_overrides_table_alias=self.PERSON_ID_OVERRIDES_TABLE_ALIAS, diff --git a/posthog/queries/trends/lifecycle.py b/posthog/queries/trends/lifecycle.py index 4821d5295a363..6397439d8922b 100644 --- a/posthog/queries/trends/lifecycle.py +++ b/posthog/queries/trends/lifecycle.py @@ -188,7 +188,9 @@ def _get_date_filter(self): ) def _determine_should_join_distinct_ids(self) -> None: - self._should_join_distinct_ids = self._person_on_events_mode != PersonOnEventsMode.V1_ENABLED + self._should_join_distinct_ids = ( + self._person_on_events_mode != PersonOnEventsMode.PERSON_ID_NO_OVERRIDE_PROPERTIES_ON_EVENTS + ) def _determine_should_join_persons(self) -> None: self._should_join_persons = self._person_on_events_mode == PersonOnEventsMode.DISABLED diff --git a/posthog/queries/trends/total_volume.py b/posthog/queries/trends/total_volume.py index 31f5d83b4c15c..9c7977ca02592 100644 --- a/posthog/queries/trends/total_volume.py +++ b/posthog/queries/trends/total_volume.py @@ -59,9 +59,9 @@ def _total_volume_query(self, entity: Entity, filter: Filter, team: Team) -> Tup interval_func = get_interval_func_ch(filter.interval) person_id_alias = f"{self.DISTINCT_ID_TABLE_ALIAS}.person_id" - if team.person_on_events_mode == PersonOnEventsMode.V2_ENABLED: + if team.person_on_events_mode == PersonOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_ON_EVENTS: person_id_alias = f"if(notEmpty({self.PERSON_ID_OVERRIDES_TABLE_ALIAS}.person_id), {self.PERSON_ID_OVERRIDES_TABLE_ALIAS}.person_id, {self.EVENT_TABLE_ALIAS}.person_id)" - elif team.person_on_events_mode == PersonOnEventsMode.V1_ENABLED: + elif team.person_on_events_mode == PersonOnEventsMode.PERSON_ID_NO_OVERRIDE_PROPERTIES_ON_EVENTS: person_id_alias = f"{self.EVENT_TABLE_ALIAS}.person_id" aggregate_operation, join_condition, math_params = process_math( diff --git a/posthog/queries/trends/trends_actors.py b/posthog/queries/trends/trends_actors.py index 228eac4f799e3..1648e7575e7b2 100644 --- a/posthog/queries/trends/trends_actors.py +++ b/posthog/queries/trends/trends_actors.py @@ -104,7 +104,7 @@ def actor_query(self, limit_actors: Optional[bool] = True) -> Tuple[str, Dict]: team=self._team, entity=self.entity, should_join_distinct_ids=not self.is_aggregating_by_groups - and self._team.person_on_events_mode != PersonOnEventsMode.V1_ENABLED, + and self._team.person_on_events_mode != PersonOnEventsMode.PERSON_ID_NO_OVERRIDE_PROPERTIES_ON_EVENTS, extra_event_properties=["$window_id", "$session_id"] if self._filter.include_recordings else [], extra_fields=extra_fields, person_on_events_mode=self._team.person_on_events_mode, diff --git a/posthog/queries/trends/trends_event_query.py b/posthog/queries/trends/trends_event_query.py index 6ef5cf009dafc..d3bcc9f381290 100644 --- a/posthog/queries/trends/trends_event_query.py +++ b/posthog/queries/trends/trends_event_query.py @@ -10,7 +10,7 @@ def get_query(self) -> Tuple[str, Dict[str, Any]]: person_id_field = "" if self._should_join_distinct_ids: person_id_field = f", {self._person_id_alias} as person_id" - elif self._person_on_events_mode == PersonOnEventsMode.V1_ENABLED: + elif self._person_on_events_mode == PersonOnEventsMode.PERSON_ID_NO_OVERRIDE_PROPERTIES_ON_EVENTS: person_id_field = f", {self.EVENT_TABLE_ALIAS}.person_id as person_id" _fields = ( diff --git a/posthog/queries/trends/trends_event_query_base.py b/posthog/queries/trends/trends_event_query_base.py index 16f742febc52b..082aed8e556a0 100644 --- a/posthog/queries/trends/trends_event_query_base.py +++ b/posthog/queries/trends/trends_event_query_base.py @@ -80,7 +80,7 @@ def get_query_base(self) -> Tuple[str, Dict[str, Any]]: return query, self.params def _determine_should_join_distinct_ids(self) -> None: - if self._person_on_events_mode == PersonOnEventsMode.V1_ENABLED: + if self._person_on_events_mode == PersonOnEventsMode.PERSON_ID_NO_OVERRIDE_PROPERTIES_ON_EVENTS: self._should_join_distinct_ids = False is_entity_per_user = self._entity.math in ( diff --git a/posthog/queries/trends/util.py b/posthog/queries/trends/util.py index 382201a9e0203..b91e4d6ac185d 100644 --- a/posthog/queries/trends/util.py +++ b/posthog/queries/trends/util.py @@ -174,9 +174,9 @@ def determine_aggregator(entity: Entity, team: Team) -> str: return f'"$group_{entity.math_group_type_index}"' elif team.aggregate_users_by_distinct_id: return "e.distinct_id" - elif team.person_on_events_mode == PersonOnEventsMode.V1_ENABLED: + elif team.person_on_events_mode == PersonOnEventsMode.PERSON_ID_NO_OVERRIDE_PROPERTIES_ON_EVENTS: return "e.person_id" - elif team.person_on_events_mode == PersonOnEventsMode.V2_ENABLED: + elif team.person_on_events_mode == PersonOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_ON_EVENTS: return f"if(notEmpty(overrides.person_id), overrides.person_id, e.person_id)" else: return "pdi.person_id" diff --git a/posthog/queries/util.py b/posthog/queries/util.py index adf37193f6c29..be08fb732c8ce 100644 --- a/posthog/queries/util.py +++ b/posthog/queries/util.py @@ -178,7 +178,7 @@ def get_person_properties_mode(team: Team) -> PersonPropertiesMode: if team.person_on_events_mode == PersonOnEventsMode.DISABLED: return PersonPropertiesMode.USING_PERSON_PROPERTIES_COLUMN - if team.person_on_events_mode == PersonOnEventsMode.V2_ENABLED: + if team.person_on_events_mode == PersonOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_ON_EVENTS: return PersonPropertiesMode.DIRECT_ON_EVENTS_WITH_POE_V2 return PersonPropertiesMode.DIRECT_ON_EVENTS diff --git a/posthog/schema.py b/posthog/schema.py index aa9976ddd1e84..4c4af88191c09 100644 --- a/posthog/schema.py +++ b/posthog/schema.py @@ -427,6 +427,7 @@ class PersonsOnEventsMode(str, Enum): v1_mixed = "v1_mixed" v2_enabled = "v2_enabled" v3_enabled = "v3_enabled" + person_id_override_properties_joined = "person_id_override_properties_joined" class HogQLQueryModifiers(BaseModel): diff --git a/posthog/session_recordings/queries/session_recording_list_from_replay_summary.py b/posthog/session_recordings/queries/session_recording_list_from_replay_summary.py index 1b6ed593abaa5..4ff24160ae3e6 100644 --- a/posthog/session_recordings/queries/session_recording_list_from_replay_summary.py +++ b/posthog/session_recordings/queries/session_recording_list_from_replay_summary.py @@ -197,7 +197,7 @@ def _data_to_return(self, results: List[Any]) -> List[Dict[str, Any]]: def get_query(self) -> Tuple[str, Dict[str, Any]]: # we don't support PoE V1 - hopefully that's ok - if self._person_on_events_mode == PersonOnEventsMode.V2_ENABLED: + if self._person_on_events_mode == PersonOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_ON_EVENTS: return "", {} prop_query, prop_params = self._get_prop_groups( @@ -302,7 +302,7 @@ def _determine_should_join_events(self): ) has_poe_filters = ( - self._person_on_events_mode == PersonOnEventsMode.V2_ENABLED + self._person_on_events_mode == PersonOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_ON_EVENTS and len( [ pg @@ -314,7 +314,8 @@ def _determine_should_join_events(self): ) has_poe_person_filter = ( - self._person_on_events_mode == PersonOnEventsMode.V2_ENABLED and self._filter.person_uuid + self._person_on_events_mode == PersonOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_ON_EVENTS + and self._filter.person_uuid ) return filters_by_event_or_action or has_event_property_filters or has_poe_filters or has_poe_person_filter @@ -370,7 +371,7 @@ def format_event_filter(self, entity: Entity, prepend: str, team_id: int) -> Tup allow_denormalized_props=True, has_person_id_joined=True, person_properties_mode=PersonPropertiesMode.DIRECT_ON_EVENTS_WITH_POE_V2 - if self._person_on_events_mode == PersonOnEventsMode.V2_ENABLED + if self._person_on_events_mode == PersonOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_ON_EVENTS else PersonPropertiesMode.USING_PERSON_PROPERTIES_COLUMN, hogql_context=self._filter.hogql_context, ) @@ -415,7 +416,7 @@ def build_event_filters(self) -> SummaryEventFiltersSQL: -- select the unique events in this session to support filtering sessions by presence of an event groupUniqArray(event) as event_names,""" - if self._person_on_events_mode == PersonOnEventsMode.V2_ENABLED: + if self._person_on_events_mode == PersonOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_ON_EVENTS: person_id_clause, person_id_params = self._get_person_id_clause condition_sql += person_id_clause params = {**params, **person_id_params} @@ -491,7 +492,10 @@ def get_query(self, select_event_ids: bool = False) -> Tuple[str, Dict[str, Any] values=[ g for g in self._filter.property_groups.flat - if (self._person_on_events_mode == PersonOnEventsMode.V2_ENABLED and g.type == "person") + if ( + self._person_on_events_mode == PersonOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_ON_EVENTS + and g.type == "person" + ) or ( (g.type == "hogql" and "person.properties" not in g.key) or (g.type != "hogql" and "cohort" not in g.type and g.type != "person") @@ -505,7 +509,7 @@ def get_query(self, select_event_ids: bool = False) -> Tuple[str, Dict[str, Any] # but would need careful monitoring allow_denormalized_props=settings.ALLOW_DENORMALIZED_PROPS_IN_LISTING, person_properties_mode=PersonPropertiesMode.DIRECT_ON_EVENTS_WITH_POE_V2 - if self._person_on_events_mode == PersonOnEventsMode.V2_ENABLED + if self._person_on_events_mode == PersonOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_ON_EVENTS else PersonPropertiesMode.USING_PERSON_PROPERTIES_COLUMN, ) diff --git a/posthog/settings/__init__.py b/posthog/settings/__init__.py index dd72e63bcc92a..455b7e8dc34a1 100644 --- a/posthog/settings/__init__.py +++ b/posthog/settings/__init__.py @@ -96,7 +96,6 @@ # Only written in specific scripts - do not use outside of them. PERSON_ON_EVENTS_V2_OVERRIDE = get_from_env("PERSON_ON_EVENTS_V2_OVERRIDE", optional=True, type_cast=str_to_bool) -PERSON_ON_EVENTS_V3_OVERRIDE = get_from_env("PERSON_ON_EVENTS_V3_OVERRIDE", optional=True, type_cast=str_to_bool) # Wether to use insight queries converted to HogQL. HOGQL_INSIGHTS_OVERRIDE = get_from_env("HOGQL_INSIGHTS_OVERRIDE", optional=True, type_cast=str_to_bool) diff --git a/posthog/test/test_team.py b/posthog/test/test_team.py index 25f73dcfa87a9..c6e9a681b4839 100644 --- a/posthog/test/test_team.py +++ b/posthog/test/test_team.py @@ -138,7 +138,7 @@ def test_team_on_cloud_uses_feature_flag_to_determine_person_on_events(self, moc with self.is_cloud(True): with override_instance_config("PERSON_ON_EVENTS_ENABLED", False): team = Team.objects.create_with_data(organization=self.organization) - self.assertEqual(team.person_on_events_mode, PersonOnEventsMode.V2_ENABLED) + self.assertEqual(team.person_on_events_mode, PersonOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_ON_EVENTS) # called more than once when evaluating hogql mock_feature_enabled.assert_called_with( "persons-on-events-v2-reads-enabled", @@ -159,7 +159,7 @@ def test_team_on_self_hosted_uses_instance_setting_to_determine_person_on_events with self.is_cloud(False): with override_instance_config("PERSON_ON_EVENTS_V2_ENABLED", True): team = Team.objects.create_with_data(organization=self.organization) - self.assertEqual(team.person_on_events_mode, PersonOnEventsMode.V2_ENABLED) + self.assertEqual(team.person_on_events_mode, PersonOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_ON_EVENTS) mock_feature_enabled.assert_not_called() with override_instance_config("PERSON_ON_EVENTS_V2_ENABLED", False): diff --git a/posthog/utils.py b/posthog/utils.py index baa85c2c6d614..152d103e76710 100644 --- a/posthog/utils.py +++ b/posthog/utils.py @@ -1305,8 +1305,9 @@ def patch(wrapper): class PersonOnEventsMode(str, Enum): DISABLED = "disabled" - V1_ENABLED = "v1_enabled" - V2_ENABLED = "v2_enabled" + PERSON_ID_NO_OVERRIDE_PROPERTIES_ON_EVENTS = "v1_enabled" + PERSON_ID_OVERRIDE_PROPERTIES_ON_EVENTS = "v2_enabled" + PERSON_ID_OVERRIDE_PROPERTIES_JOINED = "person_id_override_properties_joined" def label_for_team_id_to_track(team_id: int) -> str: