Update person property filtering (#2024)

* change person prop base and fix cohort query

* try clear signal

* remove signal

ee/clickhouse/models/cohort.py

@@ -20,23 +20,32 @@ def format_filter_query(cohort: Cohort) -> Tuple[str, Dict[str, Any]]:
 
         elif group.get("properties"):
             filter = Filter(data=group)
+            props = []
 
             for idx, prop in enumerate(filter.properties):
                 prepend = "{}_cohort_group_{}".format(cohort.pk, group_idx)
 
                 arg = "v{}_{}".format(prepend, idx)
                 operator_clause, value = get_operator(prop, arg)
 
-                prop_filters = "(ep.key = %(k{prepend}_{idx})s) AND {operator_clause}".format(
-                    idx=idx, operator_clause=operator_clause, prepend=prepend
+                key_statement = "(ep.key = %(k{prepend}_{idx})s)".format(idx=idx, prepend=prepend)
+                prop_filters = "{key_statement} AND {operator_clause}".format(
+                    key_statement=key_statement, operator_clause=operator_clause
                 )
                 clause = GET_DISTINCT_IDS_BY_PROPERTY_SQL.format(
-                    filters=prop_filters, negation="NOT " if prop.operator and "not" in prop.operator else ""
+                    key_statement=key_statement,
+                    filters=prop_filters,
+                    negation="NOT " if prop.operator and "not" in prop.operator else "",
                 )
 
-                filters.append("(" + clause + ")")
+                props.append("(" + clause + ")")
                 params.update({"k{}_{}".format(prepend, idx): prop.key, arg: value})
 
+            prop_separator = " AND distinct_id IN "
+            joined_prop_filter = prop_separator.join(props)
+            final_prop_query = CALCULATE_COHORT_PEOPLE_SQL.format(query=joined_prop_filter)
+            filters.append("(" + final_prop_query + ")")
+
     separator = " OR distinct_id IN "
     joined_filter = separator.join(filters)
     person_id_query = CALCULATE_COHORT_PEOPLE_SQL.format(query=joined_filter)

ee/clickhouse/models/property.py

@@ -28,14 +28,16 @@ def parse_prop_clauses(key: str, filters: List[Property], team: Team, prepend: s
             arg = "v{}_{}".format(prepend, idx)
             operator_clause, value = get_operator(prop, arg)
 
-            filter = "(ep.key = %(k{prepend}_{idx})s) {and_statement} {operator_clause}".format(
-                idx=idx,
+            key_statement = "(ep.key = %(k{prepend}_{idx})s)".format(idx=idx, prepend=prepend)
+            filter = "{key_statement} {and_statement} {operator_clause}".format(
+                key_statement=key_statement,
                 and_statement="AND" if operator_clause else "",
                 operator_clause=operator_clause,
-                prepend=prepend,
             )
             clause = GET_DISTINCT_IDS_BY_PROPERTY_SQL.format(
-                filters=filter, negation="NOT " if prop.operator and "not" in prop.operator else ""
+                key_statement=key_statement,
+                filters=filter,
+                negation="NOT " if prop.operator and "not" in prop.operator else "",
             )
             final += "AND distinct_id IN ({clause}) ".format(clause=clause)
             params.update({"k{}_{}".format(prepend, idx): prop.key, arg: value})

ee/clickhouse/models/test/test_property.py

@@ -9,6 +9,7 @@
 from posthog.models.event import Event
 from posthog.models.filter import Filter
 from posthog.models.person import Person
+from posthog.models.team import Team
 
 
 def _create_event(**kwargs) -> Event:
@@ -24,11 +25,15 @@ def _create_person(**kwargs) -> Person:
 
 
 class TestPropFormat(ClickhouseTestMixin, BaseTest):
-    def test_prop_cohort(self):
+    def test_prop_cohort_basic(self):
 
         _create_person(distinct_ids=["some_other_id"], team_id=self.team.pk, properties={"$some_prop": "something"})
 
-        _create_person(distinct_ids=["some_id"], team_id=self.team.pk, properties={"$another_prop": "something"})
+        _create_person(
+            distinct_ids=["some_id"],
+            team_id=self.team.pk,
+            properties={"$some_prop": "something", "$another_prop": "something"},
+        )
         _create_event(
             event="$pageview", team=self.team, distinct_id="some_id", properties={"attr": "some_val"},
         )
@@ -45,11 +50,59 @@ def test_prop_cohort(self):
 
         filter = Filter(data={"properties": [{"key": "id", "value": cohort1.pk, "type": "cohort"}],})
         query, params = parse_prop_clauses("uuid", filter.properties, self.team)
+        final_query = "SELECT uuid FROM events WHERE team_id = %(team_id)s {}".format(query)
+        result = sync_execute(final_query, {**params, "team_id": self.team.pk})
+        self.assertEqual(len(result), 1)
+
+    def test_prop_cohort_multiple_groups(self):
+
+        _create_person(distinct_ids=["some_other_id"], team_id=self.team.pk, properties={"$some_prop": "something"})
+
+        _create_person(distinct_ids=["some_id"], team_id=self.team.pk, properties={"$another_prop": "something"})
+        _create_event(
+            event="$pageview", team=self.team, distinct_id="some_id", properties={"attr": "some_val"},
+        )
+
+        _create_event(
+            event="$pageview", team=self.team, distinct_id="some_other_id", properties={"attr": "some_val"},
+        )
+
+        cohort1 = Cohort.objects.create(
+            team=self.team,
+            groups=[{"properties": {"$some_prop": "something"}}, {"properties": {"$another_prop": "something"}}],
+            name="cohort1",
+        )
 
+        filter = Filter(data={"properties": [{"key": "id", "value": cohort1.pk, "type": "cohort"}],})
+        query, params = parse_prop_clauses("uuid", filter.properties, self.team)
         final_query = "SELECT uuid FROM events WHERE team_id = %(team_id)s {}".format(query)
         result = sync_execute(final_query, {**params, "team_id": self.team.pk})
         self.assertEqual(len(result), 2)
 
+    def test_prop_cohort_with_negation(self):
+        team2 = Team.objects.create()
+
+        _create_person(distinct_ids=["some_other_id"], team_id=self.team.pk, properties={"$some_prop": "something"})
+
+        _create_person(distinct_ids=["some_id"], team_id=team2.pk, properties={"$another_prop": "something"})
+        _create_event(
+            event="$pageview", team=self.team, distinct_id="some_id", properties={"attr": "some_val"},
+        )
+
+        _create_event(
+            event="$pageview", team=self.team, distinct_id="some_other_id", properties={"attr": "some_val"},
+        )
+
+        cohort1 = Cohort.objects.create(
+            team=self.team, groups=[{"properties": {"$some_prop__is_not": "something"}}], name="cohort1",
+        )
+
+        filter = Filter(data={"properties": [{"key": "id", "value": cohort1.pk, "type": "cohort"}],})
+        query, params = parse_prop_clauses("uuid", filter.properties, self.team)
+        final_query = "SELECT uuid FROM events WHERE team_id = %(team_id)s {}".format(query)
+        result = sync_execute(final_query, {**params, "team_id": self.team.pk})
+        self.assertEqual(len(result), 0)
+
     def test_prop_person(self):
 
         _create_person(

ee/clickhouse/sql/person.py

@@ -345,7 +345,15 @@
 SELECT distinct_id FROM person_distinct_id WHERE person_id {negation}IN 
 (
     SELECT id
-    FROM persons_properties_up_to_date_view AS ep
+    FROM persons_properties_view ep JOIN
+    (
+        SELECT id, key, max(created_at) created_at
+        FROM
+        persons_properties_view as ep
+        WHERE {key_statement} AND team_id = %(team_id)s
+        GROUP BY id, key
+    ) latest
+        ON ep.id = latest.id AND ep.created_at = latest.created_at
     WHERE {filters} AND team_id = %(team_id)s
-)
+) AND team_id = %(team_id)s
 """