Skip to content

Commit

Permalink
Allow selected_as at the root of dynamic/2 (#4020)
Browse files Browse the repository at this point in the history
  • Loading branch information
greg-rychlewski authored Oct 3, 2022
1 parent f280459 commit b7d303a
Show file tree
Hide file tree
Showing 6 changed files with 115 additions and 38 deletions.
12 changes: 12 additions & 0 deletions lib/ecto/query.ex
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,18 @@ defmodule Ecto.Query do
from query, select: [:period, :metric], select_merge: ^%{metric: metric}
Aliasing fields with `selected_as/2` and referencing them with `selected_as/1`
is also allowed:
fields = %{
period: dynamic([p], selected_as(p.month, :month)),
metric: dynamic([p], p.distance)
}
order = dynamic(selected_as(:month))
from query, select: ^fields, order_by: ^order
## Updates
A `dynamic` is also supported inside updates, for example:
Expand Down
21 changes: 21 additions & 0 deletions lib/ecto/query/builder.ex
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ defmodule Ecto.Query.Builder do
ntile: {1, :integer}
]

@select_alias_dummy_value []

@typedoc """
Quoted types store primitive types and types in the format
{source, quoted}. The latter are handled directly in the planner,
Expand Down Expand Up @@ -781,6 +783,12 @@ defmodule Ecto.Query.Builder do
@spec escape_params(list()) :: list()
def escape_params(list), do: Enum.reverse(list)

@doc """
Escape the select alias map
"""
@spec escape_select_aliases(map()) :: Macro.t
def escape_select_aliases(%{} = aliases), do: {:%{}, [], Map.to_list(aliases)}

@doc """
Escapes a variable according to the given binds.
Expand Down Expand Up @@ -1217,6 +1225,19 @@ defmodule Ecto.Query.Builder do
end)
end

@doc """
Called by the select escaper at compile time and dynamic builder at runtime to track select aliases
"""
def add_select_alias(aliases, name) do
case aliases do
%{^name => _} ->
error! "the alias `#{inspect(name)}` has been specified more than once using `selected_as/2`"

aliases ->
Map.put(aliases, name, @select_alias_dummy_value)
end
end

@doc """
Applies a query at compilation time or at runtime.
Expand Down
51 changes: 34 additions & 17 deletions lib/ecto/query/builder/dynamic.ex
Original file line number Diff line number Diff line change
Expand Up @@ -4,32 +4,42 @@ defmodule Ecto.Query.Builder.Dynamic do
@moduledoc false

alias Ecto.Query.Builder
alias Ecto.Query.Builder.Select

@doc """
Builds a dynamic expression.
"""
@spec build([Macro.t], Macro.t, Macro.Env.t) :: Macro.t
def build(binding, expr, env) do
{query, vars} = Builder.escape_binding(quote(do: query), binding, env)
{expr, {params, acc}} = Builder.escape(expr, :any, {[], %{subqueries: []}}, vars, env)
{expr, {params, acc}} = escape(expr, :any, {[], %{subqueries: [], aliases: %{}}}, vars, env)
aliases = Builder.escape_select_aliases(acc.aliases)
params = Builder.escape_params(params)

quote do
%Ecto.Query.DynamicExpr{fun: fn query ->
_ = unquote(query)
{unquote(expr), unquote(params), unquote(Enum.reverse(acc.subqueries))}
{unquote(expr), unquote(params), unquote(Enum.reverse(acc.subqueries)), unquote(aliases)}
end,
binding: unquote(Macro.escape(binding)),
file: unquote(env.file),
line: unquote(env.line)}
end
end

defp escape({:selected_as, _, [_, _]} = expr, _type, _params_acc, vars, env) do
Select.escape(expr, vars, env)
end

defp escape(expr, type, params_acc, vars, env) do
Builder.escape(expr, type, params_acc, vars, env)
end

@doc """
Expands a dynamic expression for insertion into the given query.
"""
def fully_expand(query, %{file: file, line: line, binding: binding} = dynamic) do
{expr, {binding, params, subqueries, _count}} = expand(query, dynamic, {binding, [], [], 0})
{expr, {binding, params, subqueries, _aliases, _count}} = expand(query, dynamic, {binding, [], [], %{}, 0})
{expr, binding, Enum.reverse(params), Enum.reverse(subqueries), file, line}
end

Expand All @@ -40,16 +50,16 @@ defmodule Ecto.Query.Builder.Dynamic do
list is not reversed. This is useful when the dynamic expression
is given in the middle of an expression.
"""
def partially_expand(query, %{binding: binding} = dynamic, params, subqueries, count) do
{expr, {_binding, params, subqueries, count}} =
expand(query, dynamic, {binding, params, subqueries, count})
def partially_expand(query, %{binding: binding} = dynamic, params, subqueries, aliases, count) do
{expr, {_binding, params, subqueries, aliases, count}} =
expand(query, dynamic, {binding, params, subqueries, aliases, count})

