feat(insights): HogQL calculation of saved legacy insights v4 (#21781)

* Revert "revert(insights): HogQL calculation of saved legacy insights v3 (#21778)" This reverts commit c0be1d1. * Move HogQL in insight serialization to its own flag
PostHog · Apr 24, 2024 · d7d03b5 · d7d03b5
1 parent 6e653e8
commit d7d03b5
Show file tree

Hide file tree

Showing 31 changed files with 556 additions and 333 deletions.
diff --git a/bin/build-schema.mjs → bin/build-schema-json.mjs b/bin/build-schema.mjs → bin/build-schema-json.mjs
diff --git a/bin/build-schema-python.sh b/bin/build-schema-python.sh
@@ -0,0 +1,28 @@
+#!/usr/bin/env bash
+
+set -e
+
+# Generate schema.py from schema.json
+datamodel-codegen \
+    --class-name='SchemaRoot' --collapse-root-models --target-python-version 3.10 --disable-timestamp \
+    --use-one-literal-as-default --use-default --use-default-kwarg --use-subclass-enum \
+    --input frontend/src/queries/schema.json --input-file-type jsonschema \
+    --output posthog/schema.py --output-model-type pydantic_v2.BaseModel
+
+# Format schema.py
+ruff format posthog/schema.py
+
+# Check schema.py and autofix
+ruff check --fix posthog/schema.py
+
+# HACK: Datamodel-codegen output for enum-type fields with a default is invalid – the default value is a plain string,
+# and not the expected enum member. We fix this using sed, which is pretty hacky, but does the job.
+# Specifically, we need to replace `Optional[PropertyOperator] = "exact"`
+# with `Optional[PropertyOperator] = PropertyOperator("exact")` to make the default value valid.
+# Remove this when https://github.com/koxudaxi/datamodel-code-generator/issues/1929 is resolved.
+if [[ "$OSTYPE" == "darwin"* ]]; then
+    # sed needs `-i` to be followed by `''` on macOS
+    sed -i '' -e 's/Optional\[PropertyOperator\] = \("[A-Za-z_]*"\)/Optional[PropertyOperator] = PropertyOperator(\1)/g' posthog/schema.py
+else
+    sed -i -e 's/Optional\[PropertyOperator\] = \("[A-Za-z_]*"\)/Optional[PropertyOperator] = PropertyOperator(\1)/g' posthog/schema.py
+fi
diff --git a/frontend/src/lib/constants.tsx b/frontend/src/lib/constants.tsx
@@ -177,6 +177,7 @@ export const FEATURE_FLAGS = {
     HOGQL_INSIGHTS_STICKINESS: 'hogql-insights-stickiness', // owner: @Gilbert09
     HOGQL_INSIGHTS_FUNNELS: 'hogql-insights-funnels', // owner: @thmsobrmlr
     HOGQL_INSIGHT_LIVE_COMPARE: 'hogql-insight-live-compare', // owner: @mariusandra
+    HOGQL_IN_INSIGHT_SERIALIZATION: 'hogql-in-insight-serialization', // owner: @Twixes
     BI_VIZ: 'bi_viz', // owner: @Gilbert09
     WEBHOOKS_DENYLIST: 'webhooks-denylist', // owner: #team-pipeline
     PERSONS_HOGQL_QUERY: 'persons-hogql-query', // owner: @mariusandra

diff --git a/frontend/src/queries/schema.json b/frontend/src/queries/schema.json
@@ -1200,7 +1200,8 @@
                     "type": "string"
                 },
                 "operator": {
-                    "$ref": "#/definitions/PropertyOperator"
+                    "$ref": "#/definitions/PropertyOperator",
+                    "default": "exact"
                 },
                 "type": {
                     "const": "event",

diff --git a/frontend/src/queries/schema.ts b/frontend/src/queries/schema.ts
@@ -66,7 +66,6 @@ export enum NodeKind {
     SavedInsightNode = 'SavedInsightNode',
     InsightVizNode = 'InsightVizNode',
 
-    // New queries, not yet implemented
     TrendsQuery = 'TrendsQuery',
     FunnelsQuery = 'FunnelsQuery',
     RetentionQuery = 'RetentionQuery',

diff --git a/frontend/src/types.ts b/frontend/src/types.ts
@@ -666,6 +666,7 @@ interface BasePropertyFilter {
 /** Sync with plugin-server/src/types.ts */
 export interface EventPropertyFilter extends BasePropertyFilter {
     type: PropertyFilterType.Event
+    /** @default 'exact' */
     operator: PropertyOperator
 }
 

diff --git a/mypy-baseline.txt b/mypy-baseline.txt
@@ -108,7 +108,6 @@ posthog/hogql_queries/legacy_compatibility/filter_to_query.py:0: error: Argument
 posthog/hogql_queries/legacy_compatibility/filter_to_query.py:0: error: Dict entry 0 has incompatible type "str": "PathsFilter"; expected "str": "TrendsFilter"  [dict-item]
 posthog/hogql_queries/legacy_compatibility/filter_to_query.py:0: error: Dict entry 0 has incompatible type "str": "LifecycleFilter"; expected "str": "TrendsFilter"  [dict-item]
 posthog/hogql_queries/legacy_compatibility/filter_to_query.py:0: error: Dict entry 0 has incompatible type "str": "StickinessFilter"; expected "str": "TrendsFilter"  [dict-item]
-posthog/hogql_queries/legacy_compatibility/feature_flag.py:0: error: Item "AnonymousUser" of "User | AnonymousUser" has no attribute "email"  [union-attr]
 posthog/api/utils.py:0: error: Incompatible types in assignment (expression has type "type[EventDefinition]", variable has type "type[EnterpriseEventDefinition]")  [assignment]
 posthog/api/utils.py:0: error: Argument 1 to "UUID" has incompatible type "int | str"; expected "str | None"  [arg-type]
 ee/billing/quota_limiting.py:0: error: List comprehension has incompatible type List[int]; expected List[str]  [misc]
@@ -301,7 +300,6 @@ posthog/queries/breakdown_props.py:0: error: Argument 1 to "translate_hogql" has
 posthog/queries/funnels/base.py:0: error: "HogQLContext" has no attribute "person_on_events_mode"  [attr-defined]
 posthog/queries/funnels/base.py:0: error: Argument 1 to "translate_hogql" has incompatible type "str | int"; expected "str"  [arg-type]
 ee/clickhouse/queries/funnels/funnel_correlation.py:0: error: Statement is unreachable  [unreachable]
-posthog/caching/calculate_results.py:0: error: Argument 3 to "process_query" has incompatible type "bool"; expected "LimitContext | None"  [arg-type]
 posthog/api/person.py:0: error: Argument 1 to <tuple> has incompatible type "*tuple[str, ...]"; expected "type[BaseRenderer]"  [arg-type]
 posthog/api/person.py:0: error: Argument 1 to "loads" has incompatible type "str | None"; expected "str | bytes | bytearray"  [arg-type]
 posthog/api/person.py:0: error: Argument "user" to "log_activity" has incompatible type "User | AnonymousUser"; expected "User | None"  [arg-type]
@@ -343,11 +341,6 @@ posthog/hogql_queries/insights/lifecycle_query_runner.py:0: note: Consider using
 posthog/hogql_queries/insights/lifecycle_query_runner.py:0: error: Argument 1 to "sorted" has incompatible type "list[Any] | None"; expected "Iterable[Any]"  [arg-type]
 posthog/hogql_queries/insights/lifecycle_query_runner.py:0: error: Item "SelectUnionQuery" of "SelectQuery | SelectUnionQuery" has no attribute "select_from"  [union-attr]
 posthog/hogql_queries/insights/lifecycle_query_runner.py:0: error: Item "None" of "JoinExpr | Any | None" has no attribute "sample"  [union-attr]
-posthog/hogql_queries/legacy_compatibility/process_insight.py:0: error: Incompatible types in assignment (expression has type "PathFilter", variable has type "RetentionFilter")  [assignment]
-posthog/hogql_queries/legacy_compatibility/process_insight.py:0: error: Incompatible types in assignment (expression has type "StickinessFilter", variable has type "RetentionFilter")  [assignment]
-posthog/hogql_queries/legacy_compatibility/process_insight.py:0: error: Incompatible types in assignment (expression has type "Filter", variable has type "RetentionFilter")  [assignment]
-posthog/api/insight.py:0: error: Argument 1 to "is_insight_with_hogql_support" has incompatible type "Insight | DashboardTile"; expected "Insight"  [arg-type]
-posthog/api/insight.py:0: error: Argument 1 to "process_insight" has incompatible type "Insight | DashboardTile"; expected "Insight"  [arg-type]
 posthog/api/insight.py:0: error: Argument 1 to <tuple> has incompatible type "*tuple[str, ...]"; expected "type[BaseRenderer]"  [arg-type]
 posthog/api/dashboards/dashboard.py:0: error: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases  [misc]
 posthog/api/feature_flag.py:0: error: Metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases  [misc]
@@ -369,19 +362,6 @@ posthog/tasks/exports/test/test_export_utils.py:0: error: Function is missing a
 posthog/tasks/exports/test/test_csv_exporter_url_sanitising.py:0: error: Function is missing a type annotation for one or more arguments  [no-untyped-def]
 posthog/tasks/exports/test/test_csv_exporter_url_sanitising.py:0: error: Function is missing a type annotation for one or more arguments  [no-untyped-def]
 posthog/tasks/exports/test/test_csv_exporter_renders.py:0: error: Function is missing a type annotation  [no-untyped-def]
-posthog/tasks/exports/test/test_csv_exporter.py:0: error: Function is missing a return type annotation  [no-untyped-def]
-posthog/tasks/exports/test/test_csv_exporter.py:0: error: Function is missing a type annotation  [no-untyped-def]
-posthog/tasks/exports/test/test_csv_exporter.py:0: error: Function is missing a type annotation for one or more arguments  [no-untyped-def]
-posthog/tasks/exports/test/test_csv_exporter.py:0: error: Function is missing a type annotation for one or more arguments  [no-untyped-def]
-posthog/tasks/exports/test/test_csv_exporter.py:0: error: Function is missing a type annotation for one or more arguments  [no-untyped-def]
-posthog/tasks/exports/test/test_csv_exporter.py:0: error: Function is missing a type annotation for one or more arguments  [no-untyped-def]
-posthog/tasks/exports/test/test_csv_exporter.py:0: error: Function is missing a type annotation for one or more arguments  [no-untyped-def]
-posthog/tasks/exports/test/test_csv_exporter.py:0: error: Function is missing a type annotation for one or more arguments  [no-untyped-def]
-posthog/tasks/exports/test/test_csv_exporter.py:0: error: Function is missing a type annotation for one or more arguments  [no-untyped-def]
-posthog/tasks/exports/test/test_csv_exporter.py:0: error: Function is missing a type annotation for one or more arguments  [no-untyped-def]
-posthog/tasks/exports/test/test_csv_exporter.py:0: error: Function is missing a type annotation for one or more arguments  [no-untyped-def]
-posthog/tasks/exports/test/test_csv_exporter.py:0: error: Function is missing a type annotation for one or more arguments  [no-untyped-def]
-posthog/tasks/exports/test/test_csv_exporter.py:0: error: Function is missing a type annotation for one or more arguments  [no-untyped-def]
 posthog/session_recordings/queries/session_recording_list_from_replay_summary.py:0: error: Missing return statement  [empty-body]
 posthog/session_recordings/queries/session_recording_list_from_replay_summary.py:0: note: If the method is meant to be abstract, use @abc.abstractmethod
 posthog/session_recordings/queries/session_recording_list_from_replay_summary.py:0: error: Missing return statement  [empty-body]
@@ -531,6 +511,19 @@ posthog/temporal/data_imports/pipelines/zendesk/helpers.py:0: error: Argument 1
 posthog/temporal/data_imports/pipelines/zendesk/helpers.py:0: error: Argument 1 to "ensure_pendulum_datetime" has incompatible type "DateTime | Date | datetime | date | str | float | int | None"; expected "DateTime | Date | datetime | date | str | float | int"  [arg-type]
 posthog/temporal/data_imports/pipelines/zendesk/helpers.py:0: error: Item "None" of "DateTime | None" has no attribute "int_timestamp"  [union-attr]
 posthog/temporal/data_imports/pipelines/zendesk/helpers.py:0: error: Argument 1 to "ensure_pendulum_datetime" has incompatible type "str | None"; expected "DateTime | Date | datetime | date | str | float | int"  [arg-type]
+posthog/tasks/exports/test/test_csv_exporter.py:0: error: Function is missing a return type annotation  [no-untyped-def]
+posthog/tasks/exports/test/test_csv_exporter.py:0: error: Function is missing a type annotation  [no-untyped-def]
+posthog/tasks/exports/test/test_csv_exporter.py:0: error: Function is missing a type annotation for one or more arguments  [no-untyped-def]
+posthog/tasks/exports/test/test_csv_exporter.py:0: error: Function is missing a type annotation for one or more arguments  [no-untyped-def]
+posthog/tasks/exports/test/test_csv_exporter.py:0: error: Function is missing a type annotation for one or more arguments  [no-untyped-def]
+posthog/tasks/exports/test/test_csv_exporter.py:0: error: Function is missing a type annotation for one or more arguments  [no-untyped-def]
+posthog/tasks/exports/test/test_csv_exporter.py:0: error: Function is missing a type annotation for one or more arguments  [no-untyped-def]
+posthog/tasks/exports/test/test_csv_exporter.py:0: error: Function is missing a type annotation for one or more arguments  [no-untyped-def]
+posthog/tasks/exports/test/test_csv_exporter.py:0: error: Function is missing a type annotation for one or more arguments  [no-untyped-def]
+posthog/tasks/exports/test/test_csv_exporter.py:0: error: Function is missing a type annotation for one or more arguments  [no-untyped-def]
+posthog/tasks/exports/test/test_csv_exporter.py:0: error: Function is missing a type annotation for one or more arguments  [no-untyped-def]
+posthog/tasks/exports/test/test_csv_exporter.py:0: error: Function is missing a type annotation for one or more arguments  [no-untyped-def]
+posthog/tasks/exports/test/test_csv_exporter.py:0: error: Function is missing a type annotation for one or more arguments  [no-untyped-def]
 posthog/queries/trends/test/test_person.py:0: error: "str" has no attribute "get"  [attr-defined]
 posthog/queries/trends/test/test_person.py:0: error: Invalid index type "int" for "HttpResponse"; expected type "str | bytes"  [index]
 posthog/queries/trends/test/test_person.py:0: error: "str" has no attribute "get"  [attr-defined]

diff --git a/package.json b/package.json
@@ -33,8 +33,8 @@
         "build": "pnpm copy-scripts && pnpm build:esbuild",
         "build:esbuild": "node frontend/build.mjs",
         "schema:build": "pnpm run schema:build:json && pnpm run schema:build:python",
-        "schema:build:json": "ts-node bin/build-schema.mjs && prettier --write frontend/src/queries/schema.json",
-        "schema:build:python": "datamodel-codegen --class-name='SchemaRoot' --collapse-root-models --target-python-version 3.10 --disable-timestamp --use-one-literal-as-default --use-default --use-default-kwarg --use-subclass-enum --input frontend/src/queries/schema.json --input-file-type jsonschema --output posthog/schema.py --output-model-type pydantic_v2.BaseModel && ruff format posthog/schema.py && ruff check --fix posthog/schema.py",
+        "schema:build:json": "ts-node bin/build-schema-json.mjs && prettier --write frontend/src/queries/schema.json",
+        "schema:build:python": "bash bin/build-schema-python.sh",
         "grammar:build": "npm run grammar:build:python && npm run grammar:build:cpp",
         "grammar:build:python": "cd posthog/hogql/grammar && antlr -Dlanguage=Python3 HogQLLexer.g4 && antlr -visitor -no-listener -Dlanguage=Python3 HogQLParser.g4",
         "grammar:build:cpp": "cd posthog/hogql/grammar && antlr -o ../../../hogql_parser -Dlanguage=Cpp HogQLLexer.g4 && antlr -o ../../../hogql_parser -visitor -no-listener -Dlanguage=Cpp HogQLParser.g4",

diff --git a/posthog/api/insight.py b/posthog/api/insight.py
@@ -35,11 +35,7 @@
 from posthog.api.tagged_item import TaggedItemSerializerMixin, TaggedItemViewSetMixin
 from posthog.api.utils import format_paginated_url
 from posthog.auth import SharingAccessTokenAuthentication
-from posthog.caching.fetch_from_cache import (
-    InsightResult,
-    fetch_cached_insight_result,
-    synchronously_update_cache,
-)
+from posthog.caching.fetch_from_cache import InsightResult, fetch_cached_insight_result, synchronously_update_cache
 from posthog.caching.insights_api import should_refresh_insight
 from posthog.constants import (
     INSIGHT,
@@ -58,8 +54,8 @@
 from posthog.hogql.errors import ExposedHogQLError
 from posthog.hogql.timings import HogQLTimings
 from posthog.hogql_queries.apply_dashboard_filters import DATA_TABLE_LIKE_NODE_KINDS
-from posthog.hogql_queries.legacy_compatibility.feature_flag import hogql_insights_enabled
-from posthog.hogql_queries.legacy_compatibility.process_insight import is_insight_with_hogql_support, process_insight
+from posthog.hogql_queries.legacy_compatibility.feature_flag import should_use_hogql_backend_in_insight_serialization
+from posthog.hogql_queries.legacy_compatibility.filter_to_query import filter_to_query
 from posthog.kafka_client.topics import KAFKA_METRICS_TIME_TO_SEE_DATA
 from posthog.models import DashboardTile, Filter, Insight, User
 from posthog.models.activity_logging.activity_log import (
@@ -510,7 +506,7 @@ def to_representation(self, instance: Insight):
 
         dashboard: Optional[Dashboard] = self.context.get("dashboard")
         representation["filters"] = instance.dashboard_filters(dashboard=dashboard)
-        representation["query"] = instance.dashboard_query(dashboard=dashboard)
+        representation["query"] = instance.get_effective_query(dashboard=dashboard)
 
         if "insight" not in representation["filters"] and not representation["query"]:
             representation["filters"]["insight"] = "TRENDS"
@@ -521,14 +517,34 @@ def to_representation(self, instance: Insight):
 
     @lru_cache(maxsize=1)
     def insight_result(self, insight: Insight) -> InsightResult:
+        from posthog.caching.calculate_results import calculate_for_query_based_insight
+
         dashboard = self.context.get("dashboard", None)
         dashboard_tile = self.dashboard_tile_from_context(insight, dashboard)
-        target = insight if dashboard is None else dashboard_tile
 
-        if hogql_insights_enabled(self.context.get("request", None).user) and is_insight_with_hogql_support(
-            target or insight
+        if insight.query:
+            try:
+                return calculate_for_query_based_insight(
+                    insight, dashboard=dashboard, refresh_requested=refresh_requested_by_client(self.context["request"])
+                )
+            except ExposedHogQLError as e:
+                raise ValidationError(str(e))
+
+        if not self.context["request"].user.is_anonymous and should_use_hogql_backend_in_insight_serialization(
+            self.context["request"].user
         ):
-            return process_insight(target or insight, insight.team)
+            # TRICKY: As running `filters`-based insights on the HogQL-based engine is a transitional mechanism,
+            # we fake the insight being properly `query`-based.
+            # To prevent the lie from accidentally being saved to Postgres, we roll it back in the `finally` branch.
+            insight.query = filter_to_query(insight.filters).model_dump()
+            try:
+                return calculate_for_query_based_insight(
+                    insight, dashboard=dashboard, refresh_requested=refresh_requested_by_client(self.context["request"])
+                )
+            except ExposedHogQLError as e:
+                raise ValidationError(str(e))
+            finally:
+                insight.query = None
 
         is_shared = self.context.get("is_shared", False)
         refresh_insight_now, refresh_frequency = should_refresh_insight(
@@ -539,10 +555,9 @@ def insight_result(self, insight: Insight) -> InsightResult:
         )
         if refresh_insight_now:
             INSIGHT_REFRESH_INITIATED_COUNTER.labels(is_shared=is_shared).inc()
-            return synchronously_update_cache(insight, dashboard, refresh_frequency)
+            return synchronously_update_cache(insight, dashboard, refresh_frequency=refresh_frequency)
 
-        # :TODO: Clear up if tile can be null or not
-        return fetch_cached_insight_result(target or insight, refresh_frequency)
+        return fetch_cached_insight_result(dashboard_tile or insight, refresh_frequency)
 
     @lru_cache(maxsize=1)  # each serializer instance should only deal with one insight/tile combo
     def dashboard_tile_from_context(self, insight: Insight, dashboard: Optional[Dashboard]) -> Optional[DashboardTile]:

diff --git a/posthog/api/query.py b/posthog/api/query.py
@@ -3,6 +3,7 @@
 
 from django.http import JsonResponse
 from drf_spectacular.utils import OpenApiResponse
+from posthog.hogql_queries.query_runner import ExecutionMode
 from rest_framework import viewsets
 from rest_framework.decorators import action
 from rest_framework.exceptions import ValidationError, NotAuthenticated
@@ -75,7 +76,13 @@ def create(self, request, *args, **kwargs) -> Response:
 
         tag_queries(query=request.data["query"])
         try:
-            result = process_query_model(self.team, data.query, refresh_requested=data.refresh)
+            result = process_query_model(
+                self.team,
+                data.query,
+                execution_mode=ExecutionMode.CALCULATION_ALWAYS
+                if data.refresh
+                else ExecutionMode.RECENT_CACHE_CALCULATE_IF_STALE,
+            )
             return Response(result)
         except (ExposedHogQLError, ExposedCHQueryError) as e:
             raise ValidationError(str(e), getattr(e, "code_name", None))