Skip to content

Commit

Permalink
fix PoEv2 failures
Browse files Browse the repository at this point in the history
  • Loading branch information
mariusandra committed Nov 23, 2023
1 parent d151196 commit eccb279
Show file tree
Hide file tree
Showing 12 changed files with 79 additions and 57 deletions.
5 changes: 4 additions & 1 deletion posthog/hogql/database/database.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,10 @@ def create_hogql_database(team_id: int, modifiers: Optional[HogQLQueryModifiers]
)
database.events.fields["person_id"] = ExpressionField(
name="person_id",
expr=parse_expr("ifNull(override.override_person_id, event_person_id)", start=None),
expr=parse_expr(
"ifNull(nullIf(override.override_person_id, '00000000-0000-0000-0000-000000000000'), event_person_id)",
start=None,
),
)
database.events.fields["poe"].fields["id"] = database.events.fields["person_id"]
database.events.fields["person"] = FieldTraverser(chain=["poe"])
Expand Down
4 changes: 2 additions & 2 deletions posthog/hogql/database/schema/test/test_event_sessions.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def setUp(self):

def _select(self, query: str) -> ast.SelectQuery:
select_query = cast(ast.SelectQuery, clone_expr(parse_select(query), clear_locations=True))
return cast(ast.SelectQuery, resolve_types(select_query, self.context))
return cast(ast.SelectQuery, resolve_types(select_query, self.context, dialect="clickhouse"))

def _compare_operators(self, query: ast.SelectQuery, table_name: str) -> List[ast.Expr]:
assert query.where is not None and query.type is not None
Expand Down Expand Up @@ -143,7 +143,7 @@ def setUp(self):

def _select(self, query: str) -> ast.SelectQuery:
select_query = cast(ast.SelectQuery, clone_expr(parse_select(query), clear_locations=True))
return cast(ast.SelectQuery, resolve_types(select_query, self.context))
return cast(ast.SelectQuery, resolve_types(select_query, self.context, dialect="clickhouse"))

