Repository: elixir-ecto/ecto
Pull request #4280 (2023-09-19 03:46:06)

Respect late binding types
I was reading Elixir Forum and came across a post that made it seem like as/parent_as types weren't being respected. Reference: https://elixirforum.com/t/ecto-types-and-named-bindiings/58370

When I looked into it I found that `as/parent_as` and `field(as/parent_as, _)` were having their types set to `:any`. When I fixed this I also noticed some tests started failing. So it actually uncovered some faulty tests:

1. Some of the late binding tests were comparing the `code` field to an integer when code is a binary. So casting should fail. When I made my changes the tests started failing. I updated the tests to cast to an integer field to make them pass and to ensure casting was being done properly.
2. Some of the parent_as tests were referencing a field that didn't exist on the parent. Namely, a `deleted` field on the posts schema. It wasn't failing before because the type of the field wasn't being looked up. Now that it's being looked up it failed. I fixed the test in a similar manner described above.


Patch:
diff --git a/lib/ecto/query/builder.ex b/lib/ecto/query/builder.ex
index 2a2ee10a..6e1770b2 100644
--- a/lib/ecto/query/builder.ex
+++ b/lib/ecto/query/builder.ex
@@ -629,14 +629,7 @@ defmodule Ecto.Query.Builder do
 
   defp escape_field!({kind, _, [value]}, field, _vars)
        when kind in [:as, :parent_as] do
-    value =
-      case value do
-        {:^, _, [value]} ->
-          value
-
-        other ->
-          quoted_atom!(other, "#{kind}/1")
-      end
+    value = late_binding!(kind, value)
     as    = {:{}, [], [kind, [], [value]]}
     field = quoted_atom!(field, "field/2")
     dot   = {:{}, [], [:., [], [as, field]]}
@@ -1003,6 +996,20 @@ defmodule Ecto.Query.Builder do
   def atom!(other, used_ref),
     do: error!("expected atom in #{used_ref}, got: `#{inspect other}`")
 
+  @doc """
+  Checks if the value of a late binding is an interpolation or
+  a quoted atom.
+  """
+  def late_binding!(kind, value) do
+    case value do
+      {:^, _, [value]} ->
+        value
+
+      other ->
+        quoted_atom!(other, "#{kind}/1")
+    end
+  end
+
   defp escape_json_path(path) when is_list(path) do
     Enum.map(path, &quoted_json_path_element!/1)
   end
@@ -1140,10 +1147,22 @@ defmodule Ecto.Query.Builder do
     when is_atom(var) and is_atom(context) and is_atom(field),
     do: {find_var!(var, vars), field}
 
+  def quoted_type({{:., _, [{kind, _, [value]}, field]}, _, []}, _vars)
+      when kind in [:as, :parent_as] do
+    value = late_binding!(kind, value)
+    {{:{}, [], [kind, [], [value]]}, field}
+  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}
 
+  def quoted_type({:field, _, [{kind, _, [value]}, field]}, _vars)
+      when kind in [:as, :parent_as] and is_atom(field) do
+    value = late_binding!(kind, value)
+    {{:{}, [], [kind, [], [value]]}, field}
+  end
+
   # Unquoting code here means the second argument of field will
   # always be unquoted twice, one by the type checking and another
   # in the query itself. We are assuming this is not an issue
@@ -1152,6 +1171,12 @@ defmodule Ecto.Query.Builder do
     when is_atom(var) and is_atom(context),
     do: {find_var!(var, vars), code}
 
+  def quoted_type({:field, _, [{kind, _, [value]}, {:^, _, [code]}]}, _vars)
+      when kind in [:as, :parent_as] do
+    value = late_binding!(kind, value)
+    {{:{}, [], [kind, [], [value]]}, code}
+  end
+
   # Interval
   def quoted_type({:datetime_add, _, [_, _, _]}, _vars), do: :naive_datetime
   def quoted_type({:date_add, _, [_, _, _]}, _vars), do: :date
diff --git a/lib/ecto/query/planner.ex b/lib/ecto/query/planner.ex
index dce2198a..cd767915 100644
--- a/lib/ecto/query/planner.ex
+++ b/lib/ecto/query/planner.ex
@@ -1844,6 +1844,12 @@ defmodule Ecto.Query.Planner do
     {composite, type!(kind, query, expr, ix, field, allow_virtuals?)}
   end
 
