fix(hogql): reverse join order for actors (#21216)

PostHog · Apr 2, 2024 · edb3a46 · edb3a46
1 parent 25db327
commit edb3a46
Show file tree

Hide file tree

Showing 10 changed files with 447 additions and 445 deletions.
diff --git a/posthog/hogql/test/test_resolver.py b/posthog/hogql/test/test_resolver.py
@@ -359,10 +359,12 @@ def test_visit_hogqlx_tag_source(self):
         node = cast(ast.SelectQuery, resolve_types(self._select(query), self.context, dialect="hogql"))
         hogql = print_prepared_ast(node, HogQLContext(team_id=self.team.pk, enable_select_queries=True), "hogql")
         expected = (
-            f"SELECT id, email FROM "
-            f"(SELECT id, properties.email AS email FROM persons INNER JOIN "
-            f"(SELECT DISTINCT person_id FROM events) "
-            f"AS source ON equals(persons.id, source.person_id) ORDER BY id ASC) "
-            f"LIMIT 10000"
+            "SELECT id, email FROM "
+            "(SELECT id, properties.email AS email FROM "
+            "(SELECT DISTINCT person_id FROM events) "
+            "AS source INNER JOIN "
+            "persons ON equals(persons.id, source.person_id) ORDER BY id ASC) "
+            "LIMIT 10000"
         )
+
         assert hogql == expected
diff --git a/posthog/hogql_queries/actors_query_runner.py b/posthog/hogql_queries/actors_query_runner.py
@@ -142,11 +142,11 @@ def source_table_join(self) -> ast.JoinExpr:
         source_alias = "source"
 
         return ast.JoinExpr(
-            table=ast.Field(chain=[self.strategy.origin]),
+            table=source_query,
+            alias=source_alias,
             next_join=ast.JoinExpr(
-                table=source_query,
+                table=ast.Field(chain=[self.strategy.origin]),
                 join_type="INNER JOIN",
-                alias=source_alias,
                 constraint=ast.JoinConstraint(
                     expr=ast.CompareOperation(
                         op=ast.CompareOperationOp.Eq,

diff --git a/posthog/hogql_queries/insights/funnels/test/__snapshots__/test_funnel.ambr b/posthog/hogql_queries/insights/funnels/test/__snapshots__/test_funnel.ambr
@@ -98,13 +98,6 @@
          toTimeZone(persons.created_at, 'UTC') AS created_at,
          1
   FROM
-    (SELECT argMax(person.created_at, person.version) AS created_at,
-            person.id AS id
-     FROM person
-     WHERE equals(person.team_id, 2)
-     GROUP BY person.id
-     HAVING ifNull(equals(argMax(person.is_deleted, person.version), 0), 0) SETTINGS optimize_aggregation_in_order=1) AS persons
-  INNER JOIN
     (SELECT aggregation_target AS actor_id
      FROM
        (SELECT aggregation_target AS aggregation_target,
@@ -185,7 +178,14 @@
         HAVING ifNull(equals(steps, max_steps), isNull(steps)
                       and isNull(max_steps)))
      WHERE ifNull(in(steps, [2, 3]), 0)
-     ORDER BY aggregation_target ASC) AS source ON equals(persons.id, source.actor_id)
+     ORDER BY aggregation_target ASC) AS source
+  INNER JOIN
+    (SELECT argMax(person.created_at, person.version) AS created_at,
+            person.id AS id
+     FROM person
+     WHERE equals(person.team_id, 2)
+     GROUP BY person.id
+     HAVING ifNull(equals(argMax(person.is_deleted, person.version), 0), 0) SETTINGS optimize_aggregation_in_order=1) AS persons ON equals(persons.id, source.actor_id)
   ORDER BY toTimeZone(persons.created_at, 'UTC') DESC
   LIMIT 101
   OFFSET 0 SETTINGS readonly=2,
@@ -493,13 +493,6 @@
          toTimeZone(persons.created_at, 'UTC') AS created_at,
          1
   FROM
-    (SELECT argMax(person.created_at, person.version) AS created_at,
-            person.id AS id
-     FROM person
-     WHERE equals(person.team_id, 2)
-     GROUP BY person.id
-     HAVING ifNull(equals(argMax(person.is_deleted, person.version), 0), 0) SETTINGS optimize_aggregation_in_order=1) AS persons
-  INNER JOIN
     (SELECT aggregation_target AS actor_id
      FROM
        (SELECT aggregation_target AS aggregation_target,
@@ -592,7 +585,14 @@
         HAVING ifNull(equals(steps, max_steps), isNull(steps)
                       and isNull(max_steps)))
      WHERE ifNull(in(steps, [1, 2, 3]), 0)
-     ORDER BY aggregation_target ASC) AS source ON equals(persons.id, source.actor_id)
+     ORDER BY aggregation_target ASC) AS source
+  INNER JOIN
+    (SELECT argMax(person.created_at, person.version) AS created_at,
+            person.id AS id
+     FROM person
+     WHERE equals(person.team_id, 2)
+     GROUP BY person.id
+     HAVING ifNull(equals(argMax(person.is_deleted, person.version), 0), 0) SETTINGS optimize_aggregation_in_order=1) AS persons ON equals(persons.id, source.actor_id)
   ORDER BY toTimeZone(persons.created_at, 'UTC') DESC
   LIMIT 101
   OFFSET 0 SETTINGS readonly=2,
@@ -607,13 +607,6 @@
          toTimeZone(persons.created_at, 'UTC') AS created_at,
          1
   FROM
-    (SELECT argMax(person.created_at, person.version) AS created_at,
-            person.id AS id
-     FROM person
-     WHERE equals(person.team_id, 2)
-     GROUP BY person.id
-     HAVING ifNull(equals(argMax(person.is_deleted, person.version), 0), 0) SETTINGS optimize_aggregation_in_order=1) AS persons
-  INNER JOIN
     (SELECT aggregation_target AS actor_id
      FROM
        (SELECT aggregation_target AS aggregation_target,
@@ -706,7 +699,14 @@
         HAVING ifNull(equals(steps, max_steps), isNull(steps)
                       and isNull(max_steps)))
      WHERE ifNull(in(steps, [2, 3]), 0)
-     ORDER BY aggregation_target ASC) AS source ON equals(persons.id, source.actor_id)
+     ORDER BY aggregation_target ASC) AS source
+  INNER JOIN
+    (SELECT argMax(person.created_at, person.version) AS created_at,
+            person.id AS id
+     FROM person
+     WHERE equals(person.team_id, 2)
+     GROUP BY person.id
+     HAVING ifNull(equals(argMax(person.is_deleted, person.version), 0), 0) SETTINGS optimize_aggregation_in_order=1) AS persons ON equals(persons.id, source.actor_id)
   ORDER BY toTimeZone(persons.created_at, 'UTC') DESC
   LIMIT 101
   OFFSET 0 SETTINGS readonly=2,
@@ -721,13 +721,6 @@
          toTimeZone(persons.created_at, 'UTC') AS created_at,
          1
   FROM
-    (SELECT argMax(person.created_at, person.version) AS created_at,
-            person.id AS id
-     FROM person
-     WHERE equals(person.team_id, 2)
-     GROUP BY person.id
-     HAVING ifNull(equals(argMax(person.is_deleted, person.version), 0), 0) SETTINGS optimize_aggregation_in_order=1) AS persons
-  INNER JOIN
     (SELECT aggregation_target AS actor_id
      FROM
        (SELECT aggregation_target AS aggregation_target,
@@ -820,7 +813,14 @@
         HAVING ifNull(equals(steps, max_steps), isNull(steps)
                       and isNull(max_steps)))
      WHERE ifNull(in(steps, [3]), 0)
-     ORDER BY aggregation_target ASC) AS source ON equals(persons.id, source.actor_id)
+     ORDER BY aggregation_target ASC) AS source
+  INNER JOIN
+    (SELECT argMax(person.created_at, person.version) AS created_at,
+            person.id AS id
+     FROM person
+     WHERE equals(person.team_id, 2)
+     GROUP BY person.id
+     HAVING ifNull(equals(argMax(person.is_deleted, person.version), 0), 0) SETTINGS optimize_aggregation_in_order=1) AS persons ON equals(persons.id, source.actor_id)
   ORDER BY toTimeZone(persons.created_at, 'UTC') DESC
   LIMIT 101
   OFFSET 0 SETTINGS readonly=2,