feat(insights): string breakdowns (#21023)

frontend/src/queries/query.ts

@@ -220,7 +220,13 @@ export async function query<N extends DataNode = DataNode>(
                 (hogQLInsightsFunnelsFlagEnabled && isFunnelsQuery(queryNode))
             ) {
                 if (hogQLInsightsLiveCompareEnabled) {
-                    const legacyFunction = legacyUrl ? fetchLegacyUrl : fetchLegacyInsights
+                    const legacyFunction = (): any => {
+                        try {
+                            return legacyUrl ? fetchLegacyUrl : fetchLegacyInsights
+                        } catch (e) {
+                            console.error('Error fetching legacy insights', e)
+                        }
+                    }
                     let legacyResponse: any
                     ;[response, legacyResponse] = await Promise.all([
                         executeQuery(queryNode, methodOptions, refresh, queryId),

frontend/src/scenes/insights/views/InsightsTable/InsightsTable.tsx

@@ -3,6 +3,7 @@ import './InsightsTable.scss'
 import { useActions, useValues } from 'kea'
 import { getSeriesColor } from 'lib/colors'
 import { LemonTable, LemonTableColumn } from 'lib/lemon-ui/LemonTable'
+import { compare as compareFn } from 'natural-orderby'
 import { insightLogic } from 'scenes/insights/insightLogic'
 import { insightSceneLogic } from 'scenes/insights/insightSceneLogic'
 import { isTrendsFilter } from 'scenes/insights/sharedUtils'
@@ -157,7 +158,7 @@ export function InsightsTable({
                 }
                 const labelA = formatItemBreakdownLabel(a)
                 const labelB = formatItemBreakdownLabel(b)
-                return labelA.localeCompare(labelB)
+                return compareFn()(labelA, labelB)
             },
         })
         if (isTrends && display === ChartDisplayType.WorldMap) {

mypy-baseline.txt

@@ -346,20 +346,12 @@ posthog/hogql_queries/sessions_timeline_query_runner.py:0: error: Statement is u
 posthog/hogql_queries/insights/trends/breakdown.py:0: error: Item "None" of "BreakdownFilter | None" has no attribute "breakdown_type"  [union-attr]
 posthog/hogql_queries/insights/trends/breakdown.py:0: error: Item "None" of "BreakdownFilter | None" has no attribute "breakdown_histogram_bin_count"  [union-attr]
 posthog/hogql_queries/insights/trends/breakdown.py:0: error: Item "None" of "BreakdownFilter | None" has no attribute "breakdown_type"  [union-attr]
-posthog/hogql_queries/insights/trends/breakdown.py:0: error: Item "None" of "BreakdownFilter | None" has no attribute "breakdown"  [union-attr]
-posthog/hogql_queries/insights/trends/breakdown.py:0: error: Argument 1 to "parse_expr" has incompatible type "str | float | list[str | float] | Any | None"; expected "str"  [arg-type]
-posthog/hogql_queries/insights/trends/breakdown.py:0: error: Item "None" of "BreakdownFilter | None" has no attribute "breakdown_type"  [union-attr]
-posthog/hogql_queries/insights/trends/breakdown.py:0: error: Item "None" of "BreakdownFilter | None" has no attribute "breakdown_type"  [union-attr]
-posthog/hogql_queries/insights/trends/breakdown.py:0: error: Item "None" of "BreakdownFilter | None" has no attribute "breakdown"  [union-attr]
-posthog/hogql_queries/insights/trends/breakdown.py:0: error: Argument 1 to "parse_expr" has incompatible type "str | float | list[str | float] | Any | None"; expected "str"  [arg-type]
-posthog/hogql_queries/insights/trends/breakdown.py:0: error: Statement is unreachable  [unreachable]
 posthog/hogql_queries/insights/trends/breakdown.py:0: error: Argument "exprs" to "Or" has incompatible type "list[CompareOperation]"; expected "list[Expr]"  [arg-type]
 posthog/hogql_queries/insights/trends/breakdown.py:0: note: "List" is invariant -- see https://mypy.readthedocs.io/en/stable/common_issues.html#variance
 posthog/hogql_queries/insights/trends/breakdown.py:0: note: Consider using "Sequence" instead, which is covariant
-posthog/hogql_queries/insights/trends/breakdown.py:0: error: Incompatible types in assignment (expression has type "float", variable has type "int")  [assignment]
-posthog/hogql_queries/insights/trends/breakdown.py:0: error: Incompatible types in assignment (expression has type "float", variable has type "int")  [assignment]
-posthog/hogql_queries/insights/trends/breakdown.py:0: error: Incompatible types in assignment (expression has type "str", variable has type "int")  [assignment]
-posthog/hogql_queries/insights/trends/breakdown.py:0: error: Incompatible types in assignment (expression has type "str", variable has type "int")  [assignment]
+posthog/hogql_queries/insights/trends/breakdown.py:0: error: Item "None" of "BreakdownFilter | None" has no attribute "breakdown_type"  [union-attr]
+posthog/hogql_queries/insights/trends/breakdown.py:0: error: Item "None" of "BreakdownFilter | None" has no attribute "breakdown"  [union-attr]
+posthog/hogql_queries/insights/trends/breakdown.py:0: error: Argument 1 to "parse_expr" has incompatible type "str | float | list[str | float] | Any | None"; expected "str"  [arg-type]
 posthog/hogql_queries/insights/trends/breakdown.py:0: error: Item "None" of "BreakdownFilter | None" has no attribute "breakdown_type"  [union-attr]
 posthog/hogql_queries/insights/trends/breakdown.py:0: error: Item "None" of "BreakdownFilter | None" has no attribute "breakdown"  [union-attr]
 posthog/hogql_queries/insights/trends/breakdown.py:0: error: Argument "breakdown_field" to "get_properties_chain" has incompatible type "str | float | list[str | float] | Any | None"; expected "str"  [arg-type]

package.json

@@ -140,6 +140,7 @@
         "maplibre-gl": "^3.5.1",
         "md5": "^2.3.0",
         "monaco-editor": "^0.39.0",
+        "natural-orderby": "^3.0.2",
         "papaparse": "^5.4.1",
         "pmtiles": "^2.11.0",
         "postcss": "^8.4.31",

posthog/hogql_queries/insights/trends/breakdown.py

@@ -3,9 +3,7 @@
 from posthog.hogql.parser import parse_expr
 from posthog.hogql.timings import HogQLTimings
 from posthog.hogql_queries.insights.trends.breakdown_values import (
-    BREAKDOWN_NULL_NUMERIC_LABEL,
     BREAKDOWN_NULL_STRING_LABEL,
-    BREAKDOWN_OTHER_NUMERIC_LABEL,
     BREAKDOWN_OTHER_STRING_LABEL,
     BreakdownValues,
 )
@@ -19,6 +17,10 @@
 from posthog.schema import ActionsNode, EventsNode, DataWarehouseNode, HogQLQueryModifiers, InCohortVia, TrendsQuery
 
 
+def hogql_to_string(expr: ast.Expr) -> ast.Call:
+    return ast.Call(name="toString", args=[expr])
+
+
 class Breakdown:
     query: TrendsQuery
     team: Team
@@ -27,7 +29,7 @@ class Breakdown:
     timings: HogQLTimings
     modifiers: HogQLQueryModifiers
     events_filter: ast.Expr
-    breakdown_values_override: Optional[List[str | int | float]]
+    breakdown_values_override: Optional[List[str]]
 
     def __init__(
         self,
@@ -38,7 +40,7 @@ def __init__(
         timings: HogQLTimings,
         modifiers: HogQLQueryModifiers,
         events_filter: ast.Expr,
-        breakdown_values_override: Optional[List[str | int | float]] = None,
+        breakdown_values_override: Optional[List[str]] = None,
     ):
         self.team = team
         self.query = query
@@ -70,39 +72,25 @@ def placeholders(self) -> Dict[str, ast.Expr]:
 
         return {"cross_join_breakdown_values": ast.Alias(alias="breakdown_value", expr=values)}
 
-    def column_expr(self) -> ast.Expr:
+    def column_expr(self) -> ast.Alias:
         if self.is_histogram_breakdown:
             return ast.Alias(alias="breakdown_value", expr=self._get_breakdown_histogram_multi_if())
-        elif self.query.breakdownFilter.breakdown_type == "hogql":
-            return ast.Alias(
-                alias="breakdown_value",
-                expr=parse_expr(self.query.breakdownFilter.breakdown),
-            )
-        elif self.query.breakdownFilter.breakdown_type == "cohort":
+
+        if self.query.breakdownFilter.breakdown_type == "cohort":
             if self.modifiers.inCohortVia == InCohortVia.leftjoin_conjoined:
                 return ast.Alias(
                     alias="breakdown_value",
-                    expr=ast.Field(chain=["__in_cohort", "cohort_id"]),
+                    expr=hogql_to_string(ast.Field(chain=["__in_cohort", "cohort_id"])),
                 )
 
             cohort_breakdown = (
                 0 if self.query.breakdownFilter.breakdown == "all" else int(self.query.breakdownFilter.breakdown)  # type: ignore
             )
             return ast.Alias(
                 alias="breakdown_value",
-                expr=ast.Constant(value=cohort_breakdown),
-            )
-
-        if self.query.breakdownFilter.breakdown_type == "hogql":
-            return ast.Alias(
-                alias="breakdown_value",
-                expr=parse_expr(self.query.breakdownFilter.breakdown),
+                expr=hogql_to_string(ast.Constant(value=cohort_breakdown)),
             )
 
-        # If there's no breakdown values
-        if len(self._breakdown_values) == 1 and self._breakdown_values[0] is None:
-            return ast.Alias(alias="breakdown_value", expr=ast.Field(chain=self._properties_chain))
-
         return ast.Alias(alias="breakdown_value", expr=self._get_breakdown_transform_func)
 
     def events_where_filter(self) -> ast.Expr | None:
@@ -148,27 +136,22 @@ def events_where_filter(self) -> ast.Expr | None:
         else:
             left = ast.Field(chain=self._properties_chain)
 
+        if not self.is_histogram_breakdown:
+            left = hogql_to_string(left)
+
         compare_ops = []
         for _value in self._breakdown_values:
-            value: Optional[str | int | float] = _value
+            value: Optional[str] = str(_value)  # non-cohorts are always strings
             # If the value is one of the "other" values, then use the `transform()` func
-            if (
-                value == BREAKDOWN_OTHER_STRING_LABEL
-                or value == BREAKDOWN_OTHER_NUMERIC_LABEL
-                or value == float(BREAKDOWN_OTHER_NUMERIC_LABEL)
-            ):
+            if value == BREAKDOWN_OTHER_STRING_LABEL:
                 transform_func = self._get_breakdown_transform_func
                 compare_ops.append(
                     ast.CompareOperation(
                         left=transform_func, op=ast.CompareOperationOp.Eq, right=ast.Constant(value=value)
                     )
                 )
             else:
-                if (
-                    value == BREAKDOWN_NULL_STRING_LABEL
-                    or value == BREAKDOWN_NULL_NUMERIC_LABEL
-                    or value == float(BREAKDOWN_NULL_NUMERIC_LABEL)
-                ):
+                if value == BREAKDOWN_NULL_STRING_LABEL:
                     value = None
 
                 compare_ops.append(
@@ -184,30 +167,25 @@ def events_where_filter(self) -> ast.Expr | None:
 
     @cached_property
     def _get_breakdown_transform_func(self) -> ast.Call:
-        values = self._breakdown_values
-        all_values_are_ints_or_none = all(isinstance(value, int) or value is None for value in values)
-        all_values_are_floats_or_none = all(isinstance(value, float) or value is None for value in values)
-
-        if all_values_are_ints_or_none:
-            breakdown_other_value = BREAKDOWN_OTHER_NUMERIC_LABEL
-            breakdown_null_value = BREAKDOWN_NULL_NUMERIC_LABEL
-        elif all_values_are_floats_or_none:
-            breakdown_other_value = float(BREAKDOWN_OTHER_NUMERIC_LABEL)
-            breakdown_null_value = float(BREAKDOWN_NULL_NUMERIC_LABEL)
-        else:
-            breakdown_other_value = BREAKDOWN_OTHER_STRING_LABEL
-            breakdown_null_value = BREAKDOWN_NULL_STRING_LABEL
+        if self.query.breakdownFilter.breakdown_type == "hogql":
+            return self._get_breakdown_values_transform(parse_expr(self.query.breakdownFilter.breakdown))
+        return self._get_breakdown_values_transform(ast.Field(chain=self._properties_chain))
 
+    def _get_breakdown_values_transform(self, node: ast.Expr) -> ast.Call:
+        breakdown_values = self._breakdown_values_ast
         return ast.Call(
             name="transform",
             args=[
                 ast.Call(
                     name="ifNull",
-                    args=[ast.Field(chain=self._properties_chain), ast.Constant(value=breakdown_null_value)],
+                    args=[
+                        hogql_to_string(node),
+                        ast.Constant(value=BREAKDOWN_NULL_STRING_LABEL),
+                    ],
                 ),
-                self._breakdown_values_ast,
-                self._breakdown_values_ast,
-                ast.Constant(value=breakdown_other_value),
+                breakdown_values,
+                breakdown_values,
+                ast.Constant(value=BREAKDOWN_OTHER_STRING_LABEL),
             ],
         )
 
@@ -220,15 +198,21 @@ def _breakdown_buckets_ast(self) -> ast.Array:
 
         return ast.Array(exprs=list(map(lambda v: ast.Constant(value=v), values)))
 
-    @cached_property
+    @property
     def _breakdown_values_ast(self) -> ast.Array:
-        return ast.Array(exprs=[ast.Constant(value=v) for v in self._breakdown_values])
+        exprs: list[ast.Expr] = []
+        for value in self._breakdown_values:
+            if isinstance(value, str):
+                exprs.append(ast.Constant(value=value))
+            else:
+                exprs.append(hogql_to_string(ast.Constant(value=value)))
+        return ast.Array(exprs=exprs)
 
     @cached_property
-    def _all_breakdown_values(self) -> List[str | int | float | None]:
+    def _all_breakdown_values(self) -> List[str | int | None]:
         # Used in the actors query
         if self.breakdown_values_override is not None:
-            return cast(List[str | int | float | None], self.breakdown_values_override)
+            return cast(List[str | int | None], self.breakdown_values_override)
 
         if self.query.breakdownFilter is None:
             return []
@@ -243,25 +227,12 @@ def _all_breakdown_values(self) -> List[str | int | float | None]:
                 query_date_range=self.query_date_range,
                 modifiers=self.modifiers,
             )
-            return cast(List[str | int | float | None], breakdown.get_breakdown_values())
+            return cast(List[str | int | None], breakdown.get_breakdown_values())
 
     @cached_property
-    def _breakdown_values(self) -> List[str | int | float]:
-        values = self._all_breakdown_values
-        if len(values) == 0 or all(value is None for value in values):
-            return []
-
-        if None in values:
-            all_values_are_ints_or_none = all(isinstance(value, int) or value is None for value in values)
-            all_values_are_floats_or_none = all(isinstance(value, float) or value is None for value in values)
-
-            if all_values_are_ints_or_none:
-                values = [v if v is not None else BREAKDOWN_NULL_NUMERIC_LABEL for v in values]
-            elif all_values_are_floats_or_none:
-                values = [v if v is not None else float(BREAKDOWN_NULL_NUMERIC_LABEL) for v in values]
-            else:
-                values = [v if v is not None else BREAKDOWN_NULL_STRING_LABEL for v in values]
-        return cast(List[str | int | float], values)
+    def _breakdown_values(self) -> List[str | int]:
+        values = [BREAKDOWN_NULL_STRING_LABEL if v is None else v for v in self._all_breakdown_values]
+        return cast(List[str | int], values)
 
     @cached_property
     def has_breakdown_values(self) -> bool:

posthog/hogql_queries/insights/trends/breakdown_values.py

@@ -97,6 +97,9 @@ def get_breakdown_values(self) -> List[str | int]:
                 ),
             )
 
+        if not self.histogram_bin_count:
+            select_field.expr = ast.Call(name="toString", args=[select_field.expr])
+
         if self.chart_display_type == ChartDisplayType.WorldMap:
             breakdown_limit = BREAKDOWN_VALUES_LIMIT_FOR_COUNTRIES
         else:
@@ -211,23 +214,9 @@ def get_breakdown_values(self) -> List[str | int]:
 
             # Add "other" value if "other" is not hidden and we're not bucketing numeric values
             if self.hide_other_aggregation is not True and self.histogram_bin_count is None:
-                all_values_are_ints_or_none = all(isinstance(value, int) or value is None for value in values)
-                all_values_are_floats_or_none = all(isinstance(value, float) or value is None for value in values)
-                all_values_are_string_or_none = all(isinstance(value, str) or value is None for value in values)
-
-                if all_values_are_string_or_none:
-                    values = [BREAKDOWN_NULL_STRING_LABEL if value in (None, "") else value for value in values]
-                    if needs_other:
-                        values.insert(0, BREAKDOWN_OTHER_STRING_LABEL)
-                elif all_values_are_ints_or_none or all_values_are_floats_or_none:
-                    if all_values_are_ints_or_none:
-                        values = [BREAKDOWN_NULL_NUMERIC_LABEL if value is None else value for value in values]
-                        if needs_other:
-                            values.insert(0, BREAKDOWN_OTHER_NUMERIC_LABEL)
-                    else:
-                        values = [float(BREAKDOWN_NULL_NUMERIC_LABEL) if value is None else value for value in values]
-                        if needs_other:
-                            values.insert(0, float(BREAKDOWN_OTHER_NUMERIC_LABEL))
+                values = [BREAKDOWN_NULL_STRING_LABEL if value in (None, "") else value for value in values]
+                if needs_other:
+                    values = [BREAKDOWN_OTHER_STRING_LABEL] + values
 
         if len(values) == 0:
             values.insert(0, None)