8000 Use unique var names when computing bindings by josevalim · Pull Request #4625 · elixir-ecto/ecto · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Use unique var names when computing bindings #4625

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 22, 2025
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
17 changes: 11 additions & 6 deletions lib/ecto/query/builder.ex
Original file line number Diff line number Diff line change
Expand Up @@ -1007,11 +1007,13 @@ defmodule Ecto.Query.Builder do
{query, vars}

{vars, [_ | tail]} ->
var = Macro.unique_var(:query, __MODULE__)

query =
quote do
query = Ecto.Queryable.to_query(unquote(query))
escape_count = Ecto.Query.Builder.count_binds(query)
query
unquote(var) = Ecto.Queryable.to_query(unquote(query))
escape_count = Ecto.Query.Builder.count_binds(unquote(var))
unquote(var)
end

tail =
Expand All @@ -1029,18 +1031,21 @@ defmodule Ecto.Query.Builder do
defp calculate_named_binds(query, []), do: {query, []}

defp calculate_named_binds(query, vars) do
var = Macro.unique_var(:query, __MODULE__)

assignments =
for {:named, key, name} <- vars do
quote do
unquote({key, [], __MODULE__}) = unquote(__MODULE__).count_alias!(query, unquote(name))
unquote({key, [], __MODULE__}) =
unquote(__MODULE__).count_alias!(unquote(var), unquote(name))
end
end

query =
quote do
query = Ecto.Queryable.to_query(unquote(query))
unquote(var) = Ecto.Queryable.to_query(unquote(query))
unquote_splicing(assignments)
query
unquote(var)
end

pairs =
Expand Down
130 changes: 99 additions & 31 deletions lib/ecto/query/builder/join.ex
{:^, _, [prefix]} -> prefix
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ defmodule Ecto.Query.Builder.Join do
{:x, {:{}, [], [:fragment, [], [raw: "foo"]]}, nil, nil, []}
"""
@spec escape(Macro.t, Keyword.t, Macro.Env.t) :: {atom, Macro.t | nil, Macro.t | nil, list}
@spec escape(Macro.t(), Keyword.t(), Macro.Env.t()) ::
{atom, Macro.t() | nil, Macro.t() | nil, list}
def escape({:in, _, [{var, _, context}, expr]}, vars, env)
when is_atom(var) and is_atom(context) do
{_, expr, assoc, prelude, params} = escape(expr, vars, env)
Expand Down Expand Up @@ -76,14 +77,14 @@ defmodule Ecto.Query.Builder.Join do
{:_, {string, schema}, nil, nil, []}

_ ->
Builder.error! "malformed join `#{Macro.to_string(join)}` in query expression"
Builder.error!("malformed join `#{Macro.to_string(join)}` in query expression")
end
end

def escape({:assoc, _, [{var, _, context}, field]}, vars, _env)
when is_atom(var) and is_atom(context) do
ensure_field!(field)
var = Builder.find_var!(var, vars)
var = Builder.find_var!(var, vars)
field = Builder.quoted_atom!(field, "field/2")
{:_, nil, {var, field}, nil, []}
end
Expand All @@ -103,7 +104,8 @@ defmodule Ecto.Query.Builder.Join do
def escape(join, vars, env) do
case Macro.expand(join, env) do
^join ->
Builder.error! "malformed join `#{Macro.to_string(join)}` in query expression"
Builder.error!("malformed join `#{Macro.to_string(join)}` in query expression")

join ->
escape(join, vars, env)
end
Expand All @@ -114,10 +116,13 @@ defmodule Ecto.Query.Builder.Join do
"""
def join!(expr) when is_atom(expr),
do: {nil, expr}

def join!(exp 8000 r) when is_binary(expr),
do: {expr, nil}

def join!({source, module}) when is_binary(source) and is_atom(module),
do: {source, module}

def join!(expr),
do: Ecto.Queryable.to_query(expr)

