From 1bf80d4c38d2a35bdb3959d8801d6b3851552afc Mon Sep 17 00:00:00 2001 From: Michael Matloka Date: Thu, 5 Sep 2024 17:49:41 +0200 Subject: [PATCH] fix(insights): Clear "data warehouse not supported in funnels" error (#24809) Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> --- posthog/api/test/test_query.py | 35 +++++++++++++++++++ .../hogql_queries/insights/funnels/base.py | 8 +++-- .../insights/funnels/funnel_event_query.py | 10 +++++- .../hogql_queries/insights/utils/entities.py | 17 ++++++--- 4 files changed, 63 insertions(+), 7 deletions(-) diff --git a/posthog/api/test/test_query.py b/posthog/api/test/test_query.py index 78339bd3f30c2..e5a85099efd08 100644 --- a/posthog/api/test/test_query.py +++ b/posthog/api/test/test_query.py @@ -11,8 +11,10 @@ from posthog.models.utils import UUIDT from posthog.schema import ( CachedEventsQueryResponse, + DataWarehouseNode, EventPropertyFilter, EventsQuery, + FunnelsQuery, HogQLPropertyFilter, HogQLQuery, PersonPropertyFilter, @@ -731,6 +733,39 @@ def test_invalid_query_kind(self): api_response.content, ) + def test_funnel_query_with_data_warehouse_node_temporarily_raises(self): + # As of September 2024, funnels don't support data warehouse tables YET, so we want a helpful error message + api_response = self.client.post( + f"/api/projects/{self.team.id}/query/", + { + "query": FunnelsQuery( + series=[ + DataWarehouseNode( + id="xyz", + table_name="xyz", + id_field="id", + distinct_id_field="customer_email", + timestamp_field="created", + ), + DataWarehouseNode( + id="abc", + table_name="abc", + id_field="id", + distinct_id_field="customer_email", + timestamp_field="timestamp", + ), + ], + ).model_dump() + }, + ) + self.assertEqual(api_response.status_code, 400) + self.assertDictEqual( + api_response.json(), + self.validation_error_response( + "Data warehouse tables are not supported in funnels just yet. For now, please try this funnel without the data warehouse-based step." + ), + ) + def test_missing_query(self): api_response = self.client.post(f"/api/projects/{self.team.id}/query/", {"query": {}}) self.assertEqual(api_response.status_code, 400) diff --git a/posthog/hogql_queries/insights/funnels/base.py b/posthog/hogql_queries/insights/funnels/base.py index 477c205dd968c..84ddf66eafccd 100644 --- a/posthog/hogql_queries/insights/funnels/base.py +++ b/posthog/hogql_queries/insights/funnels/base.py @@ -299,7 +299,9 @@ def _serialize_step( action_id = step.event type = "events" elif isinstance(step, DataWarehouseNode): - raise NotImplementedError("DataWarehouseNode is not supported in funnels") + raise ValidationError( + "Data warehouse tables are not supported in funnels just yet. For now, please try this funnel without the data warehouse-based step." + ) else: action = Action.objects.get(pk=step.id) name = action.name @@ -584,7 +586,9 @@ def _build_step_query( action = Action.objects.get(pk=int(entity.id), team=self.context.team) event_expr = action_to_expr(action) elif isinstance(entity, DataWarehouseNode): - raise NotImplementedError("DataWarehouseNode is not supported in funnels") + raise ValidationError( + "Data warehouse tables are not supported in funnels just yet. For now, please try this funnel without the data warehouse-based step." + ) elif entity.event is None: # all events event_expr = ast.Constant(value=1) diff --git a/posthog/hogql_queries/insights/funnels/funnel_event_query.py b/posthog/hogql_queries/insights/funnels/funnel_event_query.py index 8acb0f7dea87b..c4cb9507534ef 100644 --- a/posthog/hogql_queries/insights/funnels/funnel_event_query.py +++ b/posthog/hogql_queries/insights/funnels/funnel_event_query.py @@ -7,7 +7,13 @@ from posthog.hogql_queries.utils.query_date_range import QueryDateRange from posthog.models.action.action import Action from posthog.models.property.property import PropertyName -from posthog.schema import ActionsNode, EventsNode, FunnelExclusionActionsNode, FunnelExclusionEventsNode +from posthog.schema import ( + ActionsNode, + DataWarehouseNode, + EventsNode, + FunnelExclusionActionsNode, + FunnelExclusionEventsNode, +) from rest_framework.exceptions import ValidationError @@ -143,6 +149,8 @@ def _entity_expr(self, skip_entity_filter: bool) -> ast.Expr | None: events.update(action.get_step_events()) except Action.DoesNotExist: raise ValidationError(f"Action ID {node.id} does not exist!") + elif isinstance(node, DataWarehouseNode): + continue # Data warehouse nodes aren't based on events else: raise ValidationError("Series and exclusions must be compose of action and event nodes") diff --git a/posthog/hogql_queries/insights/utils/entities.py b/posthog/hogql_queries/insights/utils/entities.py index 794ce6170da11..b14653b338035 100644 --- a/posthog/hogql_queries/insights/utils/entities.py +++ b/posthog/hogql_queries/insights/utils/entities.py @@ -1,6 +1,7 @@ from posthog.schema import ( ActionsNode, CohortPropertyFilter, + DataWarehouseNode, EmptyPropertyFilter, EventsNode, FunnelExclusionActionsNode, @@ -9,16 +10,16 @@ ) from posthog.types import AnyPropertyFilter, EntityNode, ExclusionEntityNode from collections import Counter -from rest_framework.exceptions import ValidationError def is_equal_type(a: EntityNode, b: EntityNode | ExclusionEntityNode) -> bool: if isinstance(a, EventsNode): return isinstance(b, EventsNode) or isinstance(b, FunnelExclusionEventsNode) - elif isinstance(a, ActionsNode): + if isinstance(a, ActionsNode): return isinstance(b, ActionsNode) or isinstance(b, FunnelExclusionActionsNode) - else: - raise ValidationError(detail=f"Type comparision for {type(a)} and {type(b)} not implemented.") + if isinstance(a, DataWarehouseNode): + return isinstance(b, DataWarehouseNode) + raise ValueError(detail=f"Type comparison for {type(a)} and {type(b)} not implemented.") def is_equal(a: EntityNode, b: EntityNode | ExclusionEntityNode, compare_properties=True) -> bool: @@ -44,6 +45,14 @@ def is_equal(a: EntityNode, b: EntityNode | ExclusionEntityNode, compare_propert ): return False + # different data source + if ( + isinstance(a, DataWarehouseNode) + and isinstance(b, DataWarehouseNode) + and (a.id != b.id or a.id_field != b.id_field) + ): + return False + # different properties if compare_properties and _sorted_property_reprs(a.properties) != _sorted_property_reprs(b.properties): return False