{expr, params, subqueries, count}
{expr, params, subqueries, aliases, count}
end

def partially_expand(kind, query, %{binding: binding} = dynamic, params, count) do
{expr, {_binding, params, subqueries, count}} =
expand(query, dynamic, {binding, params, [], count})
{expr, {_binding, params, subqueries, _aliases, count}} =
expand(query, dynamic, {binding, params, [], %{}, count})

if subqueries != [] do
raise ArgumentError, "subqueries are not allowed in `#{kind}` expressions"
Expand All @@ -58,27 +68,34 @@ defmodule Ecto.Query.Builder.Dynamic do
{expr, params, count}
end

defp expand(query, %{fun: fun}, {binding, params, subqueries, count}) do
{dynamic_expr, dynamic_params, dynamic_subqueries} = fun.(query)
defp expand(query, %{fun: fun}, {binding, params, subqueries, aliases, count}) do
{dynamic_expr, dynamic_params, dynamic_subqueries, dynamic_aliases} = fun.(query)
aliases = merge_aliases(aliases, dynamic_aliases)

Macro.postwalk(dynamic_expr, {binding, params, subqueries, count}, fn
{:^, meta, [ix]}, {binding, params, subqueries, count} ->
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, count})
expand(query, dynamic, {binding, params, subqueries, aliases, count})

param ->
{{:^, meta, [count]}, {binding, [param | params], subqueries, count + 1}}
{{:^, meta, [count]}, {binding, [param | params], subqueries, aliases, count + 1}}
end

{:subquery, i}, {binding, params, subqueries, count} ->
{:subquery, i}, {binding, params, subqueries, aliases, count} ->
subquery = Enum.fetch!(dynamic_subqueries, i)
ix = length(subqueries)
{{:subquery, ix}, {binding, [{:subquery, ix} | params], [subquery | subqueries], count + 1}}
{{:subquery, ix}, {binding, [{:subquery, ix} | params], [subquery | subqueries], aliases, count + 1}}

expr, acc ->
{expr, acc}
end)
end

defp merge_aliases(old_aliases, new_aliases) do
Enum.reduce(new_aliases, old_aliases, fn {alias, _}, aliases ->
Builder.add_select_alias(aliases, alias)
end)
end
end
31 changes: 10 additions & 21 deletions lib/ecto/query/builder/select.ex
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ defmodule Ecto.Query.Builder.Select do

alias Ecto.Query.Builder

@dummy_value []

@doc """
Escapes a select.
Expand Down Expand Up @@ -117,7 +115,7 @@ defmodule Ecto.Query.Builder.Select do
defp escape({:selected_as, _, [expr, name]}, {params, acc}, vars, env) when is_atom(name) do
{escaped, {params, acc}} = Builder.escape(expr, :any, {params, acc}, vars, env)
expr = {:{}, [], [:selected_as, [], [escaped, name]]}
aliases = add_alias(acc.aliases, name)
aliases = Builder.add_select_alias(acc.aliases, name)
{expr, {params, %{acc | aliases: aliases}}}
end

Expand Down Expand Up @@ -204,12 +202,13 @@ defmodule Ecto.Query.Builder.Select do
Called at runtime for interpolated/dynamic selects.
"""
def select!(kind, query, fields, file, line) when is_map(fields) do
{expr, {params, subqueries, _count}} = expand_nested(fields, {[], [], 0}, query)
{expr, {params, subqueries, aliases, _count}} = expand_nested(fields, {[], [], %{}, 0}, query)

%Ecto.Query.SelectExpr{
expr: expr,
params: Enum.reverse(params),
subqueries: Enum.reverse(subqueries),
aliases: aliases,
file: file,
line: line
}
Expand All @@ -231,22 +230,22 @@ defmodule Ecto.Query.Builder.Select do
end
end

