Skip to content

Commit

Permalink
fix(hogql): fix trends actors include recordings (#22288)
Browse files Browse the repository at this point in the history
  • Loading branch information
thmsobrmlr committed May 15, 2024
1 parent 9dc3e0e commit fc9aea5
Show file tree
Hide file tree
Showing 8 changed files with 38 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ export function BoldNumber({ showPersonsModal = true }: ChartParams): JSX.Elemen
query: {
kind: NodeKind.InsightActorsQuery,
source: querySource!,
includeRecordings: true,
},
additionalSelect: {
value_at_data_point: 'event_count',
Expand Down
1 change: 1 addition & 0 deletions frontend/src/scenes/insights/views/WorldMap/WorldMap.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ const WorldMapSVG = React.memo(
query: {
kind: NodeKind.InsightActorsQuery,
source: querySource!,
includeRecordings: true,
},
additionalSelect: {
value_at_data_point: 'event_count',
Expand Down
10 changes: 9 additions & 1 deletion frontend/src/scenes/trends/persons-modal/PersonsModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,15 @@ export function PersonsModal({
startExport({
export_format: ExporterFormat.CSV,
export_context: query
? { source: actorsQuery as Record<string, any> }
? {
source: {
...actorsQuery,
select: actorsQuery!.select?.filter(
(c) => c !== 'matched_recordings'
),
source: { ...actorsQuery!.source, includeRecordings: false },
},
}
: { path: originalUrl },
})
}}
Expand Down
1 change: 1 addition & 0 deletions frontend/src/scenes/trends/viz/datasetToActorsQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,6 @@ export function datasetToActorsQuery({ query, dataset, day, index }: DatasetToAc
series: dataset.action?.order ?? 0,
breakdown,
compare,
includeRecordings: true,
}
}
2 changes: 1 addition & 1 deletion mypy-baseline.txt
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ posthog/hogql_queries/insights/trends/trends_query_runner.py:0: error: Signature
posthog/hogql_queries/insights/trends/trends_query_runner.py:0: note: Superclass:
posthog/hogql_queries/insights/trends/trends_query_runner.py:0: note: def to_actors_query(self) -> SelectQuery | SelectUnionQuery
posthog/hogql_queries/insights/trends/trends_query_runner.py:0: note: Subclass:
posthog/hogql_queries/insights/trends/trends_query_runner.py:0: note: def to_actors_query(self, time_frame: str | None, series_index: int, breakdown_value: str | int | None = ..., compare_value: Compare | None = ...) -> SelectQuery | SelectUnionQuery
posthog/hogql_queries/insights/trends/trends_query_runner.py:0: note: def to_actors_query(self, time_frame: str | None, series_index: int, breakdown_value: str | int | None = ..., compare_value: Compare | None = ..., include_recordings: bool | None = ...) -> SelectQuery | SelectUnionQuery
posthog/hogql_queries/insights/trends/trends_query_runner.py:0: error: Statement is unreachable [unreachable]
posthog/hogql_queries/insights/trends/trends_query_runner.py:0: error: Argument 1 to "_event_property" of "TrendsQueryRunner" has incompatible type "str | float | list[str | float] | None"; expected "str" [arg-type]
posthog/hogql_queries/insights/stickiness_query_runner.py:0: error: Module "django.utils.timezone" does not explicitly export attribute "datetime" [attr-defined]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ def to_query(self) -> ast.SelectQuery | ast.SelectUnionQuery:
series_index=query.series or 0,
breakdown_value=query.breakdown,
compare_value=query.compare,
include_recordings=query.includeRecordings,
)
elif isinstance(self.source_runner, FunnelsQueryRunner):
funnels_runner = cast(FunnelsQueryRunner, self.source_runner)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

from posthog.hogql import ast
from posthog.hogql.constants import LimitContext
from posthog.hogql.parser import parse_expr, parse_select
from posthog.hogql.parser import parse_expr
from posthog.hogql.property import action_to_expr, property_to_expr
from posthog.hogql.timings import HogQLTimings
from posthog.hogql_queries.insights.trends.aggregation_operations import AggregationOperations
Expand Down Expand Up @@ -39,6 +39,7 @@ class TrendsActorsQueryBuilder:
time_frame: Optional[datetime]
breakdown_value: Optional[str | int] = None
compare_value: Optional[Compare] = None
include_recordings: Optional[bool] = None

def __init__(
self,
Expand All @@ -50,6 +51,7 @@ def __init__(
time_frame: Optional[str | datetime],
breakdown_value: Optional[str | int] = None,
compare_value: Optional[Compare] = None,
include_recordings: Optional[bool] = None,
limit_context: LimitContext = LimitContext.QUERY,
):
self.trends_query = trends_query
Expand Down Expand Up @@ -78,6 +80,7 @@ def __init__(

self.breakdown_value = breakdown_value
self.compare_value = compare_value
self.include_recordings = include_recordings

@cached_property
def trends_date_range(self) -> QueryDateRange:
Expand Down Expand Up @@ -144,17 +147,14 @@ def is_total_value(self) -> bool:
return self.trends_display.is_total_value()

def build_actors_query(self) -> ast.SelectQuery | ast.SelectUnionQuery:
# TODO: add matching_events only when including recordings
return parse_select(
"""
SELECT
actor_id,
count() as event_count,
groupUniqArray(100)((timestamp, uuid, $session_id, $window_id)) as matching_events
FROM {events_query}
GROUP BY actor_id
""",
placeholders={"events_query": self._get_events_query()},
return ast.SelectQuery(
select=[
ast.Field(chain=["actor_id"]),
ast.Alias(alias="event_count", expr=self._get_actor_value_expr()),
*self._get_matching_recordings_expr(),
],
select_from=ast.JoinExpr(table=self._get_events_query()),
group_by=[ast.Field(chain=["actor_id"])],
)

def _get_events_query(self) -> ast.SelectQuery:
Expand All @@ -163,8 +163,8 @@ def _get_events_query(self) -> ast.SelectQuery:
ast.Alias(alias="actor_id", expr=self._actor_id_expr()),
ast.Field(chain=["e", "timestamp"]),
ast.Field(chain=["e", "uuid"]),
ast.Field(chain=["e", "$session_id"]), # TODO: only when including recordings
ast.Field(chain=["e", "$window_id"]), # TODO: only when including recordings
*([ast.Field(chain=["e", "$session_id"])] if self.include_recordings else []),
*([ast.Field(chain=["e", "$window_id"])] if self.include_recordings else []),
],
select_from=ast.JoinExpr(
table=ast.Field(chain=["events"]),
Expand All @@ -175,6 +175,14 @@ def _get_events_query(self) -> ast.SelectQuery:
)
return query

def _get_actor_value_expr(self) -> ast.Expr:
return parse_expr("count()")

def _get_matching_recordings_expr(self) -> list[ast.Expr]:
if not self.include_recordings:
return []
return [parse_expr("groupUniqArray(100)((timestamp, uuid, $session_id, $window_id)) as matching_events")]

def _actor_id_expr(self) -> ast.Expr:
if self.entity.math == "unique_group" and self.entity.math_group_type_index is not None:
return ast.Field(chain=["e", f"$group_{int(self.entity.math_group_type_index)}"])
Expand Down
2 changes: 2 additions & 0 deletions posthog/hogql_queries/insights/trends/trends_query_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ def to_actors_query(
series_index: int,
breakdown_value: Optional[str | int] = None,
compare_value: Optional[Compare] = None,
include_recordings: Optional[bool] = None,
) -> ast.SelectQuery | ast.SelectUnionQuery:
with self.timings.measure("trends_to_actors_query"):
query_builder = TrendsActorsQueryBuilder(
Expand All @@ -166,6 +167,7 @@ def to_actors_query(
series_index=series_index,
breakdown_value=breakdown_value,
compare_value=compare_value,
include_recordings=include_recordings,
)

query = query_builder.build_actors_query()
Expand Down

0 comments on commit fc9aea5

Please sign in to comment.