Switch to hogql filters for set_once properties

PostHog · Oct 16, 2023 · 206fb13 · 206fb13
1 parent 3f724eb
commit 206fb13
Show file tree

Hide file tree

Showing 7 changed files with 124 additions and 66 deletions.
diff --git a/frontend/src/queries/schema.json b/frontend/src/queries/schema.json
@@ -2404,7 +2404,14 @@
         },
         "WebAnalyticsPropertyFilters": {
             "items": {
-                "$ref": "#/definitions/EventPropertyFilter"
+                "anyOf": [
+                    {
+                        "$ref": "#/definitions/EventPropertyFilter"
+                    },
+                    {
+                        "$ref": "#/definitions/HogQLPropertyFilter"
+                    }
+                ]
             },
             "type": "array"
         },

diff --git a/frontend/src/queries/schema.ts b/frontend/src/queries/schema.ts
@@ -1,27 +1,28 @@
 import {
     AnyPropertyFilter,
+    BaseMathType,
     Breakdown,
     BreakdownKeyType,
     BreakdownType,
-    PropertyGroupFilter,
-    EventType,
-    IntervalType,
-    BaseMathType,
-    PropertyMathType,
     CountPerActorMathType,
-    GroupMathType,
+    EventPropertyFilter,
+    EventType,
     FilterType,
-    TrendsFilterType,
     FunnelsFilterType,
-    RetentionFilterType,
-    PathsFilterType,
-    StickinessFilterType,
-    LifecycleFilterType,
-    LifecycleToggle,
+    GroupMathType,
     HogQLMathType,
+    HogQLPropertyFilter,
     InsightLogicProps,
     InsightShortId,
-    EventPropertyFilter,
+    IntervalType,
+    LifecycleFilterType,
+    LifecycleToggle,
+    PathsFilterType,
+    PropertyGroupFilter,
+    PropertyMathType,
+    RetentionFilterType,
+    StickinessFilterType,
+    TrendsFilterType,
 } from '~/types'
 import { ComponentType } from 'react'
 
@@ -540,7 +541,7 @@ export interface PersonsQuery extends DataNode {
     response?: PersonsQueryResponse
 }
 