defp expand_nested(%Ecto.Query.DynamicExpr{} = dynamic, {params, subqueries, count}, query) do
{expr, params, subqueries, count} =
Ecto.Query.Builder.Dynamic.partially_expand(query, dynamic, params, subqueries, count)
defp expand_nested(%Ecto.Query.DynamicExpr{} = dynamic, {params, subqueries, aliases, count}, query) do
{expr, params, subqueries, aliases, count} =
Ecto.Query.Builder.Dynamic.partially_expand(query, dynamic, params, subqueries, aliases, count)

{expr, {params, subqueries, count}}
{expr, {params, subqueries, aliases, count}}
end

defp expand_nested(%Ecto.SubQuery{} = subquery, {params, subqueries, count}, _query) do
defp expand_nested(%Ecto.SubQuery{} = subquery, {params, subqueries, aliases, count}, _query) do
index = length(subqueries)
# used both in ast and in parameters, as a placeholder.
expr = {:subquery, index}
params = [expr | params]
subqueries = [subquery | subqueries]
count = count + 1

{expr, {params, subqueries, count}}
{expr, {params, subqueries, aliases, count}}
end

defp expand_nested(%type{} = fields, acc, query) do
Expand Down Expand Up @@ -462,16 +461,6 @@ defmodule Ecto.Query.Builder.Select do
%{acc | take: take}
end

defp add_alias(aliases, name) do
case aliases do
%{^name => _} ->
Builder.error! "the alias `#{inspect(name)}` has been specified more than once using `selected_as/2`"

aliases ->
Map.put(aliases, name, @dummy_value)
end
end

defp bump_subquery_params(new_params, old_subqueries) do
len = length(old_subqueries)

Expand Down Expand Up @@ -519,7 +508,7 @@ defmodule Ecto.Query.Builder.Select do

defp merge_aliases(old_aliases, new_aliases) do
Enum.reduce(new_aliases, old_aliases, fn {alias, _}, aliases ->
add_alias(aliases, alias)
Builder.add_select_alias(aliases, alias)
end)
end
end
28 changes: 28 additions & 0 deletions test/ecto/query/builder/select_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,34 @@ defmodule Ecto.Query.Builder.SelectTest do
assert %{select: _, merge: _} = query.select.aliases
end

test "supports dynamic selected values with selected_as/2" do
escaped_alias = {:selected_as, [], [{{:., [], [{:&, [], [0]}, :title]}, [], []}, :alias]}
escaped_alias2 = {:selected_as, [], [{{:., [], [{:&, [], [0]}, :title]}, [], []}, :alias2]}

# map
select_fields = %{title: dynamic([p], selected_as(p.title, :alias))}
merge_fields = %{title2: dynamic([p], selected_as(p.title, :alias2))}

query = from p in "posts", select: ^select_fields, select_merge: ^merge_fields
assert {:%{}, [], [title: escaped_alias, title2: escaped_alias2]} == query.select.expr
assert %{alias: _, alias2: _} = query.select.aliases

# struct
fields = %Post{
title: dynamic([p], selected_as(p.title, :alias)),
}

query = from p in "posts", select: ^fields
assert {:%, [], [_, {:%{}, [], [title: escaped_alias]}]} = query.select.expr
assert %{alias: _} = query.select.aliases

# single field
field = dynamic([p], selected_as(p.title, :alias))
query = from p in "posts", select: ^field
assert escaped_alias == query.select.expr
assert %{alias: _} = query.select.aliases
end

test "raises if name given to selected_as/2 is not an atom" do
message = "selected_as/2 expects `name` to be an atom, got `\"ident\"`"

Expand Down
10 changes: 10 additions & 0 deletions test/ecto/query/planner_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -1753,6 +1753,16 @@ defmodule Ecto.Query.PlannerTest do
end
end

test "with dynamic/2" do
fields = %{
id: dynamic([p], selected_as(p.id, :alias)),
id2: dynamic([p], selected_as(p.id, :alias2))
}

order = dynamic(selected_as(:alias))
from(p in "posts", select: ^fields, order_by: ^order) |> normalize()
end

test "raises if selected_as/2 is used in a subquery" do
message = ~r"`selected_as/2` can only be used in the outer most `select` expression."

Expand Down

0 comments on commit b7d303a

Please sign in to comment.