Skip to content

Commit

Permalink
feat(hogql): Add feature flag for opting queries into v3 persons-on-e…
Browse files Browse the repository at this point in the history
…vents (#21150)
  • Loading branch information
tkaemming authored Mar 27, 2024
1 parent ccee1fb commit 205376b
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 4 deletions.
18 changes: 18 additions & 0 deletions posthog/api/test/__snapshots__/test_insight.ambr
Original file line number Diff line number Diff line change
Expand Up @@ -1616,6 +1616,24 @@
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",
Expand Down
14 changes: 11 additions & 3 deletions posthog/hogql/modifiers.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
from typing import Optional, TYPE_CHECKING

from posthog.schema import HogQLQueryModifiers, InCohortVia, MaterializationMode, PersonsArgMaxVersion
from posthog.utils import PersonOnEventsMode
from posthog.schema import (
HogQLQueryModifiers,
InCohortVia,
MaterializationMode,
PersonsArgMaxVersion,
PersonsOnEventsMode,
)

if TYPE_CHECKING:
from posthog.models import Team
Expand All @@ -16,7 +21,10 @@ def create_default_modifiers_for_team(
modifiers = modifiers.model_copy()

if modifiers.personsOnEventsMode is None:
modifiers.personsOnEventsMode = team.person_on_events_mode or PersonOnEventsMode.DISABLED
if team.person_on_events_v3_querying_enabled:
modifiers.personsOnEventsMode = PersonsOnEventsMode.v3_enabled
else:
modifiers.personsOnEventsMode = team.person_on_events_mode

if modifiers.personsArgMaxVersion is None:
modifiers.personsArgMaxVersion = PersonsArgMaxVersion.auto
Expand Down
9 changes: 8 additions & 1 deletion posthog/hogql/test/test_modifiers.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@


class TestModifiers(BaseTest):
@override_settings(PERSON_ON_EVENTS_OVERRIDE=False, PERSON_ON_EVENTS_V2_OVERRIDE=False)
@override_settings(
PERSON_ON_EVENTS_OVERRIDE=False,
PERSON_ON_EVENTS_V2_OVERRIDE=False,
PERSON_ON_EVENTS_V3_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)
Expand All @@ -23,6 +27,9 @@ 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"

Expand Down
19 changes: 19 additions & 0 deletions posthog/models/team/team.py
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,25 @@ 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,
)

@property
def strict_caching_enabled(self) -> bool:
enabled_teams = get_list(get_instance_setting("STRICT_CACHING_TEAMS"))
Expand Down
1 change: 1 addition & 0 deletions posthog/settings/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@

# 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)
Expand Down

0 comments on commit 205376b

Please sign in to comment.