Fix subquery order in query builders

lib/ecto/query/builder/distinct.ex

@@ -69,7 +69,7 @@ defmodule Ecto.Query.Builder.Distinct do
     distinct = quote do: %Ecto.Query.ByExpr{
                            expr: unquote(expr),
                            params: unquote(params),
-                           subqueries: unquote(acc.subqueries),
+                           subqueries: unquote(Enum.reverse(acc.subqueries)),
                            file: unquote(env.file),
                            line: unquote(env.line)}
     Builder.apply_query(query, __MODULE__, [distinct], env)

lib/ecto/query/builder/group_by.ex

@@ -49,8 +49,14 @@ defmodule Ecto.Query.Builder.GroupBy do
   Shared between group_by and partition_by.
   """
   def group_or_partition_by!(kind, query, exprs, params) do
+    {expr, params, subqueries} = group_or_partition_by!(kind, query, exprs, params, [])
+    {expr, params, Enum.reverse(subqueries)}
+  end
+
+  @doc false
+  def group_or_partition_by!(kind, query, exprs, params, subqueries) do
     {expr, {params, _, subqueries}} =
-      Enum.map_reduce(List.wrap(exprs), {params, length(params), []}, fn
+      Enum.map_reduce(List.wrap(exprs), {params, length(params), subqueries}, fn
         field, params_count when is_atom(field) ->
           {to_field(field), params_count}
 
@@ -99,7 +105,7 @@ defmodule Ecto.Query.Builder.GroupBy do
     group_by = quote do: %Ecto.Query.ByExpr{
                            expr: unquote(expr),
                            params: unquote(params),
-                           subqueries: unquote(acc.subqueries),
+                           subqueries: unquote(Enum.reverse(acc.subqueries)),
                            file: unquote(env.file),
                            line: unquote(env.line)}
     Builder.apply_query(query, __MODULE__, [group_by], env)

lib/ecto/query/builder/order_by.ex

@@ -159,8 +159,14 @@ defmodule Ecto.Query.Builder.OrderBy do
   Shared between order_by and distinct.
   """
   def order_by_or_distinct!(kind, query, exprs, params) do
+    {expr, params, subqueries} = order_by_or_distinct!(kind, query, exprs, params, [])
+    {expr, params, Enum.reverse(subqueries)}
+  end
+
+  @doc false
+  def order_by_or_distinct!(kind, query, exprs, params, subqueries) do
     {expr, {params, _, subqueries}} =
