Skip to content

Commit

Permalink
fix a lot of tests
Browse files Browse the repository at this point in the history
  • Loading branch information
mariusandra committed Oct 4, 2024
1 parent 047f97a commit 028a47d
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 18 deletions.
2 changes: 1 addition & 1 deletion posthog/hogql/placeholders.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ def _visit_placeholder_via_fields(self, node):

def visit_placeholder(self, node):
# TODO: HOG_PLACEHOLDERS feature flag
if True:
if len("use a flag here") > 0:
return self._visit_placeholder_via_bytecode(node)
else:
return self._visit_placeholder_via_fields(node)
37 changes: 20 additions & 17 deletions posthog/hogql_queries/insights/trends/aggregation_operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from posthog.constants import NON_TIME_SERIES_DISPLAY_TYPES
from posthog.hogql import ast
from posthog.hogql.parser import parse_expr, parse_select
from posthog.hogql.placeholders import replace_placeholders
from posthog.hogql_queries.insights.data_warehouse_mixin import (
DataWarehouseInsightQueryMixin,
)
Expand All @@ -21,6 +22,10 @@
)


def create_placeholder(name: str) -> ast.Placeholder:
return ast.Placeholder(expr=ast.Field(chain=[name]))


class AggregationOperations(DataWarehouseInsightQueryMixin):
team: Team
series: Union[EventsNode, ActionsNode, DataWarehouseNode]
Expand Down Expand Up @@ -51,13 +56,9 @@ def select_aggregation(self) -> ast.Expr:
actor = "e.distinct_id" if self.team.aggregate_users_by_distinct_id else "e.person_id"
return parse_expr(f"count(DISTINCT {actor})")
elif self.series.math == "weekly_active":
return ast.Placeholder(
expr=ast.Field(chain=["replaced"])
) # This gets replaced when doing query orchestration
return create_placeholder("replaced") # This gets replaced when doing query orchestration
elif self.series.math == "monthly_active":
return ast.Placeholder(
expr=ast.Field(chain=["replaced"])
) # This gets replaced when doing query orchestration
return create_placeholder("replaced") # This gets replaced when doing query orchestration
elif self.series.math == "unique_session":
return parse_expr('count(DISTINCT e."$session_id")')
elif self.series.math == "unique_group" and self.series.math_group_type_index is not None:
Expand Down Expand Up @@ -174,12 +175,12 @@ def _interval_function_name(self) -> str:
return f"toStartOf{self.query_date_range.interval_name.title()}"

def _actors_parent_select_query(
self, inner_query: ast.SelectQuery | ast.SelectUnionQuery
self, # inner_query: ast.SelectQuery | ast.SelectUnionQuery
) -> ast.SelectQuery | ast.SelectUnionQuery:
if self.is_count_per_actor_variant():
query = parse_select(
"SELECT total FROM {inner_query}",
placeholders={"inner_query": inner_query},
placeholders={"inner_query": create_placeholder("inner_query")},
)

if not self.is_total_value:
Expand All @@ -202,7 +203,7 @@ def _actors_parent_select_query(
""",
placeholders={
**self.query_date_range.to_placeholders(),
"inner_query": inner_query,
"inner_query": create_placeholder("inner_query"),
},
),
)
Expand All @@ -219,7 +220,7 @@ def _actors_parent_select_query(
return query

def _actors_inner_select_query(
self, cross_join_select_query: ast.SelectQuery | ast.SelectUnionQuery
self, # , cross_join_select_query: ast.SelectQuery | ast.SelectUnionQuery
) -> ast.SelectQuery | ast.SelectUnionQuery:
if self.is_count_per_actor_variant():
if self.series.math == "avg_count_per_actor":
Expand Down Expand Up @@ -248,7 +249,7 @@ def _actors_inner_select_query(
FROM {inner_query}
""",
placeholders={
"inner_query": cross_join_select_query,
"inner_query": create_placeholder("events_query"),
"total_alias": total_alias,
},
)
Expand Down Expand Up @@ -282,7 +283,7 @@ def _actors_inner_select_query(
placeholders={
**self.query_date_range.to_placeholders(),
**self._interval_placeholders(),
"cross_join_select_query": cross_join_select_query,
"cross_join_select_query": create_placeholder("events_query"),
},
),
)
Expand Down Expand Up @@ -394,8 +395,8 @@ def _actors_events_query(

def get_actors_query_orchestrator(self, events_where_clause: ast.Expr, sample_value: ast.RatioExpr):
events_query = cast(ast.SelectQuery, self._actors_events_query(events_where_clause, sample_value))
inner_select = cast(ast.SelectQuery, self._actors_inner_select_query(events_query))
parent_select = cast(ast.SelectQuery, self._actors_parent_select_query(inner_select))
inner_select = cast(ast.SelectQuery, self._actors_inner_select_query())
parent_select = cast(ast.SelectQuery, self._actors_parent_select_query())

class QueryOrchestrator:
events_query_builder: QueryAlternator
Expand All @@ -408,9 +409,11 @@ def __init__(self):
self.parent_select_query_builder = QueryAlternator(parent_select)

def build(self):
self.events_query_builder.build()
self.inner_select_query_builder.build()
return self.parent_select_query_builder.build()
events_query = self.events_query_builder.build()
inner_query = replace_placeholders(
self.inner_select_query_builder.build(), {"events_query": events_query}
)
return replace_placeholders(self.parent_select_query_builder.build(), {"inner_query": inner_query})

return QueryOrchestrator()

Expand Down

0 comments on commit 028a47d

Please sign in to comment.