def _clean(self, table_name: str, query: ast.SelectQuery, expr: ast.Expr) -> ast.Expr:
assert query.type is not None
Expand Down
6 changes: 3 additions & 3 deletions posthog/hogql/printer.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,15 +96,15 @@ def prepare_ast_for_printing(
context.database = context.database or create_hogql_database(context.team_id, context.modifiers)

with context.timings.measure("resolve_types"):
node = resolve_types(node, context, scopes=[node.type for node in stack] if stack else None)
node = resolve_types(node, context, dialect=dialect, scopes=[node.type for node in stack] if stack else None)
if context.modifiers.inCohortVia == "leftjoin":
with context.timings.measure("resolve_in_cohorts"):
resolve_in_cohorts(node, stack, context)
resolve_in_cohorts(node, dialect, stack, context)
if dialect == "clickhouse":
with context.timings.measure("resolve_property_types"):
node = resolve_property_types(node, context)
with context.timings.measure("resolve_lazy_tables"):
resolve_lazy_tables(node, stack, context)
resolve_lazy_tables(node, dialect, stack, context)

# We support global query settings, and local subquery settings.
# If the global query is a select query with settings, merge the two.
Expand Down
23 changes: 16 additions & 7 deletions posthog/hogql/resolver.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from datetime import date, datetime
from typing import List, Optional, Any, cast
from typing import List, Optional, Any, cast, Literal
from uuid import UUID

from posthog.hogql import ast
Expand Down Expand Up @@ -56,20 +56,27 @@ def resolve_constant_data_type(constant: Any) -> ConstantType:
def resolve_types(
node: ast.Expr,
context: HogQLContext,
dialect: Literal["hogql", "clickhouse"],
scopes: Optional[List[ast.SelectQueryType]] = None,
) -> ast.Expr:
return Resolver(scopes=scopes, context=context).visit(node)
return Resolver(scopes=scopes, context=context, dialect=dialect).visit(node)


class Resolver(CloningVisitor):
"""The Resolver visits an AST and 1) resolves all fields, 2) assigns types to nodes, 3) expands all CTEs."""

def __init__(self, context: HogQLContext, scopes: Optional[List[ast.SelectQueryType]] = None):
def __init__(
self,
context: HogQLContext,
dialect: Literal["hogql", "clickhouse"] = "clickhouse",
scopes: Optional[List[ast.SelectQueryType]] = None,
):
super().__init__()
# Each SELECT query creates a new scope (type). Store all of them in a list as we traverse the tree.
self.scopes: List[ast.SelectQueryType] = scopes or []
self.current_view_depth: int = 0
self.context = context
self.dialect = dialect
self.database = context.database
self.cte_counter = 0

Expand Down Expand Up @@ -461,10 +468,12 @@ def visit_field(self, node: ast.Field):
node.type = loop_type

if isinstance(node.type, ast.ExpressionFieldType):
new_expr = clone_expr(node.type.expr)
new_node = ast.Alias(alias=node.type.name, expr=new_expr, hidden=True)
new_node = self.visit(new_node)
return new_node
# only swap out expression fields in ClickHouse
if self.dialect == "clickhouse":
new_expr = clone_expr(node.type.expr)
new_node = ast.Alias(alias=node.type.name, expr=new_expr, hidden=True)
new_node = self.visit(new_node)
return new_node

if isinstance(node.type, ast.FieldType) and node.start is not None and node.end is not None:
self.context.add_notice(
Expand Down
2 changes: 2 additions & 0 deletions posthog/hogql/test/test_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from posthog.models import PropertyDefinition, Cohort
from posthog.schema import HogQLMetadata, HogQLMetadataResponse
from posthog.test.base import APIBaseTest, ClickhouseTestMixin
from django.test import override_settings


class TestMetadata(ClickhouseTestMixin, APIBaseTest):
Expand Down Expand Up @@ -135,6 +136,7 @@ def test_metadata_table(self):
metadata = self._expr("is_identified", "persons")
self.assertEqual(metadata.isValid, True)

@override_settings(PERSON_ON_EVENTS_OVERRIDE=True, PERSON_ON_EVENTS_V2_OVERRIDE=False)
def test_metadata_in_cohort(self):
cohort = Cohort.objects.create(team=self.team, name="cohort_name")
query = (
Expand Down
4 changes: 2 additions & 2 deletions posthog/hogql/test/test_modifiers.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,9 @@ def test_modifiers_persons_on_events_mode_mapping(self):
(
PersonsOnEventsMode.v2_enabled,
"events.event",
"ifNull(events__override.override_person_id, events.person_id) AS id",
"ifNull(nullIf(events__override.override_person_id, %(hogql_val_0)s), events.person_id) AS id",
"events.person_properties",
"toTimeZone(events.person_created_at, %(hogql_val_0)s) AS created_at",
"toTimeZone(events.person_created_at, %(hogql_val_1)s) AS created_at",
),
]

Expand Down
2 changes: 1 addition & 1 deletion posthog/hogql/test/test_printer.py
Original file line number Diff line number Diff line change
Expand Up @@ -774,7 +774,7 @@ def test_select_sample(self):
f"AS persons SAMPLE 0.1 ON equals(persons.id, events__pdi.person_id) WHERE equals(events.team_id, {self.team.pk}) LIMIT 10000",
)

with override_settings(PERSON_ON_EVENTS_OVERRIDE=True):
with override_settings(PERSON_ON_EVENTS_OVERRIDE=True, PERSON_ON_EVENTS_V2_OVERRIDE=False):
context = HogQLContext(
team_id=self.team.pk,
enable_select_queries=True,
Expand Down
4 changes: 2 additions & 2 deletions posthog/hogql/test/test_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ def test_query_select_person_with_joins_without_poe(self):
self.assertEqual(response.results[0][3], "[email protected]")

@pytest.mark.usefixtures("unittest_snapshot")
@override_settings(PERSON_ON_EVENTS_OVERRIDE=True)
@override_settings(PERSON_ON_EVENTS_OVERRIDE=True, PERSON_ON_EVENTS_V2_OVERRIDE=False)
def test_query_select_person_with_poe_without_joins(self):
with freeze_time("2020-01-10"):
self._create_random_events()
Expand Down Expand Up @@ -473,7 +473,7 @@ def test_prop_cohort_basic(self):
)
self.assertEqual(response.results, [("$pageview", 2)])

with override_settings(PERSON_ON_EVENTS_OVERRIDE=True):
with override_settings(PERSON_ON_EVENTS_OVERRIDE=True, PERSON_ON_EVENTS_V2_OVERRIDE=False):
response = execute_hogql_query(
"SELECT event, count(*) FROM events WHERE {cohort_filter} GROUP BY event",
team=self.team,
Expand Down
Loading

0 comments on commit eccb279

Please sign in to comment.