Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 29 additions & 20 deletions lib/ecto/query/builder.ex
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ defmodule Ecto.Query.Builder do
{{:{}, [], [:fragment, [], [expr]]}, params_acc}
end

def escape({:fragment, _, [query | frags]}, _type, params_acc, vars, env) do
def escape({:fragment, _, [query | frags]}, _type, {params, acc}, vars, env) do
pieces = expand_and_split_fragment(query, env)

if length(pieces) != length(frags) + 1 do
Expand All @@ -214,8 +214,17 @@ defmodule Ecto.Query.Builder do
)
end

{frags, params_acc} = Enum.map_reduce(frags, params_acc, &escape_fragment(&1, &2, vars, env))
{{:{}, [], [:fragment, [], merge_fragments(pieces, frags, [])]}, params_acc}
{frags, {params, acc, compile_merge?}} =
Enum.map_reduce(frags, {params, acc, true}, &escape_fragment(&1, &2, vars, env))

merged =
if compile_merge? do
merge_fragments(pieces, frags, [])
else
quote do: Ecto.Query.Builder.merge_fragments(unquote(pieces), unquote(frags), [])
end

{{:{}, [], [:fragment, [], merged]}, {params, acc}}
end

# subqueries
Expand Down Expand Up @@ -790,12 +799,10 @@ defmodule Ecto.Query.Builder do
end
end

defp escape_fragment({:splice, _meta, [{:^, _, [value]} = expr]}, params_acc, vars, env) do
checked = quote do: Ecto.Query.Builder.splice!(unquote(value))
length = quote do: length(unquote(checked))
{expr, params_acc} = escape(expr, {:splice, :any}, params_acc, vars, env)
escaped = {:{}, [], [:splice, [], [expr, length]]}
{escaped, params_acc}
defp escape_fragment({:splice, _meta, [{:^, _, [value]}]}, {params, acc, _}, _vars, _env) do
checked = quote do: Ecto.Query.Builder.splice!(unquote(value), length(unquote(params)))
param = {value, {:splice, :any}}
{{:splice, checked}, {[param | params], acc, false}}
end

defp escape_fragment({:splice, _meta, [exprs]}, params_acc, vars, env) when is_list(exprs) do
Expand All @@ -811,29 +818,31 @@ defmodule Ecto.Query.Builder do
)
end

defp escape_fragment(expr, params_acc, vars, env) do
escape(expr, :any, params_acc, vars, env)
defp escape_fragment(expr, {params, acc, compile_merge?}, vars, env) do
{expr, {params, acc}} =
escape(expr, :any, {params, acc}, vars, env)

{expr, {params, acc, compile_merge?}}
end

defp merge_fragments([raw_h | raw_t], [{:splice, exprs} | expr_t], []),
def merge_fragments([raw_h | raw_t], [{:splice, exprs} | expr_t], []),
do: [{:raw, raw_h} | merge_fragments(raw_t, expr_t, exprs)]

defp merge_fragments([raw_h | raw_t], [expr_h | expr_t], []),
def merge_fragments([raw_h | raw_t], [expr_h | expr_t], []),
do: [{:raw, raw_h}, {:expr, expr_h} | merge_fragments(raw_t, expr_t, [])]

defp merge_fragments([raw_h], [], []),
def merge_fragments([raw_h], [], []),
do: [{:raw, raw_h}]

defp merge_fragments(raw, expr, [{:splice, exprs} | splice_t]),
def merge_fragments(raw, expr, [{:splice, exprs} | splice_t]),
do: merge_fragments(raw, expr, exprs ++ splice_t)

defp merge_fragments(raw, expr, [splice_h]),
def merge_fragments(raw, expr, [splice_h]),
do: [{:expr, splice_h} | merge_fragments(raw, expr, [])]

defp merge_fragments(raw, expr, [splice_h | splice_t]),
def merge_fragments(raw, expr, [splice_h | splice_t]),
do: [{:expr, splice_h}, {:raw, ","} | merge_fragments(raw, expr, splice_t)]


for {agg, arity} <- @dynamic_aggregates do
defp call_type(unquote(agg), unquote(arity)), do: {:any, :any}
end
Expand Down Expand Up @@ -1336,9 +1345,9 @@ defmodule Ecto.Query.Builder do
@doc """
Called by escaper at runtime to verify splice in fragments.
"""
def splice!(value) do
def splice!(value, param_num) do
if is_list(value) do
value
Enum.map(value, fn _ -> {:^, [], [param_num]} end)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the weirdest part. Because we use length(params) to set the parameter indexes and we don't know how many are in here until runtime.

