Skip to content

Commit

Permalink
refactor: Simplify web_overview
Browse files Browse the repository at this point in the history
Use same methods as we're using on `stats_table` to calculate previous metrics. We've extracted this up to `web_analytics_query_runner`
  • Loading branch information
rafaeelaudibert committed Dec 12, 2024
1 parent 968ecf0 commit fd09fea
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 65 deletions.
46 changes: 0 additions & 46 deletions posthog/hogql_queries/web_analytics/stats_table.py
Original file line number Diff line number Diff line change
Expand Up @@ -351,52 +351,6 @@ def _main_inner_query(self, breakdown):

return query

def _current_period_expression(self, field="start_timestamp"):
return ast.Call(
name="and",
args=[
ast.CompareOperation(
left=ast.Field(chain=[field]),
right=self.query_date_range.date_from_as_hogql(),
op=ast.CompareOperationOp.GtEq,
),
ast.CompareOperation(
left=ast.Field(chain=[field]),
right=self.query_date_range.date_to_as_hogql(),
op=ast.CompareOperationOp.Lt,
),
],
)

def _previous_period_expression(self, field="start_timestamp"):
if not self.query_compare_to_date_range:
return ast.Constant(value=None)

return ast.Call(
name="and",
args=[
ast.CompareOperation(
left=ast.Field(chain=[field]),
right=self.query_compare_to_date_range.date_from_as_hogql(),
op=ast.CompareOperationOp.GtEq,
),
ast.CompareOperation(
left=ast.Field(chain=[field]),
right=self.query_compare_to_date_range.date_to_as_hogql(),
op=ast.CompareOperationOp.Lt,
),
],
)

def _periods_expression(self, field="timestamp"):
return ast.Call(
name="or",
args=[
self._current_period_expression(field),
self._previous_period_expression(field),
],
)

def _period_comparison_tuple(self, column, alias, function_name):
return ast.Alias(
alias=alias,
Expand Down
46 changes: 46 additions & 0 deletions posthog/hogql_queries/web_analytics/web_analytics_query_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,52 @@ def query_compare_to_date_range(self):

return None

def _current_period_expression(self, field="start_timestamp"):
return ast.Call(
name="and",
args=[
ast.CompareOperation(
left=ast.Field(chain=[field]),
right=self.query_date_range.date_from_as_hogql(),
op=ast.CompareOperationOp.GtEq,
),
ast.CompareOperation(
left=ast.Field(chain=[field]),
right=self.query_date_range.date_to_as_hogql(),
op=ast.CompareOperationOp.Lt,
),
],
)

def _previous_period_expression(self, field="start_timestamp"):
if not self.query_compare_to_date_range:
return ast.Constant(value=None)

return ast.Call(
name="and",
args=[
ast.CompareOperation(
left=ast.Field(chain=[field]),
right=self.query_compare_to_date_range.date_from_as_hogql(),
op=ast.CompareOperationOp.GtEq,
),
ast.CompareOperation(
left=ast.Field(chain=[field]),
right=self.query_compare_to_date_range.date_to_as_hogql(),
op=ast.CompareOperationOp.Lt,
),
],
)

def _periods_expression(self, field="timestamp"):
return ast.Call(
name="or",
args=[
self._current_period_expression(field),
self._previous_period_expression(field),
],
)

@cached_property
def pathname_property_filter(self) -> Optional[EventPropertyFilter]:
for p in self.query.properties:
Expand Down
23 changes: 4 additions & 19 deletions posthog/hogql_queries/web_analytics/web_overview.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,34 +110,19 @@ def inner_select(self) -> ast.SelectQuery:
WHERE and(
events.`$session_id` IS NOT NULL,
{event_type_expr},
or(
and(timestamp >= {current_date_range_start}, timestamp < {current_date_range_end}),
and(timestamp >= {previous_date_range_start}, timestamp < {previous_date_range_end})
),
{inside_timestamp_period},
{event_properties},
{session_properties}
)
GROUP BY session_id
HAVING or(
and(start_timestamp >= {current_date_range_start}, start_timestamp < {current_date_range_end}),
and(start_timestamp >= {previous_date_range_start}, start_timestamp < {previous_date_range_end})
)
HAVING {inside_start_timestamp_period}
""",
placeholders={
"event_properties": self.event_properties(),
"session_properties": self.session_properties(),
"event_type_expr": self.event_type_expr,
# It's ok that we return ast.Constant(value=None) for previous_date_range_start
# and previous_date_range_end if there's no compare_to date range
# because HogQL will simply ignore that part of the where clause, it's that good!
"current_date_range_start": self.query_date_range.date_from_as_hogql(),
"current_date_range_end": self.query_date_range.date_to_as_hogql(),
"previous_date_range_start": self.query_compare_to_date_range.date_from_as_hogql()
if self.query_compare_to_date_range
else ast.Constant(value=None),
"previous_date_range_end": self.query_compare_to_date_range.date_to_as_hogql()
if self.query_compare_to_date_range
else ast.Constant(value=None),
"inside_timestamp_period": self._periods_expression("timestamp"),
"inside_start_timestamp_period": self._periods_expression("start_timestamp"),
},
)
assert isinstance(parsed_select, ast.SelectQuery)
Expand Down

0 comments on commit fd09fea

Please sign in to comment.