-export type WebAnalyticsPropertyFilters = EventPropertyFilter[]
+export type WebAnalyticsPropertyFilters = (EventPropertyFilter | HogQLPropertyFilter)[]
 
 export interface WebAnalyticsQueryBase {
     dateRange?: DateRange

diff --git a/frontend/src/scenes/web-analytics/WebDashboard.tsx b/frontend/src/scenes/web-analytics/WebDashboard.tsx
@@ -79,16 +79,16 @@ const BreakdownValueCell: QueryContextColumnComponent = (props) => {
             propertyName = '$pathname'
             break
         case WebStatsBreakdown.InitialPage:
-            propertyName = '$set_once.$initial_pathname'
+            propertyName = '$initial_pathname'
             break
         case WebStatsBreakdown.InitialReferringDomain:
-            propertyName = '$set_once.$initial_referrer'
+            propertyName = '$initial_referrer'
             break
         case WebStatsBreakdown.InitialUTMSource:
-            propertyName = '$set_once.$initial_utm_source'
+            propertyName = '$initial_utm_source'
             break
         case WebStatsBreakdown.InitialUTMCampaign:
-            propertyName = '$set_once.$initial_utm_campaign'
+            propertyName = '$initial_utm_campaign'
             break
         case WebStatsBreakdown.Browser:
             propertyName = '$browser'

diff --git a/frontend/src/scenes/web-analytics/webAnalyticsLogic.ts b/frontend/src/scenes/web-analytics/webAnalyticsLogic.ts
@@ -2,7 +2,14 @@ import { actions, connect, kea, listeners, path, reducers, selectors, sharedList
 
 import type { webAnalyticsLogicType } from './webAnalyticsLogicType'
 import { NodeKind, QuerySchema, WebAnalyticsPropertyFilters, WebStatsBreakdown } from '~/queries/schema'
-import { BaseMathType, ChartDisplayType, EventPropertyFilter, PropertyFilterType, PropertyOperator } from '~/types'
+import {
+    BaseMathType,
+    ChartDisplayType,
+    EventPropertyFilter,
+    HogQLPropertyFilter,
+    PropertyFilterType,
+    PropertyOperator,
+} from '~/types'
 import { isNotNil } from 'lib/utils'
 
 interface Layout {
@@ -51,6 +58,11 @@ export enum PathTab {
 
 export const initialWebAnalyticsFilter = [] as WebAnalyticsPropertyFilters
 
+const setOncePropertyNames = ['$initial_pathname', '$initial_referrer', '$initial_utm_source', '$initial_utm_campaign']
+const hogqlForSetOnceProperty = (key: string, value: string): string => `properties.$set_once.${key} = '${value}'`
+const isHogqlForSetOnceProperty = (key: string, p: HogQLPropertyFilter): boolean =>
+    setOncePropertyNames.includes(key) && p.key.startsWith(`properties.$set_once.${key} = `)
+
 export const webAnalyticsLogic = kea<webAnalyticsLogicType>([
     path(['scenes', 'webAnalytics', 'webAnalyticsSceneLogic']),
     connect({}),
@@ -73,43 +85,81 @@ export const webAnalyticsLogic = kea<webAnalyticsLogicType>([
             {
                 setWebAnalyticsFilters: (_, { webAnalyticsFilters }) => webAnalyticsFilters,
                 togglePropertyFilter: (oldPropertyFilters, { key, value }) => {
-                    if (oldPropertyFilters.some((f) => f.key === key && f.operator === PropertyOperator.Exact)) {
+                    if (
+                        oldPropertyFilters.some(
+                            (f) =>
+                                (f.type === PropertyFilterType.Event &&
+                                    f.key === key &&
+                                    f.operator === PropertyOperator.Exact) ||
+                                (f.type === PropertyFilterType.HogQL && isHogqlForSetOnceProperty(key, f))
+                        )
+                    ) {
                         return oldPropertyFilters
                             .map((f) => {
-                                if (
-                                    f.type !== PropertyFilterType.Event ||
-                                    f.key !== key ||
-                                    f.operator !== PropertyOperator.Exact
-                                ) {
-                                    return f
-                                }
-                                const oldValue = (Array.isArray(f.value) ? f.value : [f.value]).filter(isNotNil)
-                                let newValue: (string | number)[]
-                                if (oldValue.includes(value)) {
-                                    // If there are multiple values for this filter, reduce that to just the one being clicked
-                                    if (oldValue.length > 1) {
-                                        newValue = [value]
-                                    } else {
+                                if (setOncePropertyNames.includes(key)) {
+                                    if (f.type !== PropertyFilterType.HogQL) {
+                                        return f
+                                    }
+                                    if (!isHogqlForSetOnceProperty(key, f)) {
+                                        return f
+                                    }
+                                    // With the hogql properties, we don't even attempt to handle arrays, to avoiding
+                                    // needing a parser on the front end. Instead the logic is much simpler
+                                    const hogql = hogqlForSetOnceProperty(key, value)
+                                    if (f.key === hogql) {
                                         return null
+                                    } else {
+                                        return {
+                                            type: PropertyFilterType.HogQL,
+                                            key,
+                                            value: hogql,
+                                        } as const
                                     }
                                 } else {
-                                    newValue = [...oldValue, value]
+                                    if (
+                                        f.key !== key ||
+                                        f.type !== PropertyFilterType.Event ||
+                                        f.operator !== PropertyOperator.Exact
+                                    ) {
+                                        return f
+                                    }
+                                    const oldValue = (Array.isArray(f.value) ? f.value : [f.value]).filter(isNotNil)
+                                    let newValue: (string | number)[]
+                                    if (oldValue.includes(value)) {
+                                        // If there are multiple values for this filter, reduce that to just the one being clicked
+                                        if (oldValue.length > 1) {
+                                            newValue = [value]
+                                        } else {
+                                            return null
+                                        }
+                                    } else {
+                                        newValue = [...oldValue, value]
+                                    }
+                                    return {
+                                        type: PropertyFilterType.Event,
+                                        key,
+                                        operator: PropertyOperator.Exact,
+                                        value: newValue,
+                                    } as const
                                 }
-                                return {
-                                    type: PropertyFilterType.Event,
-                                    key,
-                                    operator: PropertyOperator.Exact,
-                                    value: newValue,
-                                } as const
                             })
                             .filter(isNotNil)
                     } else {
-                        const newFilter: EventPropertyFilter = {
-                            type: PropertyFilterType.Event,
-                            key,
-                            value,
-                            operator: PropertyOperator.Exact,
+                        let newFilter: EventPropertyFilter | HogQLPropertyFilter
+                        if (setOncePropertyNames.includes(key)) {
+                            newFilter = {
+                                type: PropertyFilterType.HogQL,
+                                key: hogqlForSetOnceProperty(key, value),
+                            }
+                        } else {
+                            newFilter = {
+                                type: PropertyFilterType.Event,
+                                key,
+                                value,
+                                operator: PropertyOperator.Exact,
+                            }
                         }
+
                         return [...oldPropertyFilters, newFilter]
                     }
                 },

diff --git a/posthog/hogql/property.py b/posthog/hogql/property.py
@@ -129,7 +129,7 @@ def property_to_expr(
                 return ast.Or(exprs=exprs)
 
         chain = ["person", "properties"] if property.type == "person" and scope != "person" else ["properties"]
-        field = ast.Field(chain=chain + property.key.split("."))
+        field = ast.Field(chain=chain + [property.key])
 
         if operator == PropertyOperator.is_set:
             return ast.CompareOperation(op=ast.CompareOperationOp.NotEq, left=field, right=ast.Constant(value=None))

diff --git a/posthog/hogql_queries/web_analytics/stats_table.py b/posthog/hogql_queries/web_analytics/stats_table.py
@@ -84,39 +84,39 @@ def counts_breakdown(self):
             case WebStatsBreakdown.Page:
                 return parse_expr("properties.$pathname")
             case WebStatsBreakdown.InitialPage:
-                return parse_expr("events.properties.$set_once.$initial_pathname")
+                return parse_expr("properties.$set_once.$initial_pathname")
             case WebStatsBreakdown.InitialReferringDomain:
-                return parse_expr("events.properties.$set_once.$initial_referring_domain")
+                return parse_expr("properties.$set_once.$initial_referring_domain")
             case WebStatsBreakdown.InitialUTMSource:
-                return parse_expr("events.properties.$set_once.$initial_utm_source")
+                return parse_expr("properties.$set_once.$initial_utm_source")
             case WebStatsBreakdown.InitialUTMCampaign:
-                return parse_expr("events.properties.$set_once.$initial_utm_campaign")
+                return parse_expr("properties.$set_once.$initial_utm_campaign")
             case WebStatsBreakdown.Browser:
-                return parse_expr("events.properties.$browser")
+                return parse_expr("properties.$browser")
             case WebStatsBreakdown.OS:
-                return parse_expr("events.properties.$os")
+                return parse_expr("properties.$os")
             case WebStatsBreakdown.DeviceType:
-                return parse_expr("events.properties.$device_type")
+                return parse_expr("properties.$device_type")
             case _:
                 raise NotImplementedError("Breakdown not implemented")
 
     def bounce_breakdown(self):
         match self.query.breakdownBy:
             case WebStatsBreakdown.Page:
-                return parse_expr("any(events.properties.$set_once.$initial_pathname)")
+                return parse_expr("any(properties.$set_once.$initial_pathname)")
             case WebStatsBreakdown.InitialPage:
-                return parse_expr("any(events.properties.$set_once.$initial_pathname)")
+                return parse_expr("any(properties.$set_once.$initial_pathname)")
             case WebStatsBreakdown.InitialReferringDomain:
-                return parse_expr("any(events.properties.$set_once.$initial_referring_domain)")
+                return parse_expr("any(properties.$set_once.$initial_referring_domain)")
             case WebStatsBreakdown.InitialUTMSource:
-                return parse_expr("any(events.properties.$set_once.$initial_utm_source)")
+                return parse_expr("any(properties.$set_once.$initial_utm_source)")
             case WebStatsBreakdown.InitialUTMCampaign:
-                return parse_expr("any(events.properties.$set_once.$initial_utm_campaign)")
+                return parse_expr("any(properties.$set_once.$initial_utm_campaign)")
             case WebStatsBreakdown.Browser:
-                return parse_expr("any(events.properties.$browser)")
+                return parse_expr("any(properties.$browser)")
             case WebStatsBreakdown.OS:
-                return parse_expr("any(events.properties.$os)")
+                return parse_expr("any(properties.$os)")
             case WebStatsBreakdown.DeviceType:
-                return parse_expr("any(events.properties.$device_type)")
+                return parse_expr("any(properties.$device_type)")
             case _:
                 raise NotImplementedError("Breakdown not implemented")
diff --git a/posthog/schema.py b/posthog/schema.py
@@ -738,7 +738,7 @@ class WebOverviewStatsQuery(BaseModel):
     )
     dateRange: Optional[DateRange] = None
     kind: Literal["WebOverviewStatsQuery"] = "WebOverviewStatsQuery"
-    properties: List[EventPropertyFilter]
+    properties: List[Union[EventPropertyFilter, HogQLPropertyFilter]]
     response: Optional[WebOverviewStatsQueryResponse] = None
 
 
@@ -749,7 +749,7 @@ class WebStatsTableQuery(BaseModel):
     breakdownBy: WebStatsBreakdown
     dateRange: Optional[DateRange] = None
     kind: Literal["WebStatsTableQuery"] = "WebStatsTableQuery"
-    properties: List[EventPropertyFilter]
+    properties: List[Union[EventPropertyFilter, HogQLPropertyFilter]]
     response: Optional[WebStatsTableQueryResponse] = None
 
 
@@ -759,7 +759,7 @@ class WebTopClicksQuery(BaseModel):
     )
     dateRange: Optional[DateRange] = None
     kind: Literal["WebTopClicksQuery"] = "WebTopClicksQuery"
-    properties: List[EventPropertyFilter]
+    properties: List[Union[EventPropertyFilter, HogQLPropertyFilter]]
     response: Optional[WebTopClicksQueryResponse] = None