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

feat(hogql): mixed PoE mode #18064

Merged
merged 2 commits into from
Oct 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion frontend/src/queries/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -1244,7 +1244,7 @@
"type": "string"
},
"personsOnEventsMode": {
"enum": ["disabled", "v1_enabled", "v2_enabled"],
"enum": ["disabled", "v1_enabled", "v1_mixed", "v2_enabled"],
"type": "string"
}
},
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 @@ -131,7 +131,7 @@ 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' | 'v2_enabled'
personsOnEventsMode?: 'disabled' | 'v1_enabled' | 'v1_mixed' | 'v2_enabled'
personsArgMaxVersion?: 'auto' | 'v1' | 'v2'
inCohortVia?: 'leftjoin' | 'subquery'
}
Expand Down
1 change: 1 addition & 0 deletions frontend/src/scenes/debug/HogQLDebug.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export function HogQLDebug({ query, setQuery, queryKey }: HogQLDebugProps): JSX.
options={[
{ value: 'disabled', label: 'Disabled' },
{ value: 'v1_enabled', label: 'V1 Enabled' },
{ value: 'v1_mixed', label: 'V1 Mixed' },
{ value: 'v2_enabled', label: 'V2 Enabled' },
]}
onChange={(value) =>
Expand Down
22 changes: 19 additions & 3 deletions posthog/hogql/database/database.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@
from posthog.hogql.errors import HogQLException
from posthog.models.group_type_mapping import GroupTypeMapping
from posthog.models.team.team import WeekStartDay
from posthog.schema import HogQLQueryModifiers
from posthog.utils import PersonOnEventsMode
from posthog.schema import HogQLQueryModifiers, PersonsOnEventsMode


class Database(BaseModel):
Expand Down Expand Up @@ -117,7 +116,24 @@ def create_hogql_database(team_id: int, modifiers: Optional[HogQLQueryModifiers]
team = Team.objects.get(pk=team_id)
modifiers = create_default_modifiers_for_team(team, modifiers)
database = Database(timezone=team.timezone, week_start_day=team.week_start_day)
if modifiers.personsOnEventsMode != PersonOnEventsMode.DISABLED:

if modifiers.personsOnEventsMode == PersonsOnEventsMode.disabled:
# no change
database.events.fields["person"] = FieldTraverser(chain=["pdi", "person"])
database.events.fields["person_id"] = FieldTraverser(chain=["pdi", "person_id"])

elif modifiers.personsOnEventsMode == PersonsOnEventsMode.v1_mixed:
# person.id via a join, person.properties on events
database.events.fields["person_id"] = FieldTraverser(chain=["pdi", "person_id"])
database.events.fields["person"] = FieldTraverser(chain=["poe"])
database.events.fields["poe"].fields["id"] = FieldTraverser(chain=["..", "pdi", "person_id"])
database.events.fields["poe"].fields["created_at"] = FieldTraverser(chain=["..", "pdi", "person", "created_at"])
database.events.fields["poe"].fields["properties"] = StringJSONDatabaseField(name="person_properties")

elif (
modifiers.personsOnEventsMode == PersonsOnEventsMode.v1_enabled
or modifiers.personsOnEventsMode == PersonsOnEventsMode.v2_enabled
):
# TODO: split PoE v1 and v2 once SQL Expression fields are supported #15180
database.events.fields["person"] = FieldTraverser(chain=["poe"])
database.events.fields["person_id"] = StringDatabaseField(name="person_id")
Expand Down
8 changes: 8 additions & 0 deletions posthog/hogql/resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -412,14 +412,22 @@ def visit_field(self, node: ast.Field):
# Recursively resolve the rest of the chain until we can point to the deepest node.
loop_type = type
chain_to_parse = node.chain[1:]
previous_types = []
while True:
if isinstance(loop_type, FieldTraverserType):
chain_to_parse = loop_type.chain + chain_to_parse
loop_type = loop_type.table_type
continue
previous_types.append(loop_type)
if len(chain_to_parse) == 0:
break
next_chain = chain_to_parse.pop(0)
if next_chain == "..": # only support one level of ".."
previous_types.pop()
previous_types.pop()
loop_type = previous_types[-1]
next_chain = chain_to_parse.pop(0)

loop_type = loop_type.get_child(next_chain)
if loop_type is None:
raise ResolverException(f"Cannot resolve type {'.'.join(node.chain)}. Unable to resolve {next_chain}.")
Expand Down
57 changes: 48 additions & 9 deletions posthog/hogql/test/test_modifiers.py
Original file line number Diff line number Diff line change
@@ -1,42 +1,81 @@
from posthog.hogql.modifiers import create_default_modifiers_for_team
from posthog.hogql.query import execute_hogql_query
from posthog.models import Cohort
from posthog.schema import HogQLQueryModifiers
from posthog.schema import HogQLQueryModifiers, PersonsOnEventsMode
from posthog.test.base import BaseTest
from django.test import override_settings
from posthog.utils import PersonOnEventsMode


class TestModifiers(BaseTest):
@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)
assert modifiers.personsOnEventsMode == PersonOnEventsMode.DISABLED # NB! not a None
assert modifiers.personsOnEventsMode == PersonsOnEventsMode.disabled # NB! not a None
modifiers = create_default_modifiers_for_team(
self.team, HogQLQueryModifiers(personsOnEventsMode=PersonOnEventsMode.V1_ENABLED)
self.team, HogQLQueryModifiers(personsOnEventsMode=PersonsOnEventsMode.v1_enabled)
)
assert modifiers.personsOnEventsMode == PersonOnEventsMode.V1_ENABLED
assert modifiers.personsOnEventsMode == PersonsOnEventsMode.v1_enabled
modifiers = create_default_modifiers_for_team(
self.team, HogQLQueryModifiers(personsOnEventsMode=PersonOnEventsMode.V2_ENABLED)
self.team, HogQLQueryModifiers(personsOnEventsMode=PersonsOnEventsMode.v2_enabled)
)
assert modifiers.personsOnEventsMode == PersonOnEventsMode.V2_ENABLED
assert modifiers.personsOnEventsMode == PersonsOnEventsMode.v2_enabled

def test_modifiers_persons_on_events_mode_v1_enabled(self):
query = "SELECT event, person_id FROM events"

# Control
response = execute_hogql_query(
query, team=self.team, modifiers=HogQLQueryModifiers(personsOnEventsMode=PersonOnEventsMode.DISABLED)
query, team=self.team, modifiers=HogQLQueryModifiers(personsOnEventsMode=PersonsOnEventsMode.disabled)
)
assert " JOIN " in response.clickhouse

# Test
response = execute_hogql_query(
query, team=self.team, modifiers=HogQLQueryModifiers(personsOnEventsMode=PersonOnEventsMode.V1_ENABLED)
query, team=self.team, modifiers=HogQLQueryModifiers(personsOnEventsMode=PersonsOnEventsMode.v1_enabled)
)
assert " JOIN " not in response.clickhouse

def test_modifiers_persons_on_events_mode_mapping(self):
query = "SELECT event, person.id, person.properties, person.created_at FROM events"

test_cases = [
(
PersonsOnEventsMode.disabled,
"events.event",
"events__pdi__person.id",
"events__pdi__person.properties",
"toTimeZone(events__pdi__person.created_at, %(hogql_val_0)s)",
),
(
PersonsOnEventsMode.v1_enabled,
"events.event",
"events.person_id",
"events.person_properties",
"toTimeZone(events.person_created_at, %(hogql_val_0)s)",
),
(
PersonsOnEventsMode.v1_mixed,
"events.event",
"events__pdi.person_id",
"events.person_properties",
"toTimeZone(events__pdi__person.created_at, %(hogql_val_0)s)",
),
(
PersonsOnEventsMode.v2_enabled,
"events.event",
"events.person_id",
"events.person_properties",
"toTimeZone(events.person_created_at, %(hogql_val_0)s)",
),
]

for (mode, *expected) in test_cases:
response = execute_hogql_query(
query, team=self.team, modifiers=HogQLQueryModifiers(personsOnEventsMode=mode)
)
assert f"SELECT {', '.join(expected)} FROM" in response.clickhouse, f"PoE mode: {mode}"

def test_modifiers_persons_argmax_version_v2(self):
query = "SELECT * FROM persons"

Expand Down
34 changes: 33 additions & 1 deletion posthog/hogql/test/test_resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,13 @@
from posthog.hogql import ast
from posthog.hogql.context import HogQLContext
from posthog.hogql.database.database import create_hogql_database
from posthog.hogql.database.models import LazyJoin
from posthog.hogql.database.models import (
LazyJoin,
FieldTraverser,
StringJSONDatabaseField,
StringDatabaseField,
DateTimeDatabaseField,
)
from posthog.hogql.visitor import clone_expr
from posthog.hogql.parser import parse_select
from posthog.hogql.printer import print_ast
Expand Down Expand Up @@ -992,3 +998,29 @@ def test_lambda_parent_scope(self):
self.assertEqual(lambda_type.parent, node.type)
self.assertEqual(list(lambda_type.aliases.keys()), ["x"])
self.assertEqual(list(lambda_type.parent.columns.keys()), ["timestamp"])

def test_field_traverser_double_dot(self):
# Create a condition where we want to ".." out of "events.poe." to get to a higher level prop
self.database.events.fields["person"] = FieldTraverser(chain=["poe"])
self.database.events.fields["poe"].fields["id"] = FieldTraverser(chain=["..", "pdi", "person_id"])
self.database.events.fields["poe"].fields["created_at"] = FieldTraverser(
chain=["..", "pdi", "person", "created_at"]
)
self.database.events.fields["poe"].fields["properties"] = StringJSONDatabaseField(name="person_properties")

node = self._select("SELECT event, person.id, person.properties, person.created_at FROM events")
node = cast(ast.SelectQuery, resolve_types(node, self.context))

# all columns resolve to a type in the end
assert cast(ast.FieldType, node.select[0].type).resolve_database_field() == StringDatabaseField(
name="event", array=None, nullable=None
)
assert cast(ast.FieldType, node.select[1].type).resolve_database_field() == StringDatabaseField(
name="person_id", array=None, nullable=None
)
assert cast(ast.FieldType, node.select[2].type).resolve_database_field() == StringJSONDatabaseField(
name="person_properties"
)
assert cast(ast.FieldType, node.select[3].type).resolve_database_field() == DateTimeDatabaseField(
name="created_at", array=None, nullable=None
)
1 change: 1 addition & 0 deletions posthog/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@ class PersonsArgMaxVersion(str, Enum):
class PersonsOnEventsMode(str, Enum):
disabled = "disabled"
v1_enabled = "v1_enabled"
v1_mixed = "v1_mixed"
v2_enabled = "v2_enabled"


Expand Down