diff --git a/lib/ecto/query.ex b/lib/ecto/query.ex index 2228072ebc..2f717a90db 100644 --- a/lib/ecto/query.ex +++ b/lib/ecto/query.ex @@ -1959,7 +1959,8 @@ defmodule Ecto.Query do > > When selecting a literal atom, its value must be the same across > all queries. Otherwise, the value from the parent query will be - > applied to all other queries. + > applied to all other queries. This also holds true for selecting + > maps with atom keys. Union expression returns only unique rows as if each query returned distinct results. This may cause a performance penalty. If you need @@ -2011,7 +2012,8 @@ defmodule Ecto.Query do > > When selecting a literal atom, its value must be the same across > all queries. Otherwise, the value from the parent query will be - > applied to all other queries. + > applied to all other queries. This also holds true for selecting + > maps with atom keys. Note that the operations `order_by`, `limit` and `offset` of the current `query` apply to the result of the union. `order_by` must @@ -2058,7 +2060,8 @@ defmodule Ecto.Query do > > When selecting a literal atom, its value must be the same across > all queries. Otherwise, the value from the parent query will be - > applied to all other queries. + > applied to all other queries. This also holds true for selecting + > maps with atom keys. Except expression returns only unique rows as if each query returned distinct results. This may cause a performance penalty. If you need @@ -2110,7 +2113,8 @@ defmodule Ecto.Query do > > When selecting a literal atom, its value must be the same across > all queries. Otherwise, the value from the parent query will be - > applied to all other queries. + > applied to all other queries. This also holds true for selecting + > maps with atom keys. Note that the operations `order_by`, `limit` and `offset` of the current `query` apply to the result of the set difference. `order_by` @@ -2157,7 +2161,8 @@ defmodule Ecto.Query do > > When selecting a literal atom, its value must be the same across > all queries. Otherwise, the value from the parent query will be - > applied to all other queries. + > applied to all other queries. This also holds true for selecting + > maps with atom keys. Intersect expression returns only unique rows as if each query returned distinct results. This may cause a performance penalty. If you need @@ -2209,7 +2214,8 @@ defmodule Ecto.Query do > > When selecting a literal atom, its value must be the same across > all queries. Otherwise, the value from the parent query will be - > applied to all other queries. + > applied to all other queries. This also holds true for selecting + > maps with atom keys. Note that the operations `order_by`, `limit` and `offset` of the current `query` apply to the result of the set difference. `order_by` diff --git a/lib/ecto/query/builder/select.ex b/lib/ecto/query/builder/select.ex index 3de10f1d15..a343370c05 100644 --- a/lib/ecto/query/builder/select.ex +++ b/lib/ecto/query/builder/select.ex @@ -140,6 +140,11 @@ defmodule Ecto.Query.Builder.Select do {k, params_acc} end + defp escape_key({:^, _, [k]}, params_acc, _vars, _env) do + checked = quote do: Ecto.Query.Builder.Select.map_key!(unquote(k)) + {checked, params_acc} + end + defp escape_key(k, params_acc, vars, env) do escape(k, params_acc, vars, env) end @@ -174,6 +179,20 @@ defmodule Ecto.Query.Builder.Select do end end + @doc """ + Called at runtime to verify a map key + """ + def map_key!(key) when is_binary(key), do: key + def map_key!(key) when is_integer(key), do: key + def map_key!(key) when is_float(key), do: key + def map_key!(key) when is_atom(key), do: key + + def map_key!(other) do + Builder.error!( + "interpolated map keys in `:select` can only be atoms, strings or numbers, got: #{inspect(other)}" + ) + end + # atom list sigils defp take?({name, _, [_, modifiers]}) when name in ~w(sigil_w sigil_W)a do ?a in modifiers diff --git a/test/ecto/query/builder/select_test.exs b/test/ecto/query/builder/select_test.exs index c74f4df32b..476f084f63 100644 --- a/test/ecto/query/builder/select_test.exs +++ b/test/ecto/query/builder/select_test.exs @@ -486,6 +486,16 @@ defmodule Ecto.Query.Builder.SelectTest do %Ecto.Query{} |> select([], 1) |> select([], 2) end end + + test "supports interpolated map keys" do + key = :test_key + + q = from p in "posts", select: %{^key => 1} + assert {:%{}, [], [test_key: 1]} = q.select.expr + + q = from p in "posts", select: %{^:test_key => 1} + assert {:%{}, [], [test_key: 1]} = q.select.expr + end end describe "select_merge" do @@ -574,7 +584,7 @@ defmodule Ecto.Query.Builder.SelectTest do end) assert Macro.to_string(query.select.expr) == - "merge(merge(%{comments: count(&2.id())}, %{^0 => &0.name()}), %{^1 => &1.author()})" + "%{comments: count(&2.id()), name: &0.name(), author: &1.author()}" end test "supports '...' in binding list with no prior select" do @@ -672,5 +682,24 @@ defmodule Ecto.Query.Builder.SelectTest do select_merge: %{t: p.title, b: p.body} assert Macro.to_string(query.select.expr) =~ "merge" end + + test "supports interpolated map keys" do + shared_key = :shared + merge_key = :merge + + q = + from p in "posts", + select: %{^shared_key => :old}, + select_merge: %{^shared_key => :new, ^merge_key => :merge} + + assert {:%{}, [], [shared: :new, merge: :merge]} = q.select.expr + + q = + from p in "posts", + select: %{^:shared => :old}, + select_merge: %{^:shared => :new, ^:merge => :merge} + + assert {:%{}, [], [shared: :new, merge: :merge]} = q.select.expr + end end end diff --git a/test/ecto/query/planner_test.exs b/test/ecto/query/planner_test.exs index 0a07c84c20..ae9331d073 100644 --- a/test/ecto/query/planner_test.exs +++ b/test/ecto/query/planner_test.exs @@ -1639,6 +1639,22 @@ defmodule Ecto.Query.PlannerTest do assert true in union_query.select.fields end + test "normalize: select with combinations and dynamic map keys" do + union_map_key = :id + outer_map_key = :text + union_query = from(c in Comment, select: %{^union_map_key => c.id, :text => c.text}) + + {normalized_query, _, _, select} = + from(c in Comment, select: %{:id => c.id, ^outer_map_key => c.text}, union: ^union_query) + |> normalize_with_params() + + normalized_union_query = normalized_query.combinations |> List.first() |> elem(1) + + assert select.postprocess == {:map, [id: {:value, :id}, text: {:value, :string}]} + assert {:%{}, _, [id: _, text: _]} = normalized_query.select.expr + assert {:%{}, _, [id: _, text: _]} = normalized_union_query.select.expr + end + test "normalize: select on schemaless" do assert_raise Ecto.QueryError, ~r"need to explicitly pass a :select clause in query", fn -> from("posts", []) |> normalize()