diff --git a/posthog/hogql/property.py b/posthog/hogql/property.py index 70c7a7bf849..e372b446b5c 100644 --- a/posthog/hogql/property.py +++ b/posthog/hogql/property.py @@ -238,44 +238,6 @@ def _expr_to_compare_op( raise NotImplementedError(f"PropertyOperator {operator} not implemented") -def _handle_property_values( - property: Property, - value: list, - team: Team, - scope: str, - field: ast.Expr, - operator: PropertyOperator, - is_json_field: bool, -) -> ast.Expr: - if len(value) == 0: - return ast.Constant(value=1) - elif len(value) == 1: - return _expr_to_compare_op( - expr=field, - value=value[0], - operator=operator, - team=team, - property=property, - is_json_field=is_json_field, - ) - else: - # Using an AND here instead of `in()` or `notIn()`, due to Clickhouses poor handling of `null` values - exprs = [ - _expr_to_compare_op( - expr=field, - value=v, - operator=operator, - team=team, - property=property, - is_json_field=is_json_field, - ) - for v in value - ] - if operator in (PropertyOperator.NOT_ICONTAINS, PropertyOperator.NOT_REGEX, PropertyOperator.IS_NOT): - return ast.And(exprs=exprs) - return ast.Or(exprs=exprs) - - def property_to_expr( property: ( list @@ -385,34 +347,33 @@ def property_to_expr( elif property.type == "session" and scope == "replay_entity": chain = ["events", "session"] elif property.type == "data_warehouse": - if isinstance(property.key, str): + if not isinstance(property.key, str): + raise QueryError("Data warehouse property filter value must be a string") + else: split = property.key.split(".") chain = split[:-1] - property_key = split[-1] - else: - raise QueryError("Data warehouse property filter value must be a string") - - field = ast.Field(chain=[*chain, property_key]) - if isinstance(value, list): - return _handle_property_values( - property=property, - value=value, - team=team, - scope=scope, - field=field, - operator=operator, - is_json_field=False, - ) - - return _expr_to_compare_op( - expr=field, - value=value, - operator=operator, - team=team, - property=property, - is_json_field=False, - ) + property.key = split[-1] + if isinstance(value, list) and len(value) > 1: + field = ast.Field(chain=[*chain, property.key]) + exprs = [ + _expr_to_compare_op( + expr=field, + value=v, + operator=operator, + team=team, + property=property, + is_json_field=False, + ) + for v in value + ] + if ( + operator == PropertyOperator.NOT_ICONTAINS + or operator == PropertyOperator.NOT_REGEX + or operator == PropertyOperator.IS_NOT + ): + return ast.And(exprs=exprs) + return ast.Or(exprs=exprs) elif property.type == "data_warehouse_person_property": if isinstance(property.key, str): table, key = property.key.split(".") @@ -438,15 +399,33 @@ def property_to_expr( expr = ast.Call(name="argMinMerge", args=[field]) if isinstance(value, list): - return _handle_property_values( - property=property, - value=value, - team=team, - scope=scope, - field=expr, - operator=operator, - is_json_field=property.type != "session", - ) + if len(value) == 0: + return ast.Constant(value=1) + elif len(value) == 1: + value = value[0] + else: + # Using an AND here instead of `in()` or `notIn()`, due to Clickhouses poor handling of `null` values + exprs = [ + property_to_expr( + Property( + type=property.type, + key=property.key, + operator=property.operator, + group_type_index=property.group_type_index, + value=v, + ), + team, + scope, + ) + for v in value + ] + if ( + operator == PropertyOperator.NOT_ICONTAINS + or operator == PropertyOperator.NOT_REGEX + or operator == PropertyOperator.IS_NOT + ): + return ast.And(exprs=exprs) + return ast.Or(exprs=exprs) return _expr_to_compare_op( expr=expr,