diff --git a/lib/ecto/query/builder.ex b/lib/ecto/query/builder.ex index 3ae1f52337..285ea40003 100644 --- a/lib/ecto/query/builder.ex +++ b/lib/ecto/query/builder.ex @@ -623,7 +623,7 @@ defmodule Ecto.Query.Builder do defp escape_field!({var, _, context}, field, vars) when is_atom(var) and is_atom(context) do var = escape_var!(var, vars) - field = quoted_atom!(field, "field/2") + field = quoted_atom_or_string!(field, "field/2") dot = {:{}, [], [:., [], [var, field]]} {:{}, [], [dot, [], []]} end @@ -632,7 +632,7 @@ defmodule Ecto.Query.Builder do when kind in [:as, :parent_as] do value = late_binding!(kind, value) as = {:{}, [], [kind, [], [value]]} - field = quoted_atom!(field, "field/2") + field = quoted_atom_or_string!(field, "field/2") dot = {:{}, [], [:., [], [as, field]]} {:{}, [], [dot, [], []]} end @@ -760,7 +760,7 @@ defmodule Ecto.Query.Builder do when is_atom(var) and is_atom(context) and is_atom(field), do: {find_var!(var, vars), field} def validate_type!({:field, _, [{var, _, context}, field]}, vars, _env) - when is_atom(var) and is_atom(context) and is_atom(field), + when is_atom(var) and is_atom(context) and (is_atom(field) or is_binary(field)), do: {find_var!(var, vars), field} def validate_type!({:field, _, [{var, _, context}, {:^, _, [field]}]}, vars, _env) when is_atom(var) and is_atom(context), @@ -988,6 +988,27 @@ defmodule Ecto.Query.Builder do "`#{Macro.to_string(other)}`" ) + @doc """ + Checks if the field is an atom or string at compilation time or + delegate the check to runtime for interpolation. + """ + def quoted_atom_or_string!({:^, _, [expr]}, used_ref), + do: quote(do: Ecto.Query.Builder.atom_or_string!(unquote(expr), unquote(used_ref))) + + def quoted_atom_or_string!(atom, _used_ref) when is_atom(atom), + do: atom + + def quoted_atom_or_string!(string, _used_ref) when is_binary(string), + do: string + + def quoted_atom_or_string!(other, used_ref), + do: + error!( + "expected literal atom or string or interpolated value in #{used_ref}, got: " <> + "`#{Macro.to_string(other)}`" + ) + + @doc """ Called by escaper at runtime to verify that value is an atom. """ @@ -997,6 +1018,18 @@ defmodule Ecto.Query.Builder do def atom!(other, used_ref), do: error!("expected atom in #{used_ref}, got: `#{inspect other}`") + @doc """ + Called by escaper at runtime to verify that value is an atomor string. + """ + def atom_or_string!(atom, _used_ref) when is_atom(atom), + do: atom + + def atom_or_string!(string, _used_ref) when is_binary(string), + do: string + + def atom_or_string!(other, used_ref), + do: error!("expected atom or string in #{used_ref}, got: `#{inspect other}`") + @doc """ Checks if the value of a late binding is an interpolation or a quoted atom. @@ -1149,11 +1182,11 @@ defmodule Ecto.Query.Builder do end def quoted_type({:field, _, [{var, _, context}, field]}, vars) - when is_atom(var) and is_atom(context) and is_atom(field), + when is_atom(var) and is_atom(context) and (is_atom(field) or is_binary(field)), do: {find_var!(var, vars), field} def quoted_type({:field, _, [{kind, _, [value]}, field]}, _vars) - when kind in [:as, :parent_as] and is_atom(field) do + when kind in [:as, :parent_as] and (is_atom(field) or is_binary(field)) do value = late_binding!(kind, value) {{:{}, [], [kind, [], [value]]}, field} end diff --git a/lib/ecto/query/inspect.ex b/lib/ecto/query/inspect.ex index faa926f2ed..107b4f6bdc 100644 --- a/lib/ecto/query/inspect.ex +++ b/lib/ecto/query/inspect.ex @@ -275,6 +275,12 @@ defimpl Inspect, for: Ecto.Query do binding_to_expr(ix, names, part) end + # Format field/2 with string name + defp postwalk({{:., _, [{_, _, _} = binding, field]}, meta, []}, _names, _part) + when is_binary(field) do + {:field, meta, [binding, field]} + end + # Remove parens from field calls defp postwalk({{:., _, [_, _]} = dot, meta, []}, _names, _part) do {dot, [no_parens: true] ++ meta, []} diff --git a/lib/ecto/query/planner.ex b/lib/ecto/query/planner.ex index 051d5d2aa6..8d93c4cd85 100644 --- a/lib/ecto/query/planner.ex +++ b/lib/ecto/query/planner.ex @@ -2019,8 +2019,13 @@ defmodule Ecto.Query.Planner do defp field_source({source, schema, _}, field) when is_binary(source) and schema != nil do # If the field is not found we return the field itself # which will be checked and raise later. - schema.__schema__(:field_source, field) || field + + case schema.__schema__(:field_source, field) do + nil -> if is_binary(field), do: String.to_existing_atom(field), else: field + field_source -> field_source + end end + defp field_source(_, field) do field end diff --git a/lib/ecto/schema.ex b/lib/ecto/schema.ex index 151bbb3dd7..d4f8dd783b 100644 --- a/lib/ecto/schema.ex +++ b/lib/ecto/schema.ex @@ -690,8 +690,14 @@ defmodule Ecto.Schema do for clauses <- Ecto.Schema.__schema__(fields, field_sources, assocs, embeds, virtual_fields, read_only), - {args, body} <- clauses do - def __schema__(unquote_splicing(args)), do: unquote(body) + clause <- clauses do + case clause do + {args, body} -> + def __schema__(unquote_splicing(args)), do: unquote(body) + + {args, when_expr, body} -> + def __schema__(unquote_splicing(args)) when(unquote(when_expr)), do: unquote(body) + end end end @@ -2312,12 +2318,18 @@ defmodule Ecto.Schema do field_sources_quoted = for {name, _type} <- fields do - {[:field_source, name], field_sources[name] || name} + field_arg = {:field, [], Elixir} + string_name = Atom.to_string(name) + when_expr = {:in, [], [field_arg, [name, string_name]]} + {[:field_source, field_arg], when_expr, field_sources[name] || name} end types_quoted = for {name, type} <- fields do - {[:type, name], Macro.escape(type)} + field_arg = {:field, [], Elixir} + string_name = Atom.to_string(name) + when_expr = {:in, [], [field_arg, [name, string_name]]} + {[:type, field_arg], when_expr, Macro.escape(type)} end virtual_types_quoted = diff --git a/test/ecto/query/builder_test.exs b/test/ecto/query/builder_test.exs index b4b3f4cb97..d30073000d 100644 --- a/test/ecto/query/builder_test.exs +++ b/test/ecto/query/builder_test.exs @@ -44,6 +44,12 @@ defmodule Ecto.Query.BuilderTest do |> Code.eval_quoted([], __ENV__) |> elem(0) + assert {{:., [], [{:&, [], [0]}, "z"]}, [], []} == + escape(quote do field(x, "z") end, [x: 0], __ENV__) + |> elem(0) + |> Code.eval_quoted([], __ENV__) + |> elem(0) + assert {Macro.escape(quote do -&0.y() end), []} == escape(quote do -x.y() end, [x: 0], __ENV__) end @@ -251,7 +257,7 @@ defmodule Ecto.Query.BuilderTest do escape(quote(do: x.y == 1), [], __ENV__) end - assert_raise Ecto.Query.CompileError, ~r"expected literal atom or interpolated value.*got: `var`", fn -> + assert_raise Ecto.Query.CompileError, ~r"expected literal atom or string or interpolated value.*got: `var`", fn -> escape(quote(do: field(x, var)), [x: 0], __ENV__) |> elem(0) |> Code.eval_quoted([], __ENV__) end @@ -345,6 +351,9 @@ defmodule Ecto.Query.BuilderTest do assert validate_type!(quote do x.title end, [x: 0], env) == {0, :title} assert validate_type!(quote do field(x, :title) end, [x: 0], env) == {0, :title} assert validate_type!(quote do field(x, ^:title) end, [x: 0], env) == {0, :title} + assert validate_type!(quote do field(x, ^:title) end, [x: 0], env) == {0, :title} + assert validate_type!(quote do field(x, "title") end, [x: 0], env) == {0, "title"} + assert validate_type!(quote do field(x, ^"title") end, [x: 0], env) == {0, "title"} assert_raise Ecto.Query.CompileError, ~r"^type/2 expects an alias, atom", fn -> validate_type!(quote do "string" end, [x: 0], env) diff --git a/test/ecto/query/inspect_test.exs b/test/ecto/query/inspect_test.exs index 826fbb01fa..a0ece93445 100644 --- a/test/ecto/query/inspect_test.exs +++ b/test/ecto/query/inspect_test.exs @@ -505,6 +505,11 @@ defmodule Ecto.Query.InspectTest do assert i(plan(query)) == "from v0 in values (#{fields})" end + test "field/2 with string name" do + query = from p in Post, select: field(p, "visit") + assert i(query) == ~s + end + def plan(query) do {query, _, _} = Ecto.Adapter.Queryable.plan_query(:all, Ecto.TestAdapter, query) query diff --git a/test/ecto/query/planner_test.exs b/test/ecto/query/planner_test.exs index 77010e3c38..35943f6a56 100644 --- a/test/ecto/query/planner_test.exs +++ b/test/ecto/query/planner_test.exs @@ -1065,6 +1065,12 @@ defmodule Ecto.Query.PlannerTest do assert Macro.to_string(hd(query.wheres).expr) == "&0.visits() == ^0" assert cast_params == [123] + {query, cast_params, _, _} = + from(Post, as: :posts, where: field(as(^as), "visits") == ^"123") |> normalize_with_params() + + assert Macro.to_string(hd(query.wheres).expr) == "&0.visits() == ^0" + assert cast_params == [123] + assert_raise Ecto.QueryError, ~r/could not find named binding `as\(:posts\)`/, fn -> from(Post, where: as(^as).visits == ^"123") |> normalize() end @@ -1079,6 +1085,9 @@ defmodule Ecto.Query.PlannerTest do query = from(Post, as: ^as, where: field(as(^as), :visits) == ^"123") |> normalize() assert Macro.to_string(hd(query.wheres).expr) == "&0.visits() == ^0" + query = from(Post, as: ^as, where: field(as(^as), "visits") == ^"123") |> normalize() + assert Macro.to_string(hd(query.wheres).expr) == "&0.visits() == ^0" + assert_raise Ecto.QueryError, ~r/could not find named binding `as\(\{:posts\}\)`/, fn -> from(Post, where: as(^as).visits == ^"123") |> normalize() end @@ -1129,6 +1138,10 @@ defmodule Ecto.Query.PlannerTest do query = from(Post, as: :posts, join: c in subquery(child), on: true) |> normalize() assert Macro.to_string(hd(query.joins).source.query.select.expr) == "%{map: parent_as(:posts).posted()}" + child = from(c in Comment, select: %{map: field(parent_as(^as), "posted")}) + query = from(Post, as: :posts, join: c in subquery(child), on: true) |> normalize() + assert Macro.to_string(hd(query.joins).source.query.select.expr) == "%{map: parent_as(:posts).posted()}" + child = from(c in Comment, where: parent_as(^as).visits == ^"123") {query, cast_params, _, _} = @@ -1144,6 +1157,14 @@ defmodule Ecto.Query.PlannerTest do assert Macro.to_string(hd(hd(query.joins).source.query.wheres).expr) == "parent_as(:posts).visits() == ^0" assert cast_params == [123] + + child = from(c in Comment, where: field(parent_as(^as), "visits") == ^"123") + + {query, cast_params, _, _} = + from(Post, as: :posts, join: c in subquery(child), on: true) |> normalize_with_params() + + assert Macro.to_string(hd(hd(query.joins).source.query.wheres).expr) == "parent_as(:posts).visits() == ^0" + assert cast_params == [123] end test "normalize: nested parent_as" do