-      Enum.map_reduce(List.wrap(exprs), {params, length(params), []}, fn
+      Enum.map_reduce(List.wrap(exprs), {params, length(params), subqueries}, fn
         {dir, expr}, params_count when dir in @directions ->
           {expr, params} = dynamic_or_field!(kind, expr, query, params_count)
           {{dir, expr}, params}
@@ -248,7 +254,7 @@ defmodule Ecto.Query.Builder.OrderBy do
       quote do: %Ecto.Query.ByExpr{
               expr: unquote(expr),
               params: unquote(params),
-              subqueries: unquote(acc.subqueries),
+              subqueries: unquote(Enum.reverse(acc.subqueries)),
               file: unquote(env.file),
               line: unquote(env.line)
             }

lib/ecto/query/builder/windows.ex

@@ -148,7 +148,7 @@ defmodule Ecto.Query.Builder.Windows do
        %Ecto.Query.ByExpr{
          expr: unquote(compile_acc),
          params: unquote(params),
-         subqueries: unquote(acc.subqueries),
+         subqueries: unquote(Enum.reverse(acc.subqueries)),
          file: unquote(env.file),
          line: unquote(env.line)
        }
@@ -166,23 +166,23 @@ defmodule Ecto.Query.Builder.Windows do
     windows =
       Enum.map(runtime, fn {name, compile_acc, runtime_acc, params, escape_acc} ->
         {{acc, subqueries}, params} = do_runtime_window!(runtime_acc, query, {compile_acc, escape_acc.subqueries}, params)
-        expr = %Ecto.Query.ByExpr{expr: Enum.reverse(acc), params: Enum.reverse(params), file: file, line: line, subqueries: subqueries}
+        expr = %Ecto.Query.ByExpr{expr: Enum.reverse(acc), params: Enum.reverse(params), file: file, line: line, subqueries: Enum.reverse(subqueries)}
         {name, expr}
       end)
 
     apply(query, windows)
   end
 
   defp do_runtime_window!([{:order_by, order_by} | kw], query, {acc, subqueries_acc}, params) do
-    {order_by, params, subqueries} = OrderBy.order_by_or_distinct!(:order_by, query, order_by, params)
+    {order_by, params, subqueries} = OrderBy.order_by_or_distinct!(:order_by, query, order_by, params, subqueries_acc)
 
-    do_runtime_window!(kw, query, {[{:order_by, order_by} | acc], subqueries_acc ++ subqueries}, params)
+    do_runtime_window!(kw, query, {[{:order_by, order_by} | acc], subqueries}, params)
   end
 
   defp do_runtime_window!([{:partition_by, partition_by} | kw], query, {acc, subqueries_acc}, params) do
-    {partition_by, params, subqueries} = GroupBy.group_or_partition_by!(:partition_by, query, partition_by, params)
+    {partition_by, params, subqueries} = GroupBy.group_or_partition_by!(:partition_by, query, partition_by, params, subqueries_acc)
 
-    do_runtime_window!(kw, query, {[{:partition_by, partition_by} | acc], subqueries_acc ++ subqueries}, params)
+    do_runtime_window!(kw, query, {[{:partition_by, partition_by} | acc], subqueries}, params)
   end
 
   defp do_runtime_window!([{:frame, frame} | kw], query, {acc, subqueries_acc}, params) do

test/ecto/query/builder/distinct_test.exs

@@ -6,6 +6,10 @@ defmodule Ecto.Query.Builder.DistinctTest do
 
   import Ecto.Query
 
+  defp subquery_sources(%{subqueries: subqueries}) do
+    Enum.map(subqueries, fn subquery -> elem(subquery.query.from.source, 0) end)
+  end
+
   describe "escape" do
     test "handles expressions and params" do
       assert {true, {[], %{}}} ==
@@ -79,6 +83,20 @@ defmodule Ecto.Query.Builder.DistinctTest do
       assert distinct.expr == [asc: {:exists, [], [subquery: 0]}]
       assert [_] = distinct.subqueries
 
+      distinct = [
+        asc: dynamic([p], exists(from f in "foos", where: f.id == parent_as(:p).id)),
+        desc: dynamic([p], exists(from b in "bars", where: b.id == parent_as(:p).id))
+      ]
+
+      %{distinct: distinct} = from p in "posts", as: :p, distinct: ^distinct
+
+      assert distinct.expr == [
+               asc: {:exists, [], [subquery: 0]},
+               desc: {:exists, [], [subquery: 1]}
+             ]
+
+      assert subquery_sources(distinct) == ["foos", "bars"]
+
       %{distinct: distinct} =
         from p in "posts",
           as: :p,
@@ -88,6 +106,21 @@ defmodule Ecto.Query.Builder.DistinctTest do
 
       assert distinct.expr == [asc: {:exists, [], [subquery: 0]}]
       assert [_] = distinct.subqueries
+
+      %{distinct: distinct} =
+        from p in "posts",
+          as: :p,
+          distinct: [
+            asc: exists(from f in "foos", where: f.id == parent_as(:p).id),
+            desc: exists(from b in "bars", where: b.id == parent_as(:p).id)
+          ]
+
+      assert distinct.expr == [
+               asc: {:exists, [], [subquery: 0]},
+               desc: {:exists, [], [subquery: 1]}
+             ]
+
+      assert subquery_sources(distinct) == ["foos", "bars"]
     end
 
     test "raises on non-atoms" do

test/ecto/query/builder/group_by_test.exs

@@ -6,6 +6,10 @@ defmodule Ecto.Query.Builder.GroupByTest do
 
   import Ecto.Query
 
+  defp subquery_sources(%{subqueries: subqueries}) do
+    Enum.map(subqueries, fn subquery -> elem(subquery.query.from.source, 0) end)
+  end
+
   describe "escape" do
     test "handles expressions and params" do
       assert {Macro.escape(quote(do: [&0.y()])), {[], %{}}} ==
@@ -62,6 +66,15 @@ defmodule Ecto.Query.Builder.GroupByTest do
       assert group_by.expr == [{:exists, [], [{:subquery, 0}]}]
       assert [_] = group_by.subqueries
 
+      key = [
+        dynamic([q], exists(from f in "foos", where: f.title == parent_as(:q).title)),
+        dynamic([q], exists(from b in "bars", where: b.title == parent_as(:q).title))
+      ]
+
+      assert [group_by] = from(q in "q", as: :q, group_by: ^key).group_bys
+      assert group_by.expr == [{:exists, [], [{:subquery, 0}]}, {:exists, [], [{:subquery, 1}]}]
+      assert subquery_sources(group_by) == ["foos", "bars"]
+
       assert [group_by] =
                group_by(
                  "q",
@@ -71,6 +84,19 @@ defmodule Ecto.Query.Builder.GroupByTest do
 
       assert group_by.expr == [{:exists, [], [{:subquery, 0}]}]
       assert [_] = group_by.subqueries
+
+      assert [group_by] =
+               from(
+                 q in "q",
+                 as: :q,
+                 group_by: [
+                   exists(from f in "foos", where: f.title == parent_as(:q).title),
+                   exists(from b in "bars", where: b.title == parent_as(:q).title)
+                 ]
+               ).group_bys
+
+      assert group_by.expr == [{:exists, [], [{:subquery, 0}]}, {:exists, [], [{:subquery, 1}]}]
+      assert subquery_sources(group_by) == ["foos", "bars"]
     end
 
     test "raises when no a field or a list of fields" do

test/ecto/query/builder/order_by_test.exs

@@ -6,6 +6,10 @@ defmodule Ecto.Query.Builder.OrderByTest do
 
   import Ecto.Query
 
+  defp subquery_sources(%{subqueries: subqueries}) do
+    Enum.map(subqueries, fn subquery -> elem(subquery.query.from.source, 0) end)
+  end
+
   describe "escape" do
     defmacro my_custom_field(p) do
       quote(do: fragment("lower(?)", unquote(p).title))
@@ -233,6 +237,20 @@ defmodule Ecto.Query.Builder.OrderByTest do
       assert order_by.expr == [asc: {:exists, [], [subquery: 0]}]
       assert [_] = order_by.subqueries
 
+      order_by = [
+        asc: dynamic([p], exists(from f in "foos", where: f.id == parent_as(:p).id)),
+        desc: dynamic([p], exists(from b in "bars", where: b.id == parent_as(:p).id))
+      ]
+
+      %{order_bys: [order_by]} = from p in "posts", as: :p, order_by: ^order_by
+
+      assert order_by.expr == [
+               asc: {:exists, [], [subquery: 0]},
+               desc: {:exists, [], [subquery: 1]}
+             ]
+
+      assert subquery_sources(order_by) == ["foos", "bars"]
+
       %{order_bys: [order_by]} =
         from p in "posts",
           as: :p,
@@ -242,6 +260,21 @@ defmodule Ecto.Query.Builder.OrderByTest do
 
       assert order_by.expr == [asc: {:exists, [], [subquery: 0]}]
       assert [_] = order_by.subqueries
+
+      %{order_bys: [order_by]} =
+        from p in "posts",
+          as: :p,
+          order_by: [
+            asc: exists(from f in "foos", where: f.id == parent_as(:p).id),
+            desc: exists(from b in "bars", where: b.id == parent_as(:p).id)
+          ]
+
+      assert order_by.expr == [
+               asc: {:exists, [], [subquery: 0]},
+               desc: {:exists, [], [subquery: 1]}
+             ]
+
+      assert subquery_sources(order_by) == ["foos", "bars"]
     end
 
     test "supports interpolated atomnames in selected_as/1" do

test/ecto/query/builder/windows_test.exs

@@ -6,6 +6,10 @@ defmodule Ecto.Query.Builder.WindowsTest do
 
   import Ecto.Query
 
+  defp subquery_sources(%{subqueries: subqueries}) do
+    Enum.map(subqueries, fn subquery -> elem(subquery.query.from.source, 0) end)
+  end
+
   defmacrop rows_between_preceding(count) do
     quote do
       fragment(unquote("ROWS BETWEEN #{count} PRECEDING AND CURRENT ROW"))
@@ -132,6 +136,20 @@ defmodule Ecto.Query.Builder.WindowsTest do
       assert query.windows[:w].expr[:partition_by] == [{:exists, [], [subquery: 0]}]
       assert [_] = query.windows[:w].subqueries
 
+      partition_by = [
+        dynamic([p], exists(from f in "foos", where: f.title == parent_as(:q).title)),
+        dynamic([p], exists(from b in "bars", where: b.title == parent_as(:q).title))
+      ]
+
+      query = "q" |> windows([p], w: [partition_by: ^partition_by])
+
+      assert query.windows[:w].expr[:partition_by] == [
+               {:exists, [], [subquery: 0]},
+               {:exists, [], [subquery: 1]}
+             ]
+
+      assert subquery_sources(query.windows[:w]) == ["foos", "bars"]
+
       query =
         "q"
         |> windows([p],
@@ -142,6 +160,48 @@ defmodule Ecto.Query.Builder.WindowsTest do
 
       assert query.windows[:w].expr[:partition_by] == [{:exists, [], [subquery: 0]}]
       assert [_] = query.windows[:w].subqueries
+
+      query =
+        "q"
+        |> windows([p],
+          w: [
+            partition_by: [
+              exists(from f in "foos", where: f.title == parent_as(:q).title),
+              exists(from b in "bars", where: b.title == parent_as(:q).title)
+            ]
+          ]
+        )
+
+      assert query.windows[:w].expr[:partition_by] == [
+               {:exists, [], [subquery: 0]},
+               {:exists, [], [subquery: 1]}
+             ]
+
+      assert subquery_sources(query.windows[:w]) == ["foos", "bars"]
+
+      order_by = [
+        asc: dynamic([p], exists(from baz in "bazs", where: baz.title == parent_as(:q).title))
+      ]
+
+      query =
+        "q"
+        |> windows([p],
+          w: [
+            partition_by: [
+              exists(from f in "foos", where: f.title == parent_as(:q).title),
+              exists(from b in "bars", where: b.title == parent_as(:q).title)
+            ],
+            order_by: ^order_by
+          ]
+        )
+
+      assert query.windows[:w].expr[:partition_by] == [
+               {:exists, [], [subquery: 0]},
+               {:exists, [], [subquery: 1]}
+             ]
+
+      assert query.windows[:w].expr[:order_by] == [asc: {:exists, [], [subquery: 2]}]
+      assert subquery_sources(query.windows[:w]) == ["foos", "bars", "bazs"]
     end
 
     test "raises on invalid partition by" do