Allow strings in field/2

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

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, []}

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

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 =

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)

test/ecto/query/inspect_test.exs

@@ -396,7 +396,7 @@
   # TODO: AST is represented as string differently on versions pre 1.13
   if Version.match?(System.version(), ">= 1.13.0-dev") do
     test "container values" do
       assert i(from(Post, select: <<1, 2, 3>>)) ==
              "from p0 in Inspect.Post, select: \"\\x01\\x02\\x03\""
 
       foo = <<1, 2, 3>>
@@ -405,7 +405,7 @@
     end
   else
     test "container values" do
       assert i(from(Post, select: <<1, 2, 3>>)) ==
              "from p0 in Inspect.Post, select: <<1, 2, 3>>"
 
       foo = <<1, 2, 3>>
@@ -505,6 +505,11 @@
     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<from p0 in Inspect.Post, select: field(p0, "visit")>
+  end
+
   def plan(query) do
     {query, _, _} = Ecto.Adapter.Queryable.plan_query(:all, Ecto.TestAdapter, query)
     query

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