+  defp field_type!(kind, query, expr, {{bind_kind, _, [_]} = bind_expr, field}, allow_virtuals?)
+       when bind_kind in [:as, :parent_as] do
+    {ix, _, ix_query} = get_ix!(bind_expr, kind, query)
+    type!(kind, ix_query, expr, ix, field, allow_virtuals?)
+  end
+
   defp field_type!(kind, query, expr, {ix, field}, allow_virtuals?) when is_integer(ix) do
     type!(kind, query, expr, ix, field, allow_virtuals?)
   end


Test patch:
diff --git a/test/ecto/query/planner_test.exs b/test/ecto/query/planner_test.exs
index 970b4627..54f1765c 100644
--- a/test/ecto/query/planner_test.exs
+++ b/test/ecto/query/planner_test.exs
@@ -992,43 +992,52 @@ defmodule Ecto.Query.PlannerTest do
   end
 
   test "normalize: late bindings with as" do
-    query = from(Post, as: :posts, where: as(:posts).code == ^123) |> normalize()
-    assert Macro.to_string(hd(query.wheres).expr) == "&0.code() == ^0"
+    {query, cast_params, _, _} =
+      from(Post, as: :posts, where: as(:posts).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(:posts).code == ^123) |> normalize()
+      from(Post, where: as(:posts).visits == ^"123") |> normalize()
     end
   end
 
   test "normalize: late dynamic bindings with as" do
     as = :posts
 
-    query = from(Post, as: :posts, where: as(^as).code == ^123) |> normalize()
-    assert Macro.to_string(hd(query.wheres).expr) == "&0.code() == ^0"
+    {query, cast_params, _, _} =
+      from(Post, as: :posts, where: as(^as).visits == ^"123") |> normalize_with_params()
+
+    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()
 
-    query = from(Post, as: :posts, where: field(as(^as), :code) == ^123) |> normalize()
-    assert Macro.to_string(hd(query.wheres).expr) == "&0.code() == ^0"
+    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).code == ^123) |> normalize()
+      from(Post, where: as(^as).visits == ^"123") |> normalize()
     end
   end
 
   test "normalize: creating dynamic bindings with as" do
     as = {:posts}
 
-    query = from(Post, as: ^as, where: as(^as).code == ^123) |> normalize()
-    assert Macro.to_string(hd(query.wheres).expr) == "&0.code() == ^0"
+    query = from(Post, as: ^as, where: 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), :code) == ^123) |> normalize()
-    assert Macro.to_string(hd(query.wheres).expr) == "&0.code() == ^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).code == ^123) |> normalize()
+      from(Post, where: as(^as).visits == ^"123") |> normalize()
     end
 
-    query = from(Post, as: ^as, where: as(^as).code == ^123) |> normalize()
-    assert Macro.to_string(hd(query.wheres).expr) == "&0.code() == ^0"
+    query = from(Post, as: ^as, where: as(^as).visits == ^"123") |> normalize()
+    assert Macro.to_string(hd(query.wheres).expr) == "&0.visits() == ^0"
   end
 
   test "normalize: late parent bindings with as" do
@@ -1040,6 +1049,14 @@ 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, where: parent_as(:posts).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]
+
     assert_raise Ecto.SubQueryError, ~r/the parent_as in a subquery select used as a join can only access the `from` binding in query/, fn ->
       child = from(c in Comment, select: %{map: parent_as(:itself).posted})
       from(Post, as: :posts, join: c in subquery(child), on: true, as: :itself) |> normalize()
@@ -1064,10 +1081,26 @@ defmodule Ecto.Query.PlannerTest do
     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, _, _} =
+      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]
+
+    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
-    child3 = from(c in Comment, where: parent_as(:posts).deleted == false, select: c.id)
+    child3 = from(c in Comment, where: parent_as(:posts).visits > 0, select: c.id)
     child2 = from(c in Comment, where: c.id in subquery(child3), select: c.id)
     child = from(c in Comment, where: parent_as(:posts).posted == c.posted and c.id in subquery(child2))
 
@@ -1078,7 +1111,7 @@ defmodule Ecto.Query.PlannerTest do
 
   test "normalize: nested dynamic parent_as" do
     as = :posts
-    child3 = from(c in Comment, where: parent_as(^as).deleted == false, select: c.id)
+    child3 = from(c in Comment, where: parent_as(^as).visits > 0, select: c.id)
     child2 = from(c in Comment, where: c.id in subquery(child3), select: c.id)
     child = from(c in Comment, where: field(parent_as(^as), :posted) == c.posted and c.id in subquery(child2))