Allow strings in field/2

lib/ecto/query/builder.ex

@@ -691,7 +691,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
@@ -700,7 +700,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
@@ -844,9 +844,8 @@ defmodule Ecto.Query.Builder do
       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),
-      do: {find_var!(var, vars), 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),
       do: {find_var!(var, vars), field}
@@ -1104,6 +1103,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.
   """
@@ -1113,6 +1133,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.
@@ -1278,11 +1310,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),
-      do: {find_var!(var, vars), 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

@@ -282,6 +282,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

@@ -2420,6 +2420,8 @@ defmodule Ecto.Query.Planner do
 
   defp type!(_kind, _query, _expr, nil, _field, _allow_virtuals?), do: :any
 
+  defp type!(_kind, _query, _expr, _ix, field, _allow_virtuals?) when is_binary(field), do: :any
+
   defp type!(kind, query, expr, ix, field, allow_virtuals?) when is_integer(ix) do
     case get_source!(kind, query, ix) do
       {:fragment, _, _} ->

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
@@ -266,7 +272,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
 
@@ -360,6 +366,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

@@ -448,7 +448,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>>
@@ -574,6 +574,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

@@ -1339,6 +1339,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
@@ -1353,6 +1359,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
@@ -1417,6 +1426,10 @@ defmodule Ecto.Query.PlannerTest do
     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, _, _} =
@@ -1436,6 +1449,14 @@ defmodule Ecto.Query.PlannerTest do
              "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