mirrors/posthog
0
0
Fork 0
mirror of https://github.com/PostHog/posthog.git synced 2024-11-28 09:16:49 +01:00
Code Issues Releases Activity

Rename property group values (#8680)

Browse Source 
* initial working for trends with simplify

* simplify logic only if property_groups exists

* purge empty clauses

* fix tests

* fix snapshots

* update subquery logic

* update snapshots

* update snapshots

* columnoptimizer

* combine tests

* proof of concept simplification

* diff snapshots

* fix tests

* helpers and property_groups everywhere

* update snapshots

* fix simplify

* type

* missing argument

* add old properties back to simplify logic

* resolve merge conflicts, address few comments

* wip cleanup

* rm simplify filters

* redo removal

* update snapshots

* last snapshot

* update snapshot

* rename property group values

* missed one

* cruft

Co-authored-by: Eric <eeoneric@gmail.com>
This commit is contained in:
Neil Kakkar 2022-02-17 18:25:33 +00:00 committed by GitHub
parent 101e1c8d1a
commit 5c5d575939
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 96 additions and 96 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
ee/clickhouse/models/property.py
View File

@ -78,13 +78,13 @@ def parse_prop_grouped_clauses(
_top_level: bool = True,
) -> Tuple[str, Dict]:
if len(property_group.groups) == 0:
if len(property_group.values) == 0:
return "", {}
if isinstance(property_group.groups[0], PropertyGroup):
if isinstance(property_group.values[0], PropertyGroup):
group_clauses = []
final_params = {}
for idx, group in enumerate(property_group.groups):
for idx, group in enumerate(property_group.values):
if isinstance(group, PropertyGroup):
clause, params = parse_prop_grouped_clauses(
team_id=team_id,
@ -106,7 +106,7 @@ def parse_prop_grouped_clauses(
_final = f"{property_group.type} ".join(group_clauses)
else:
_final, final_params = parse_prop_clauses(
filters=cast(List[Property], property_group.groups),
filters=cast(List[Property], property_group.values),
prepend=f"{prepend}",
table_name=table_name,
allow_denormalized_props=allow_denormalized_props,

20
ee/clickhouse/models/test/test_property.py
View File

@ -378,12 +378,12 @@ class TestPropFormat(ClickhouseTestMixin, BaseTest):
data={
"properties": {
"type": "OR",
"groups": [
"values": [
{
"type": "AND",
"groups": [{"key": "attr_1", "value": "val_1"}, {"key": "attr_2", "value": "val_2"}],
"values": [{"key": "attr_1", "value": "val_1"}, {"key": "attr_2", "value": "val_2"}],
},
{"type": "OR", "groups": [{"key": "attr_1", "value": "val_2"}],},
{"type": "OR", "values": [{"key": "attr_1", "value": "val_2"}],},
],
}
}
@ -397,12 +397,12 @@ class TestPropFormat(ClickhouseTestMixin, BaseTest):
data={
"properties": {
"type": "OR",
"groups": [
"values": [
{
"type": "AND",
"groups": [{"key": "attr", "value": "val_1"}, {"key": "attr_2", "value": "val_2"}],
"values": [{"key": "attr", "value": "val_1"}, {"key": "attr_2", "value": "val_2"}],
},
{"type": "XOR", "groups": [{"key": "attr", "value": "val_2"}],},
{"type": "XOR", "values": [{"key": "attr", "value": "val_2"}],},
],
}
}
@ -435,9 +435,9 @@ class TestPropFormat(ClickhouseTestMixin, BaseTest):
data={
"properties": {
"type": "OR",
"groups": [
{"type": "OR", "groups": [{"key": "email", "type": "person", "value": "1@posthog.com"}],},
{"type": "OR", "groups": [{"key": "email", "type": "person", "value": "2@posthog.com"}],},
"values": [
{"type": "OR", "values": [{"key": "email", "type": "person", "value": "1@posthog.com"}],},
{"type": "OR", "values": [{"key": "email", "type": "person", "value": "2@posthog.com"}],},
],
}
}
@ -595,7 +595,7 @@ def test_parse_prop_clauses_defaults(snapshot):
@pytest.mark.django_db
def test_parse_groups_persons_edge_case_with_single_filter(snapshot):
filter = Filter(
data={"properties": {"type": "OR", "groups": [{"key": "email", "type": "person", "value": "1@posthog.com"}],}}
data={"properties": {"type": "OR", "values": [{"key": "email", "type": "person", "value": "1@posthog.com"}],}}
)
assert (
parse_prop_grouped_clauses(

4
ee/clickhouse/queries/breakdown_props.py
View File

@ -52,7 +52,7 @@ def get_breakdown_prop_values(
parsed_date_from, parsed_date_to, date_params = parse_timestamps(filter=filter, team_id=team_id)
prop_filters, prop_filter_params = parse_prop_grouped_clauses(
team_id=team_id,
property_group=PropertyGroup(type=PropertyOperatorType.AND, groups=filter.properties + entity.properties),
property_group=PropertyGroup(type=PropertyOperatorType.AND, values=filter.properties + entity.properties),
table_name="e",
prepend="e_brkdwn",
person_properties_mode=PersonPropertiesMode.EXCLUDE,
@ -135,7 +135,7 @@ def _format_all_query(team_id: int, filter: Filter, **kwargs) -> Tuple[str, Dict
prop_filters, prop_filter_params = parse_prop_grouped_clauses(
team_id=team_id,
property_group=PropertyGroup(type=PropertyOperatorType.AND, groups=props_to_filter),
property_group=PropertyGroup(type=PropertyOperatorType.AND, values=props_to_filter),
prepend="all_cohort_",
table_name="all_events",
)

2
ee/clickhouse/queries/event_query.py
View File

@ -188,7 +188,7 @@ class ClickhouseEventQuery(metaclass=ABCMeta):
else:
filter_query, filter_params = parse_prop_grouped_clauses(
team_id=self._team_id,
property_group=PropertyGroup(type=PropertyOperatorType.AND, groups=[prop]),
property_group=PropertyGroup(type=PropertyOperatorType.AND, values=[prop]),
prepend=f"global_{idx}",
allow_denormalized_props=True,
person_properties_mode=PersonPropertiesMode.EXCLUDE,

2
ee/clickhouse/queries/test/test_column_optimizer.py
View File

@ -17,7 +17,7 @@ PROPERTIES_OF_ALL_TYPES = [
BASE_FILTER = Filter({"events": [{"id": "$pageview", "type": "events", "order": 0}]})
FILTER_WITH_PROPERTIES = BASE_FILTER.with_data({"properties": PROPERTIES_OF_ALL_TYPES})
FILTER_WITH_GROUPS = BASE_FILTER.with_data({"properties": {"type": "AND", "groups": PROPERTIES_OF_ALL_TYPES}})
FILTER_WITH_GROUPS = BASE_FILTER.with_data({"properties": {"type": "AND", "values": PROPERTIES_OF_ALL_TYPES}})
class TestColumnOptimizer(ClickhouseTestMixin, APIBaseTest):

2
ee/clickhouse/queries/trends/breakdown.py
View File

@ -97,7 +97,7 @@ class ClickhouseTrendsBreakdown:
"parsed_date_to": parsed_date_to,
"actions_query": "AND {}".format(action_query) if action_query else "",
"event_filter": "AND event = %(event)s" if not action_query else "",
"filters": prop_filters if props_to_filter.groups else "",
"filters": prop_filters if props_to_filter.values else "",
}
_params, _breakdown_filter_params = {}, {}

22
posthog/models/filters/mixins/property.py
View File

@ -23,7 +23,7 @@ class PropertyMixin(BaseParamMixin):
loaded_props = _props
# if grouped properties
if isinstance(loaded_props, dict) and "type" in loaded_props and "groups" in loaded_props:
if isinstance(loaded_props, dict) and "type" in loaded_props and "values" in loaded_props:
# property_groups is main function from now on
# TODO: this function will go away at end of migration
return []
@ -44,7 +44,7 @@ class PropertyMixin(BaseParamMixin):
loaded_props = _props
# if grouped properties
if isinstance(loaded_props, dict) and "type" in loaded_props and "groups" in loaded_props:
if isinstance(loaded_props, dict) and "type" in loaded_props and "values" in loaded_props:
try:
return self._parse_property_group(loaded_props)
except ValidationError as e:
@ -53,18 +53,18 @@ class PropertyMixin(BaseParamMixin):
raise ValidationError(f"PropertyGroup is unparsable: {e}")
# old properties
return PropertyGroup(type=PropertyOperatorType.AND, groups=self.properties)
return PropertyGroup(type=PropertyOperatorType.AND, values=self.properties)
@cached_property
def property_groups_flat(self) -> List[Property]:
return list(self._property_groups_flat(self.property_groups))
def _property_groups_flat(self, prop_group: PropertyGroup):
for _group in prop_group.groups:
if isinstance(_group, PropertyGroup):
yield from self._property_groups_flat(_group)
for value in prop_group.values:
if isinstance(value, PropertyGroup):
yield from self._property_groups_flat(value)
else:
yield _group
yield value
def _parse_properties(self, properties: Optional[Any]) -> List[Property]:
if isinstance(properties, list):
@ -94,9 +94,9 @@ class PropertyMixin(BaseParamMixin):
return ret
def _parse_property_group(self, group: Optional[Dict]) -> PropertyGroup:
if group and "type" in group and "groups" in group:
if group and "type" in group and "values" in group:
return PropertyGroup(
PropertyOperatorType(group["type"].upper()), self._parse_property_group_list(group["groups"])
PropertyOperatorType(group["type"].upper()), self._parse_property_group_list(group["values"])
)
return PropertyGroup(PropertyOperatorType.AND, cast(List[Property], []))
@ -108,7 +108,7 @@ class PropertyMixin(BaseParamMixin):
has_property_groups = False
has_simple_properties = False
for prop in prop_list:
if "type" in prop and "groups" in prop:
if "type" in prop and "values" in prop:
has_property_groups = True
else:
has_simple_properties = True
@ -127,5 +127,5 @@ class PropertyMixin(BaseParamMixin):
return {PROPERTIES: [prop.to_dict() for prop in self.properties]}
return (
{PROPERTIES: self.property_groups.to_dict()} if self.property_groups and self.property_groups.groups else {}
{PROPERTIES: self.property_groups.to_dict()} if self.property_groups and self.property_groups.values else {}
)

118
posthog/models/filters/mixins/test/test_property.py
View File

@ -12,32 +12,32 @@ def test_property_group_multi_level_parsing():
data={
"properties": {
"type": "AND",
"groups": [
"values": [
{
"type": "AND",
"groups": [{"key": "attr", "value": "val_1"}, {"key": "attr_2", "value": "val_2"}],
"values": [{"key": "attr", "value": "val_1"}, {"key": "attr_2", "value": "val_2"}],
},
{"type": "OR", "groups": [{"key": "attr", "value": "val_2"}]},
{"type": "OR", "values": [{"key": "attr", "value": "val_2"}]},
],
}
}
)
assert filter.property_groups.type == "AND"
assert isinstance(filter.property_groups.groups[0], PropertyGroup)
assert filter.property_groups.groups[0].type == "AND"
assert isinstance(filter.property_groups.groups[0].groups[0], Property)
assert filter.property_groups.groups[0].groups[0].key == "attr"
assert filter.property_groups.groups[0].groups[0].value == "val_1"
assert isinstance(filter.property_groups.groups[0].groups[1], Property)
assert filter.property_groups.groups[0].groups[1].key == "attr_2"
assert filter.property_groups.groups[0].groups[1].value == "val_2"
assert isinstance(filter.property_groups.values[0], PropertyGroup)
assert filter.property_groups.values[0].type == "AND"
assert isinstance(filter.property_groups.values[0].values[0], Property)
assert filter.property_groups.values[0].values[0].key == "attr"
assert filter.property_groups.values[0].values[0].value == "val_1"
assert isinstance(filter.property_groups.values[0].values[1], Property)
assert filter.property_groups.values[0].values[1].key == "attr_2"
assert filter.property_groups.values[0].values[1].value == "val_2"
assert isinstance(filter.property_groups.groups[1], PropertyGroup)
assert filter.property_groups.groups[1].type == "OR"
assert isinstance(filter.property_groups.groups[1].groups[0], Property)
assert filter.property_groups.groups[1].groups[0].key == "attr"
assert filter.property_groups.groups[1].groups[0].value == "val_2"
assert isinstance(filter.property_groups.values[1], PropertyGroup)
assert filter.property_groups.values[1].type == "OR"
assert isinstance(filter.property_groups.values[1].values[0], Property)
assert filter.property_groups.values[1].values[0].key == "attr"
assert filter.property_groups.values[1].values[0].value == "val_2"
def test_property_group_simple_parsing():
@ -45,25 +45,25 @@ def test_property_group_simple_parsing():
data={
"properties": {
"type": "AND",
"groups": [{"key": "attr", "value": "val_1"}, {"key": "attr_2", "value": "val_2"}],
"values": [{"key": "attr", "value": "val_1"}, {"key": "attr_2", "value": "val_2"}],
}
}
)
assert filter.property_groups.type == "AND"
assert isinstance(filter.property_groups.groups[0], Property)
assert filter.property_groups.groups[0].key == "attr"
assert filter.property_groups.groups[0].value == "val_1"
assert isinstance(filter.property_groups.groups[1], Property)
assert filter.property_groups.groups[1].key == "attr_2"
assert filter.property_groups.groups[1].value == "val_2"
assert isinstance(filter.property_groups.values[0], Property)
assert filter.property_groups.values[0].key == "attr"
assert filter.property_groups.values[0].value == "val_1"
assert isinstance(filter.property_groups.values[1], Property)
assert filter.property_groups.values[1].key == "attr_2"
assert filter.property_groups.values[1].value == "val_2"
def test_property_group_empty_parsing():
filter = Filter(data={"properties": {}})
assert filter.property_groups.type == "AND"
assert filter.property_groups.groups == []
assert filter.property_groups.values == []
def test_property_group_invalid_parsing():
@ -72,7 +72,7 @@ def test_property_group_invalid_parsing():
data={
"properties": {
"type": "XaND",
"groups": [{"key": "attr", "value": "val_1"}, {"key": "attr_2", "value": "val_2"},],
"values": [{"key": "attr", "value": "val_1"}, {"key": "attr_2", "value": "val_2"},],
}
}
)
@ -87,11 +87,11 @@ def test_property_group_includes_unhomogenous_groups():
data={
"properties": {
"type": "AND",
"groups": [
{"type": "or", "groups": [{"key": "attr", "value": "val_1"}]},
"values": [
{"type": "or", "values": [{"key": "attr", "value": "val_1"}]},
{"key": "attr", "value": "val_1"},
{"key": "attr_2", "value": "val_2"},
{"type": "OR", "groups": []},
{"type": "OR", "values": []},
],
}
}
@ -106,12 +106,12 @@ def test_property_multi_level_to_dict():
data={
"properties": {
"type": "AND",
"groups": [
"values": [
{
"type": "AND",
"groups": [{"key": "attr", "value": "val_1"}, {"key": "attr_2", "value": "val_2"}],
"values": [{"key": "attr", "value": "val_1"}, {"key": "attr_2", "value": "val_2"}],
},
{"type": "OR", "groups": [{"key": "attr", "value": "val_2"}]},
{"type": "OR", "values": [{"key": "attr", "value": "val_2"}]},
],
}
}
@ -119,15 +119,15 @@ def test_property_multi_level_to_dict():
assert filter.property_groups.to_dict() == {
"type": "AND",
"groups": [
"values": [
{
"type": "AND",
"groups": [
"values": [
{"key": "attr", "value": "val_1", "operator": None, "type": "event"},
{"key": "attr_2", "value": "val_2", "operator": None, "type": "event"},
],
},
{"type": "OR", "groups": [{"key": "attr", "value": "val_2", "operator": None, "type": "event"}]},
{"type": "OR", "values": [{"key": "attr", "value": "val_2", "operator": None, "type": "event"}]},
],
}
@ -137,14 +137,14 @@ def test_property_group_simple_to_dict():
data={
"properties": {
"type": "AND",
"groups": [{"key": "attr", "value": "val_1"}, {"key": "attr_2", "value": "val_2"}],
"values": [{"key": "attr", "value": "val_1"}, {"key": "attr_2", "value": "val_2"}],
}
}
)
assert filter.property_groups.to_dict() == {
"type": "AND",
"groups": [
"values": [
{"key": "attr", "value": "val_1", "operator": None, "type": "event"},
{"key": "attr_2", "value": "val_2", "operator": None, "type": "event"},
],
@ -155,19 +155,19 @@ def test_property_group_simple_json_parsing():
filter = Filter(
data={
"properties": json.dumps(
{"type": "AND", "groups": [{"key": "attr", "value": "val_1"}, {"key": "attr_2", "value": "val_2"}],}
{"type": "AND", "values": [{"key": "attr", "value": "val_1"}, {"key": "attr_2", "value": "val_2"}],}
)
}
)
assert filter.property_groups.type == "AND"
assert isinstance(filter.property_groups.groups[0], Property)
assert filter.property_groups.groups[0].key == "attr"
assert filter.property_groups.groups[0].value == "val_1"
assert isinstance(filter.property_groups.groups[1], Property)
assert filter.property_groups.groups[1].key == "attr_2"
assert filter.property_groups.groups[1].value == "val_2"
assert isinstance(filter.property_groups.values[0], Property)
assert filter.property_groups.values[0].key == "attr"
assert filter.property_groups.values[0].value == "val_1"
assert isinstance(filter.property_groups.values[1], Property)
assert filter.property_groups.values[1].key == "attr_2"
assert filter.property_groups.values[1].value == "val_2"
def test_property_group_multi_level_json_parsing():
@ -176,12 +176,12 @@ def test_property_group_multi_level_json_parsing():
"properties": json.dumps(
{
"type": "AND",
"groups": [
"values": [
{
"type": "AND",
"groups": [{"key": "attr", "value": "val_1"}, {"key": "attr_2", "value": "val_2"}],
"values": [{"key": "attr", "value": "val_1"}, {"key": "attr_2", "value": "val_2"}],
},
{"type": "OR", "groups": [{"key": "attr", "value": "val_2"}]},
{"type": "OR", "values": [{"key": "attr", "value": "val_2"}]},
],
}
)
@ -189,18 +189,18 @@ def test_property_group_multi_level_json_parsing():
)
assert filter.property_groups.type == "AND"
assert isinstance(filter.property_groups.groups[0], PropertyGroup)
assert filter.property_groups.groups[0].type == "AND"
assert isinstance(filter.property_groups.values[0], PropertyGroup)
assert filter.property_groups.values[0].type == "AND"
assert isinstance(filter.property_groups.groups[0].groups[0], Property)
assert filter.property_groups.groups[0].groups[0].key == "attr"
assert filter.property_groups.groups[0].groups[0].value == "val_1"
assert isinstance(filter.property_groups.groups[0].groups[1], Property)
assert filter.property_groups.groups[0].groups[1].key == "attr_2"
assert filter.property_groups.groups[0].groups[1].value == "val_2"
assert isinstance(filter.property_groups.values[0].values[0], Property)
assert filter.property_groups.values[0].values[0].key == "attr"
assert filter.property_groups.values[0].values[0].value == "val_1"
assert isinstance(filter.property_groups.values[0].values[1], Property)
assert filter.property_groups.values[0].values[1].key == "attr_2"
assert filter.property_groups.values[0].values[1].value == "val_2"
assert isinstance(filter.property_groups.groups[1], PropertyGroup)
assert filter.property_groups.groups[1].type == "OR"
assert isinstance(filter.property_groups.groups[1].groups[0], Property)
assert filter.property_groups.groups[1].groups[0].key == "attr"
assert filter.property_groups.groups[1].groups[0].value == "val_2"
assert isinstance(filter.property_groups.values[1], PropertyGroup)
assert filter.property_groups.values[1].type == "OR"
assert isinstance(filter.property_groups.values[1].values[0], Property)
assert filter.property_groups.values[1].values[0].key == "attr"
assert filter.property_groups.values[1].values[0].value == "val_2"

14
posthog/models/property.py
View File

@ -146,28 +146,28 @@ def lookup_q(key: str, value: Any) -> Q:
class PropertyGroup:
type: PropertyOperatorType
groups: Union[List[Property], List["PropertyGroup"]]
values: Union[List[Property], List["PropertyGroup"]]
def __init__(self, type: PropertyOperatorType, groups: Union[List[Property], List["PropertyGroup"]]) -> None:
def __init__(self, type: PropertyOperatorType, values: Union[List[Property], List["PropertyGroup"]]) -> None:
self.type = type
self.groups = groups
self.values = values
def combine_properties(self, operator: PropertyOperatorType, properties: List[Property]) -> "PropertyGroup":
if not properties:
return self
if len(self.groups) == 0:
if len(self.values) == 0:
return PropertyGroup(PropertyOperatorType.AND, properties)
return PropertyGroup(operator, [self, PropertyGroup(PropertyOperatorType.AND, properties)])
def to_dict(self):
result: Dict = {}
if not self.groups:
if not self.values:
return result
return {"type": self.type, "groups": [prop.to_dict() for prop in self.groups]}
return {"type": self.type, "values": [prop.to_dict() for prop in self.values]}
def __repr__(self):
params_repr = ", ".join(f"{repr(prop)}" for prop in self.groups)
params_repr = ", ".join(f"{repr(prop)}" for prop in self.values)
return f"PropertyGroup(type={self.type}-{params_repr})"