Expand All @@ -128,8 +133,19 @@ defmodule Ecto.Query.Builder.Join do
If possible, it does all calculations at compile time to avoid
runtime work.
"""
@spec build(Macro.t, atom, [Macro.t], Macro.t, Macro.t, Macro.t, atom, nil | {:ok, Ecto.Schema.prefix}, nil | String.t | [String.t], Macro.Env.t) ::
{Macro.t, Keyword.t, non_neg_integer | nil}
@spec build(
Macro.t(),
atom,
[Macro.t()],
Macro.t(),
Macro.t(),
Macro.t(),
atom,
nil | {:ok, Ecto.Schema.prefix()},
nil | String.t() | [String.t()],
Macro.Env.t()
) ::
{Macro.t(), Keyword.t(), non_neg_integer | nil}
def build(query, qual, binding, expr, count_bind, on, as, prefix, maybe_hints, env) do
{:ok, prefix} = prefix || {:ok, nil}
hints = List.wrap(maybe_hints)
Expand All @@ -141,18 +157,36 @@ defmodule Ecto.Query.Builder.Join do
)
end

prefix = case prefix do
nil -> nil
prefix when is_binary(prefix) -> prefix
prefix -> Builder.error!("`prefix` must be a compile time string or an interpolated value using ^, got: #{Macro.to_string(prefix)}")
end
prefix =
case prefix do
nil ->
nil

as = case as do
{:^, _, [as]} -> as
as when is_atom(as) -> as
as -> Builder.error!("`as` must be a compile time atom or an interpolated value using ^, got: #{Macro.to_string(as)}")
end
prefix when is_binary(prefix) ->
prefix

{:^, _, [prefix]} ->
prefix

prefix ->
Builder.error!(
"`prefix` must be a compile time string or an interpolated value using ^, got: #{Macro.to_string(prefix)}"
)
end

as =
case as do
{:^, _, [as]} ->
as

as when is_atom(as) ->
as

as ->
Builder.error!(
"`as` must be a compile time atom or an interpolated value using ^, got: #{Macro.to_string(as)}"
)
end

{query, binding} = Builder.escape_binding(query, binding, env)
{join_bind, join_source, join_assoc, join_prelude, join_params} = escape(expr, binding, env)
Expand All @@ -163,11 +197,14 @@ defmodule Ecto.Query.Builder.Join do

{count_bind, query} =
if is_nil(count_bind) do
var = Macro.unique_var(:query, __MODULE__)

query =
quote do
Ecto.Queryable.to_query(unquote(query))
unquote(var) = Ecto.Queryable.to_query(unquote(query))
end
{quote(do: Builder.count_binds(unquote(query))), query}

{quote(do: Builder.count_binds(unquote(var))), query}
else
{count_bind, query}
end
Expand Down Expand Up @@ -252,10 +289,11 @@ defmodule Ecto.Query.Builder.Join do

defp ensure_on(on, _assoc, _qual, _source, _env) when on != nil, do: on

defp ensure_on(nil, _assoc = nil, qual, source, env) when qual not in [:cross, :cross_lateral] do
defp ensure_on(nil, _assoc = nil, qual, source, env)
when qual not in [:cross, :cross_lateral] do
maybe_source =
with {source, alias} <- source,
source when source != nil <- source || alias do
source when source != nil <- source || alias do
" on #{inspect(source)}"
else
_ -> ""
Expand All @@ -275,6 +313,7 @@ defmodule Ecto.Query.Builder.Join do
def apply(%Ecto.Query{joins: joins} = query, expr, nil, _count_bind) do
%{query | joins: joins ++ [expr]}
end

def apply(%Ecto.Query{joins: joins, aliases: aliases} = query, expr, as, count_bind) do
aliases =
case aliases do
Expand All @@ -284,6 +323,7 @@ defmodule Ecto.Query.Builder.Join do

%{query | joins: joins ++ [expr], aliases: aliases}
end

def apply(query, expr, as, count_bind) do
apply(Ecto.Queryable.to_query(query), expr, as, count_bind)
end
Expand All @@ -295,20 +335,24 @@ defmodule Ecto.Query.Builder.Join do

def runtime_aliases(aliases, name, join_count) when is_integer(join_count) do
if Map.has_key?(aliases, name) do
Builder.error! "alias `#{inspect name}` already exists"
Builder.error!("alias `#{inspect(name)}` already exists")
else
Map.put(aliases, name, join_count)
end
end

defp compile_aliases({:%{}, meta, aliases}, name, join_count)
when is_atom(name) and is_integer(join_count) do
{:%{}, meta, aliases |> Map.new |> runtime_aliases(name, join_count) |> Map.to_list}
{:%{}, meta, aliases |> Map.new() |> runtime_aliases(name, join_count) |> Map.to_list()}
end

defp compile_aliases(aliases, name, join_count) do
quote do
Ecto.Query.Builder.Join.runtime_aliases(unquote(aliases), unquote(name), unquote(join_count))
Ecto.Query.Builder.Join.runtime_aliases(
unquote(aliases),
< 9E88 span class=pl-k>unquote(name),
unquote(join_count)
)
end
end

Expand All @@ -319,9 +363,20 @@ defmodule Ecto.Query.Builder.Join do
# join without expanded :on is built and applied to the query,
# so that expansion of dynamic :on accounts for the new binding
{on_expr, on_params, on_file, on_line} =
Ecto.Query.Builder.Filter.filter!(:on, apply(query, join, as, count_bind), expr, count_bind, file, line)
Ecto.Query.Builder.Filter.filter!(
:on,
apply(query, join, as, count_bind),
expr,
count_bind,
file,
line
)

join = %{
join
| on: %QueryExpr{expr: on_expr, params: on_params, line: on_line, file: on_file}
}

join = %{join | on: %QueryExpr{expr: on_expr, params: on_params, line: on_line, file: on_file}}
apply(query, join, as, count_bind)
end

Expand All @@ -335,24 +390,37 @@ defmodule Ecto.Query.Builder.Join do

defp validate_bind(bind, all) do
if bind != :_ and bind in all do
Builder.error! "variable `#{bind}` is already defined in query"
Builder.error!("variable `#{bind}` is already defined in query")
end
end

@qualifiers [:inner, :inner_lateral, :left, :left_lateral, :right, :full, :cross, :cross_lateral]
@qualifiers [
:inner,
:inner_lateral,
:left,
:left_lateral,
:right,
:full,
:cross,
:cross_lateral
]

@doc """
Called at runtime to check dynamic qualifier.
"""
def qual!(qual) when qual in @qualifiers, do: qual

def qual!(qual) do
raise ArgumentError,
"invalid join qualifier `#{inspect qual}`, accepted qualifiers are: " <>
Enum.map_join(@qualifiers, ", ", &"`#{inspect &1}`")
"invalid join qualifier `#{inspect(qual)}`, accepted qualifiers are: " <>
Enum.map_join(@qualifiers, ", ", &"`#{inspect(&1)}`")
end

defp ensure_field!({var, _, _}) when var != :^ do
Builder.error! "you passed the variable `#{var}` to `assoc/2`. Did you mean to pass the atom `:#{var}`?"
Builder.error!(
"you passed the variable `#{var}` to `assoc/2`. Did you mean to pass the atom `:#{var}`?"
)
end

defp ensure_field!(_), do: true
end
Loading
0