Doing it this way keeps parameter counting in the rest of the builder the same and planner cleans everything up anyways.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It has been a while, so can you please remind me how it works in this case? fragment(..., splice(^foo), ^bar) How do we know the parameter position for ^bar? I guess the planner goes increment all parameters later on, whenever it sees a splice or an in?

Copy link
Member Author

@greg-rychlewski greg-rychlewski Jan 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah basically like that.

To be more specific, when normalizing a query the prewalk function in the planner has an accumulator that keeps track of the current query parameter number. The counter is initialized to 0 for queries and initialized to the current parameter count when doing stuff like insert_all or conflict queries. Whenever the prewalker sees a query parameter it just increments the parameter counter.

For :in and the current implementation of :splice they have a bit of a trick in the prewalker. Since we don't know the length of the list at compile time we treat the whole list as one query parameter then in the prewalker it shifts the counter by the length of the list. See here for :in and here for :splice. Then in the adapter it sees the AST for these things and does the actual splicing of the SQL text. The parameter list itself is spliced by the planner here.

else
raise ArgumentError,
"splice(^value) expects `value` to be a list, got `#{inspect(value)}`"
Expand Down
12 changes: 0 additions & 12 deletions lib/ecto/query/planner.ex
Original file line number Diff line number Diff line change
Expand Up @@ -1642,18 +1642,6 @@ defmodule Ecto.Query.Planner do
{{quantifier, meta, [subquery]}, acc}
end

defp prewalk(
{:splice, splice_meta, [{:^, meta, [_]}, length]},
_kind,
_query,
_expr,
acc,
_adapter
) do
param = {:^, meta, [acc, length]}
{{:splice, splice_meta, [param]}, acc + length}
end

defp prewalk({{:., dot_meta, [left, field]}, meta, []}, kind, query, expr, acc, _adapter) do
{ix, ix_expr, ix_query} = get_ix!(left, kind, query)
extra = if kind == :select, do: [type: type!(kind, ix_query, expr, ix, field)], else: []
Expand Down
24 changes: 17 additions & 7 deletions test/ecto/query/planner_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -1914,9 +1914,20 @@ defmodule Ecto.Query.PlannerTest do
assert dump_params == [1, 2, 3, 4, 5]

{:in, _, [_, {:fragment, _, parts}]} = hd(query.wheres).expr
assert [_, _, _, {:expr, {:splice, _, [{:^, _, [start_ix, length]}]}}, _, _, _] = parts
assert start_ix == 1
assert length == 3

assert [
_,
{:expr, {:^, _, [0]}},
_,
{:expr, {:^, _, [1]}},
_,
{:expr, {:^, _, [2]}},
_,
{:expr, {:^, _, [3]}},
_,
{:expr, {:^, _, [4]}},
_
] = parts
end

test "normalize: fragment with nested splicing" do
Expand All @@ -1938,14 +1949,13 @@ defmodule Ecto.Query.PlannerTest do
_,
{:expr, 2},
_,
{:expr, {:splice, _, [{:^, _, [start_ix, length]}]}},
{:expr, {:^, _, [1]}},
_,
{:expr, {:^, _, [2]}},
_,
{:expr, {:^, _, [3]}},
_
] = parts

assert start_ix == 1
assert length == 2
end

test "normalize: from values list" do
Expand Down
16 changes: 11 additions & 5 deletions test/ecto/query_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -1134,16 +1134,20 @@ defmodule Ecto.QueryTest do
three = 3

query =
from p in "posts", where: p.id in fragment("(?, ?, ?)", ^1, splice(^[two, three, 4]), ^5)
from p in "posts", where: p.id in fragment("(?,?,?)", ^1, splice(^[two, three, 4]), ^5)

assert {:in, _, [_, {:fragment, _, parts}]} = hd(query.wheres).expr

assert [
raw: "(",
expr: {:^, _, [0]},
raw: ", ",
expr: {:splice, _, [{:^, _, [1]}, 3]},
raw: ", ",
raw: ",",
expr: {:^, _, [1]},
raw: ",",
expr: {:^, _, [1]},
raw: ",",
expr: {:^, _, [1]},
raw: ",",
expr: {:^, _, [2]},
raw: ")"
] = parts
Expand Down Expand Up @@ -1208,7 +1212,9 @@ defmodule Ecto.QueryTest do
raw: ",",
expr: 2,
raw: ",",
expr: {:splice, _, [{:^, _, [1]}, 2]},
expr: {:^, _, [1]},
raw: ",",
expr: {:^, _, [1]},
raw: ",",
expr: {:^, _, [2]},
raw: ")"
Expand Down
Loading