From 3b34206e2a80b0a630f06d6adc75f95e4bbd9feb Mon Sep 17 00:00:00 2001 From: Marius Andra Date: Tue, 19 Mar 2024 15:37:14 +0100 Subject: [PATCH 1/5] fix "PropertyDefinition matching query does not exist." (7 cases) --- .../trends/test/test_trends_query_runner.py | 32 +++++++++++++++++++ .../insights/trends/trends_query_runner.py | 17 ++++++---- 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/posthog/hogql_queries/insights/trends/test/test_trends_query_runner.py b/posthog/hogql_queries/insights/trends/test/test_trends_query_runner.py index 104e232a01406..8e93f7a49d83c 100644 --- a/posthog/hogql_queries/insights/trends/test/test_trends_query_runner.py +++ b/posthog/hogql_queries/insights/trends/test/test_trends_query_runner.py @@ -1114,6 +1114,38 @@ def test_breakdown_values_limit(self): ) self.assertEqual(len(response.results), 11) + def test_breakdown_values_unknown_property(self): + # same as above test, just without creating the property definition + for value in list(range(30)): + _create_event( + team=self.team, + event="$pageview", + distinct_id=f"person_{value}", + timestamp="2020-01-11T12:00:00Z", + properties={"breakdown_value": f"{value}"}, + ) + + response = self._run_trends_query( + "2020-01-09", + "2020-01-20", + IntervalType.day, + [EventsNode(event="$pageview")], + TrendsFilter(display=ChartDisplayType.ActionsLineGraph), + BreakdownFilter(breakdown="breakdown_value", breakdown_type=BreakdownType.event), + ) + + self.assertEqual(len(response.results), 26) + + response = self._run_trends_query( + "2020-01-09", + "2020-01-20", + IntervalType.day, + [EventsNode(event="$pageview")], + TrendsFilter(display=ChartDisplayType.ActionsLineGraph), + BreakdownFilter(breakdown="breakdown_value", breakdown_type=BreakdownType.event, breakdown_limit=10), + ) + self.assertEqual(len(response.results), 11) + def test_breakdown_values_world_map_limit(self): PropertyDefinition.objects.create(team=self.team, name="breakdown_value", property_type="String") diff --git a/posthog/hogql_queries/insights/trends/trends_query_runner.py b/posthog/hogql_queries/insights/trends/trends_query_runner.py index 6cf84dcb5357b..c75970d401d2d 100644 --- a/posthog/hogql_queries/insights/trends/trends_query_runner.py +++ b/posthog/hogql_queries/insights/trends/trends_query_runner.py @@ -719,13 +719,16 @@ def _event_property( field: str, field_type: PropertyDefinition.Type, group_type_index: Optional[int], - ): - return PropertyDefinition.objects.get( - name=field, - team=self.team, - type=field_type, - group_type_index=group_type_index if field_type == PropertyDefinition.Type.GROUP else None, - ).property_type + ) -> str: + try: + return PropertyDefinition.objects.get( + name=field, + team=self.team, + type=field_type, + group_type_index=group_type_index if field_type == PropertyDefinition.Type.GROUP else None, + ).property_type + except PropertyDefinition.DoesNotExist: + return "String" # TODO: Move this to posthog/hogql_queries/legacy_compatibility/query_to_filter.py def _query_to_filter(self) -> Dict[str, Any]: From 12ae985e0175ab800f29f3295df31b38c6288f1b Mon Sep 17 00:00:00 2001 From: Marius Andra Date: Tue, 19 Mar 2024 16:26:01 +0100 Subject: [PATCH 2/5] fix --- .../insights/trends/trends_query_runner.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/posthog/hogql_queries/insights/trends/trends_query_runner.py b/posthog/hogql_queries/insights/trends/trends_query_runner.py index c75970d401d2d..ba5cc3acf4d53 100644 --- a/posthog/hogql_queries/insights/trends/trends_query_runner.py +++ b/posthog/hogql_queries/insights/trends/trends_query_runner.py @@ -721,12 +721,15 @@ def _event_property( group_type_index: Optional[int], ) -> str: try: - return PropertyDefinition.objects.get( - name=field, - team=self.team, - type=field_type, - group_type_index=group_type_index if field_type == PropertyDefinition.Type.GROUP else None, - ).property_type + return ( + PropertyDefinition.objects.get( + name=field, + team=self.team, + type=field_type, + group_type_index=group_type_index if field_type == PropertyDefinition.Type.GROUP else None, + ).property_type + or "String" + ) except PropertyDefinition.DoesNotExist: return "String" From a958dfc15a94e3301af2dcbcbc61cc6e4c95e9ee Mon Sep 17 00:00:00 2001 From: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 19 Mar 2024 15:48:16 +0000 Subject: [PATCH 3/5] Update query snapshots --- .../test/__snapshots__/test_session_recordings.ambr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/posthog/session_recordings/test/__snapshots__/test_session_recordings.ambr b/posthog/session_recordings/test/__snapshots__/test_session_recordings.ambr index fad3c08168d0b..c2982e4f8526f 100644 --- a/posthog/session_recordings/test/__snapshots__/test_session_recordings.ambr +++ b/posthog/session_recordings/test/__snapshots__/test_session_recordings.ambr @@ -778,7 +778,7 @@ FROM "posthog_persondistinctid" INNER JOIN "posthog_person" ON ("posthog_persondistinctid"."person_id" = "posthog_person"."id") WHERE ("posthog_persondistinctid"."distinct_id" IN ('user2', - 'user_one_2') + 'user_one_0') AND "posthog_persondistinctid"."team_id" = 2) /*controller='project_session_recordings-list',route='api/projects/%28%3FP%3Cparent_lookup_team_id%3E%5B%5E/.%5D%2B%29/session_recordings/%3F%24'*/ ''' # --- From 3cf7c7dac29782534b9d12a08e813f1a56c31ef5 Mon Sep 17 00:00:00 2001 From: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 19 Mar 2024 16:07:33 +0000 Subject: [PATCH 4/5] Update query snapshots --- .../test/__snapshots__/test_session_recordings.ambr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/posthog/session_recordings/test/__snapshots__/test_session_recordings.ambr b/posthog/session_recordings/test/__snapshots__/test_session_recordings.ambr index c2982e4f8526f..fad3c08168d0b 100644 --- a/posthog/session_recordings/test/__snapshots__/test_session_recordings.ambr +++ b/posthog/session_recordings/test/__snapshots__/test_session_recordings.ambr @@ -778,7 +778,7 @@ FROM "posthog_persondistinctid" INNER JOIN "posthog_person" ON ("posthog_persondistinctid"."person_id" = "posthog_person"."id") WHERE ("posthog_persondistinctid"."distinct_id" IN ('user2', - 'user_one_0') + 'user_one_2') AND "posthog_persondistinctid"."team_id" = 2) /*controller='project_session_recordings-list',route='api/projects/%28%3FP%3Cparent_lookup_team_id%3E%5B%5E/.%5D%2B%29/session_recordings/%3F%24'*/ ''' # --- From 2219982f5431075dbcf881375e56e6e47d2042c7 Mon Sep 17 00:00:00 2001 From: Marius Andra Date: Tue, 19 Mar 2024 22:09:15 +0100 Subject: [PATCH 5/5] fix another case --- .../trends/test/test_trends_query_runner.py | 13 +++++++ .../insights/trends/trends_query_runner.py | 36 ++++++++++--------- 2 files changed, 32 insertions(+), 17 deletions(-) diff --git a/posthog/hogql_queries/insights/trends/test/test_trends_query_runner.py b/posthog/hogql_queries/insights/trends/test/test_trends_query_runner.py index 8e93f7a49d83c..09a7389e74c7b 100644 --- a/posthog/hogql_queries/insights/trends/test/test_trends_query_runner.py +++ b/posthog/hogql_queries/insights/trends/test/test_trends_query_runner.py @@ -364,6 +364,19 @@ def test_trends_query_formula(self): self.assertEqual("Formula (A+B)", response.results[0]["label"]) self.assertEqual([1, 0, 2, 4, 4, 0, 2, 1, 1, 0, 1], response.results[0]["data"]) + def test_trends_query_formula_breakdown_no_data(self): + self._create_test_events() + + response = self._run_trends_query( + self.default_date_from, + self.default_date_to, + IntervalType.day, + [EventsNode(event="$pageviewxxx"), EventsNode(event="$pageleavexxx")], + TrendsFilter(formula="A+B"), + BreakdownFilter(breakdown_type=BreakdownType.person, breakdown="$browser"), + ) + self.assertEqual([], response.results) + def test_trends_query_formula_aggregate(self): self._create_test_events() diff --git a/posthog/hogql_queries/insights/trends/trends_query_runner.py b/posthog/hogql_queries/insights/trends/trends_query_runner.py index ba5cc3acf4d53..608e03aa72d3f 100644 --- a/posthog/hogql_queries/insights/trends/trends_query_runner.py +++ b/posthog/hogql_queries/insights/trends/trends_query_runner.py @@ -665,25 +665,27 @@ def apply_formula(self, formula: str, results: List[Dict[str, Any]]) -> List[Dic res.append(new_result) return res - if self._trends_display.should_aggregate_values(): - series_data = list(map(lambda s: [s["aggregated_value"]], results)) - new_series_data = FormulaAST(series_data).call(formula) - - new_result = results[0] - new_result["aggregated_value"] = float(sum(new_series_data)) - new_result["data"] = None - new_result["count"] = 0 - new_result["label"] = f"Formula ({formula})" - else: - series_data = list(map(lambda s: s["data"], results)) - new_series_data = FormulaAST(series_data).call(formula) + if len(results) > 0: + if self._trends_display.should_aggregate_values(): + series_data = list(map(lambda s: [s["aggregated_value"]], results)) + new_series_data = FormulaAST(series_data).call(formula) + + new_result = results[0] + new_result["aggregated_value"] = float(sum(new_series_data)) + new_result["data"] = None + new_result["count"] = 0 + new_result["label"] = f"Formula ({formula})" + else: + series_data = list(map(lambda s: s["data"], results)) + new_series_data = FormulaAST(series_data).call(formula) - new_result = results[0] - new_result["data"] = new_series_data - new_result["count"] = float(sum(new_series_data)) - new_result["label"] = f"Formula ({formula})" + new_result = results[0] + new_result["data"] = new_series_data + new_result["count"] = float(sum(new_series_data)) + new_result["label"] = f"Formula ({formula})" - return [new_result] + return [new_result] + return [] def _is_breakdown_field_boolean(self): if not self.query.breakdownFilter or not self.query.breakdownFilter.breakdown_type: