Fix splice params in dynamic expressions

lib/ecto/query/builder.ex

@@ -800,7 +800,7 @@ defmodule Ecto.Query.Builder do
   end
 
   defp escape_fragment({:splice, _meta, [{:^, _, [value]}]}, {params, acc, _}, _vars, _env) do
-    checked = quote do: Ecto.Query.Builder.splice!(unquote(value), length(unquote(params)))
+    checked = quote do: Ecto.Query.Builder.splice!(unquote(value), unquote(length(params)))
     param = {value, {:splice, :any}}
     {{:splice, checked}, {[param | params], acc, false}}
   end

lib/ecto/query/builder/dynamic.ex

@@ -79,29 +79,58 @@ defmodule Ecto.Query.Builder.Dynamic do
     {expr, params, count}
   end
 
-  defp expand(query, %{fun: fun}, {binding, params, subqueries, aliases, count}) do
+  defp expand(query, dynamic, acc) do
+    expand(query, dynamic, acc, %{})
+  end
+
+  defp expand(query, %{fun: fun}, {binding, params, subqueries, aliases, count}, splices) do
     {dynamic_expr, dynamic_params, dynamic_subqueries, dynamic_aliases} = fun.(query)
     aliases = merge_aliases(aliases, dynamic_aliases)
 
-    Macro.postwalk(dynamic_expr, {binding, params, subqueries, aliases, count}, fn
-      {:^, meta, [ix]}, {binding, params, subqueries, aliases, count} ->
-        case Enum.fetch!(dynamic_params, ix) do
-          {%Ecto.Query.DynamicExpr{binding: new_binding} = dynamic, _} ->
-            binding = if length(new_binding) > length(binding), do: new_binding, else: binding
-            expand(query, dynamic, {binding, params, subqueries, aliases, count})
-
-          param ->
-            {{:^, meta, [count]}, {binding, [param | params], subqueries, aliases, count + 1}}
-        end
-
-      {:subquery, i}, {binding, params, subqueries, aliases, count} ->
-        subquery = Enum.fetch!(dynamic_subqueries, i)
-        ix = length(subqueries)
-        {{:subquery, ix}, {binding, [{:subquery, ix} | params], [subquery | subqueries], aliases, count + 1}}
-
-      expr, acc ->
-        {expr, acc}
-    end)
+    {expr, {binding, params, subqueries, aliases, count, _splices}} =
+      Macro.postwalk(dynamic_expr, {binding, params, subqueries, aliases, count, splices}, fn
+        {:^, meta, [ix]}, {binding, params, subqueries, aliases, count, splices} ->
+          case Enum.fetch!(dynamic_params, ix) do
+            {%Ecto.Query.DynamicExpr{binding: new_binding} = dynamic, _} ->
+              binding = if length(new_binding) > length(binding), do: new_binding, else: binding
+
+              # Splice keys are indexes into the current dynamic's params, so nested dynamics
+              # must track their own splice placeholders while continuing the outer count.
+              {expr, {binding, params, subqueries, aliases, count}} =
+                expand(query, dynamic, {binding, params, subqueries, aliases, count})
+
+              {expr, {binding, params, subqueries, aliases, count, splices}}
+
+            {_, {:splice, _}} = param ->
+              case splices do
+                %{^ix => splice_count} ->
+                  {{:^, meta, [splice_count]},
+                   {binding, params, subqueries, aliases, count, splices}}
+
+                %{} ->
+                  {{:^, meta, [count]},
+                   {binding, [param | params], subqueries, aliases, count + 1,
+                    Map.put(splices, ix, count)}}
+              end
+
+            param ->
+              {{:^, meta, [count]},
+               {binding, [param | params], subqueries, aliases, count + 1, splices}}
+          end
+
+        {:subquery, i}, {binding, params, subqueries, aliases, count, splices} ->
+          subquery = Enum.fetch!(dynamic_subqueries, i)
+          ix = length(subqueries)
+
+          {{:subquery, ix},
+           {binding, [{:subquery, ix} | params], [subquery | subqueries], aliases, count + 1,
+            splices}}
+
+        expr, acc ->
+          {expr, acc}
+      end)
+
+    {expr, {binding, params, subqueries, aliases, count}}
   end
 
   defp merge_aliases(old_aliases, new_aliases) do

test/ecto/query/planner_test.exs

@@ -173,6 +173,12 @@ defmodule Ecto.Query.PlannerTest do
     {query, cast_params, dump_params, select}
   end
 
+  defp flatten_boolean({op, _, [left, right]}, op) do
+    flatten_boolean(left, op) ++ flatten_boolean(right, op)
+  end
+
+  defp flatten_boolean(expr, _op), do: [expr]
+
   defp select_fields(fields, ix) do
     for field <- fields do
       {{:., [writable: :always], [{:&, [], [ix]}, field]}, [], []}
@@ -1983,6 +1989,54 @@ defmodule Ecto.Query.PlannerTest do
            ] = parts
   end
 
+  test "normalize: params around splicing inside dynamic" do
+    list = [1, 2, 3]
+    ids = [10, 11]
+
+    dynamic =
+      dynamic(
+        [p],
+        p.title == ^"a" and fragment("? = ANY(?)", p.id, splice(^list)) and
+          p.visits > ^1 and fragment("? = ANY(?)", p.id, splice(^ids))
+      )
+
+    {query, cast_params, dump_params, _} =
+      from(p in Post, select: p.id)
+      |> where(^dynamic)
+      |> normalize_with_params()
+
+    assert cast_params == ["a", 1, 2, 3, 1, 10, 11]
+    assert dump_params == ["a", 1, 2, 3, 1, 10, 11]
+
+    [title_expr, {:fragment, _, first_fragment}, visits_expr, {:fragment, _, second_fragment}] =
+      flatten_boolean(hd(query.wheres).expr, :and)
+
+    assert Macro.to_string(title_expr) == "&0.post_title() == ^0"
+    assert Macro.to_string(visits_expr) == "&0.visits() > ^4"
+
+    assert [
+             _,
+             {:expr, {{:., _, [{:&, _, [0]}, :id]}, _, []}},
+             _,
+             {:expr, {:^, _, [1]}},
+             _,
+             {:expr, {:^, _, [2]}},
+             _,
+             {:expr, {:^, _, [3]}},
+             _
+           ] = first_fragment
+
+    assert [
+             _,
+             {:expr, {{:., _, [{:&, _, [0]}, :id]}, _, []}},
+             _,
+             {:expr, {:^, _, [5]}},
+             _,
+             {:expr, {:^, _, [6]}},
+             _
+           ] = second_fragment
+  end
+
   test "normalize: from values list" do
     uuid = Ecto.UUID.generate()
     values = [%{bid: uuid, num: 1}, %{bid: uuid, num: 2}]

test/ecto/query_test.exs

@@ -1246,6 +1246,19 @@ defmodule Ecto.QueryTest do
              ] = parts
     end
 
+    test "evaluates interpolated expressions before runtime splicing once" do
+      Process.put(:fragment_eval_count, 0)
+
+      value = fn ->
+        Process.put(:fragment_eval_count, Process.get(:fragment_eval_count) + 1)
+        1
+      end
+
+      from p in "posts", select: fragment("(?, ?)", ^value.(), splice(^[2, 3]))
+
+      assert Process.get(:fragment_eval_count) == 1
+    end
+
     test "keeps UTF-8 encoding" do
       assert inspect(from p in "posts", where: fragment("héllò")) ==
                ~s[#Ecto.Query<from p0 in \"posts\", where: fragment("héllò")>]