From a0558c17ebfb34c0b3fe7eb1d1dd5f4582e1ad25 Mon Sep 17 00:00:00 2001 From: Marius Andra Date: Tue, 19 Sep 2023 15:28:47 +0200 Subject: [PATCH] properties --- posthog/hogql/property.py | 34 ++++++++++++++----- posthog/hogql_queries/persons_query_runner.py | 17 +++++++--- .../test/test_persons_query_runner.py | 29 ++++++++++++++-- 3 files changed, 63 insertions(+), 17 deletions(-) diff --git a/posthog/hogql/property.py b/posthog/hogql/property.py index 81efafc225a1f..7695dbe640e1c 100644 --- a/posthog/hogql/property.py +++ b/posthog/hogql/property.py @@ -1,5 +1,5 @@ import re -from typing import Any, List, Optional, Union, cast +from typing import Any, List, Optional, Union, cast, Literal from pydantic import BaseModel @@ -47,11 +47,15 @@ def visit_call(self, node: ast.Call): self.visit(arg) -def property_to_expr(property: Union[BaseModel, PropertyGroup, Property, dict, list], team: Team) -> ast.Expr: +def property_to_expr( + property: Union[BaseModel, PropertyGroup, Property, dict, list], + team: Team, + scope: Literal["event", "person"] = "event", +) -> ast.Expr: if isinstance(property, dict): property = Property(**property) elif isinstance(property, list): - properties = [property_to_expr(p, team) for p in property] + properties = [property_to_expr(p, team, scope) for p in property] if len(properties) == 0: return ast.Constant(value=True) if len(properties) == 1: @@ -80,12 +84,12 @@ def property_to_expr(property: Union[BaseModel, PropertyGroup, Property, dict, l if len(property.values) == 0: return ast.Constant(value=True) if len(property.values) == 1: - return property_to_expr(property.values[0], team) + return property_to_expr(property.values[0], team, scope) if property.type == PropertyOperatorType.AND or property.type == FilterLogicalOperator.AND: - return ast.And(exprs=[property_to_expr(p, team) for p in property.values]) + return ast.And(exprs=[property_to_expr(p, team, scope) for p in property.values]) else: - return ast.Or(exprs=[property_to_expr(p, team) for p in property.values]) + return ast.Or(exprs=[property_to_expr(p, team, scope) for p in property.values]) elif isinstance(property, BaseModel): property = Property(**property.dict()) else: @@ -96,6 +100,10 @@ def property_to_expr(property: Union[BaseModel, PropertyGroup, Property, dict, l if property.type == "hogql": return parse_expr(property.key) elif property.type == "event" or cast(Any, property.type) == "feature" or property.type == "person": + if scope == "person" and property.type != "person": + raise NotImplementedException( + f"property_to_expr for scope {scope} not implemented for type '{property.type}'" + ) operator = cast(Optional[PropertyOperator], property.operator) or PropertyOperator.exact value = property.value if isinstance(value, list): @@ -106,7 +114,7 @@ def property_to_expr(property: Union[BaseModel, PropertyGroup, Property, dict, l else: exprs = [ property_to_expr( - Property(type=property.type, key=property.key, operator=property.operator, value=v), team + Property(type=property.type, key=property.key, operator=property.operator, value=v), team, scope ) for v in value ] @@ -118,7 +126,7 @@ def property_to_expr(property: Union[BaseModel, PropertyGroup, Property, dict, l return ast.And(exprs=exprs) return ast.Or(exprs=exprs) - chain = ["person", "properties"] if property.type == "person" else ["properties"] + chain = ["person", "properties"] if property.type == "person" and scope != "person" else ["properties"] field = ast.Field(chain=chain + [property.key]) if operator == PropertyOperator.is_set: @@ -179,6 +187,10 @@ def property_to_expr(property: Union[BaseModel, PropertyGroup, Property, dict, l return ast.CompareOperation(op=op, left=field, right=ast.Constant(value=value)) elif property.type == "element": + if scope == "person": + raise NotImplementedException( + f"property_to_expr for scope {scope} not implemented for type '{property.type}'" + ) value = property.value operator = cast(Optional[PropertyOperator], property.operator) or PropertyOperator.exact if isinstance(value, list): @@ -187,7 +199,7 @@ def property_to_expr(property: Union[BaseModel, PropertyGroup, Property, dict, l else: exprs = [ property_to_expr( - Property(type=property.type, key=property.key, operator=property.operator, value=v), team + Property(type=property.type, key=property.key, operator=property.operator, value=v), team, scope ) for v in value ] @@ -219,6 +231,10 @@ def property_to_expr(property: Union[BaseModel, PropertyGroup, Property, dict, l elif property.type == "cohort" or property.type == "static-cohort" or property.type == "precalculated-cohort": if not team: raise Exception("Can not convert cohort property to expression without team") + if scope == "person": + raise NotImplementedException( + f"property_to_expr for scope {scope} not implemented for type '{property.type}'" + ) cohort = Cohort.objects.get(team=team, id=property.value) return ast.CompareOperation( diff --git a/posthog/hogql_queries/persons_query_runner.py b/posthog/hogql_queries/persons_query_runner.py index c15ada5ec0571..bfa39ab67341b 100644 --- a/posthog/hogql_queries/persons_query_runner.py +++ b/posthog/hogql_queries/persons_query_runner.py @@ -35,24 +35,31 @@ def run(self) -> PersonsQueryResponse: ) def to_query(self) -> ast.SelectQuery: - where: List[ast.Expr] = [] + where_exprs: List[ast.Expr] = [] if self.query.properties: - where.append(property_to_expr(self.query.properties, self.team)) + where_exprs.append(property_to_expr(self.query.properties, self.team, scope="person")) if self.query.fixedProperties: - where.append(property_to_expr(self.query.fixedProperties, self.team)) + where_exprs.append(property_to_expr(self.query.fixedProperties, self.team, scope="person")) if self.query.source: source = self.query.source if isinstance(source, LifecycleQuery): source_query = get_query_runner(source, self.team, self.timings).to_persons_query() - where.append( + where_exprs.append( ast.CompareOperation(op=ast.CompareOperationOp.In, left=ast.Field(chain=["id"]), right=source_query) ) else: raise ValueError(f"Queries of type '{source.kind}' are not supported as a PersonsQuery sources.") + if len(where_exprs) == 0: + where = ast.Constant(value=True) + elif len(where_exprs) == 1: + where = where_exprs[0] + else: + where = ast.And(exprs=where_exprs) + # adding +1 to the limit to check if there's a "next page" after the requested results from posthog.hogql.constants import DEFAULT_RETURNED_ROWS, MAX_SELECT_RETURNED_ROWS @@ -65,7 +72,7 @@ def to_query(self) -> ast.SelectQuery: stmt = ast.SelectQuery( select=[], select_from=ast.JoinExpr(table=ast.Field(chain=["persons"])), - where=ast.And(exprs=where) if len(where) > 0 else None, + where=where, # having=having, # group_by=group_by if has_any_aggregation else None, # order_by=order_by, diff --git a/posthog/hogql_queries/test/test_persons_query_runner.py b/posthog/hogql_queries/test/test_persons_query_runner.py index da9da6dbf9a01..253d5e067907b 100644 --- a/posthog/hogql_queries/test/test_persons_query_runner.py +++ b/posthog/hogql_queries/test/test_persons_query_runner.py @@ -1,7 +1,7 @@ from posthog.hogql import ast from posthog.hogql_queries.persons_query_runner import PersonsQueryRunner from posthog.models.utils import UUIDT -from posthog.schema import PersonsQuery +from posthog.schema import PersonsQuery, PersonPropertyFilter, HogQLPropertyFilter, PropertyOperator from posthog.test.base import APIBaseTest, ClickhouseTestMixin, _create_person, flush_persons_and_events @@ -33,11 +33,34 @@ def test_basic_persons_query(self): ast.SelectQuery( select=[], select_from=ast.JoinExpr(table=ast.Field(chain=["persons"])), - where=None, + where=ast.Constant(value=True), limit=ast.Constant(value=101), offset=ast.Constant(value=0), ), ) - response = runner.run() self.assertEqual(len(response.results), 10) + + def test_persons_query_properties(self): + random_uuid = self._create_random_persons() + runner = self._create_runner( + PersonsQuery( + properties=[ + PersonPropertyFilter(key="random_uuid", value=random_uuid, operator=PropertyOperator.exact), + HogQLPropertyFilter(key="toInt(properties.index) > 5"), + ] + ) + ) + self.assertEqual(len(runner.run().results), 4) + + def test_persons_query_fixed_properties(self): + random_uuid = self._create_random_persons() + runner = self._create_runner( + PersonsQuery( + fixedProperties=[ + PersonPropertyFilter(key="random_uuid", value=random_uuid, operator=PropertyOperator.exact), + HogQLPropertyFilter(key="toInt(properties.index) < 2"), + ] + ) + ) + self.assertEqual(len(runner.run().results), 2)