From 96c27b7172f8297e2bd163d2e139b96c4e037ade Mon Sep 17 00:00:00 2001 From: Lukasz Samson Date: Wed, 8 Jul 2026 18:48:46 +0200 Subject: [PATCH] Fix splice params in dynamic expressions --- lib/ecto/query/builder.ex | 2 +- lib/ecto/query/builder/dynamic.ex | 69 ++++++++++++++++++++++--------- test/ecto/query/planner_test.exs | 54 ++++++++++++++++++++++++ test/ecto/query_test.exs | 13 ++++++ 4 files changed, 117 insertions(+), 21 deletions(-) diff --git a/lib/ecto/query/builder.ex b/lib/ecto/query/builder.ex index 9b42956ba3..ef954fe654 100644 --- a/lib/ecto/query/builder.ex +++ b/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 diff --git a/lib/ecto/query/builder/dynamic.ex b/lib/ecto/query/builder/dynamic.ex index 32802a1131..490f782575 100644 --- a/lib/ecto/query/builder/dynamic.ex +++ b/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 diff --git a/test/ecto/query/planner_test.exs b/test/ecto/query/planner_test.exs index e6fd4b9179..d2e69924f6 100644 --- a/test/ecto/query/planner_test.exs +++ b/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}] diff --git a/test/ecto/query_test.exs b/test/ecto/query_test.exs index 25de002fc8..50b9cb75d5 100644 --- a/test/ecto/query_test.exs +++ b/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]