Allow source fields in json_extract_path (#4604)

Author: greg-rychlewski

Diff for: integration_test/cases/type.exs

@@ -431,6 +431,41 @@ defmodule Ecto.Integration.TypeTest do
     assert TestRepo.one(from o in Order, where: o.metadata["tags"][0]["name"] == "red", select: o.id) == order.id
   end
 
+  @tag :map_type
+  @tag :json_extract_path_with_field
+  @tag :json_extract_path
+  test "json_extract_path with fields in path" do
+    order = %Order{id: 1, label: "tags", metadata: %{tags: [%{name: "red"}, %{name: "green"}]}}
+    order = TestRepo.insert!(order)
+
+    assert TestRepo.one(from o in Order, select: o.metadata[o.label][1]["name"]) == "green"
+    assert TestRepo.one(from o in Order, select: o.metadata["tags"][o.id]["name"]) == "green"
+
+    assert TestRepo.one(from o in Order, select: o.metadata["tags"][field(o, ^:id)]["name"]) ==
+             "green"
+
+    squery = from o in Order, select: o.metadata["tags"][parent_as(:o).id]["name"]
+    assert TestRepo.one(from o in Order, as: :o, where: subquery(squery) == ^"green")
+
+    squery = from o in Order, select: o.metadata["tags"][field(parent_as(:o), ^:id)]["name"]
+    assert TestRepo.one(from o in Order, as: :o, where: subquery(squery) == ^"green")
+
+    assert TestRepo.one(
+             from(o in Order,
+               where: o.metadata["tags"][o.id]["name"] == "green",
+               select: o.id)
+           ) == order.id
+
+    assert TestRepo.one(
+             from(o in Order,
+               where: o.metadata["tags"][field(o, ^:id)]["name"] == "green",
+               select: o.id)
+           ) == order.id
+
+    squery = from o in Order, where: o.metadata["tags"][parent_as(:o).id]["name"] == "green"
+    assert TestRepo.one(from o in Order, as: :o, where: exists(subquery(squery)))
+  end
+
   @tag :map_type
   @tag :map_type_schemaless
   test "embeds one with custom type" do

Diff for: integration_test/support/schemas.exs

@@ -281,6 +281,7 @@ defmodule Ecto.Integration.Order do
   use Ecto.Integration.Schema
 
   schema "orders" do
+    field :label, :string
     field :metadata, :map, source: :meta
     embeds_one :item, Ecto.Integration.Item
     embeds_many :items, Ecto.Integration.Item

Diff for: lib/ecto/query/api.ex

@@ -765,11 +765,19 @@ defmodule Ecto.Query.API do
 
       from(post in Post, select: post.meta["tags"][0]["name"])
 
+  Some adapters allow path elements to be references to query source fields
+
+      from(post in Post, select: post.meta[p.title])
+      from(p in Post, join: u in User, on: p.user_id == u.id, select: p.meta[u.name])
+
   Any element of the path can be dynamic:
 
       field = "name"
       from(post in Post, select: post.meta["author"][^field])
 
+      source_field = :source_column
+      from(post in Post, select: post.meta["author"][field(p, ^source_field)])
+
   ## Warning: indexes on PostgreSQL
 
   PostgreSQL supports indexing on jsonb columns via GIN indexes.

Diff for: lib/ecto/query/builder.ex

@@ -269,7 +269,7 @@ defmodule Ecto.Query.Builder do
   def escape({:json_extract_path, _, [field, path]}, type, params_acc, vars, env) do
     validate_json_field!(field)
 
-    path = escape_json_path(path)
+    path = escape_json_path(path, vars)
     {field, params_acc} = escape(field, type, params_acc, vars, env)
     {{:{}, [], [:json_extract_path, [], [field, path]]}, params_acc}
   end
@@ -1194,36 +1194,44 @@ defmodule Ecto.Query.Builder do
     end
   end
 
-  defp escape_json_path(path) when is_list(path) do
-    Enum.map(path, &quoted_json_path_element!/1)
+  defp escape_json_path(path, vars) when is_list(path) do
+    Enum.map(path, &quoted_json_path_element!(&1, vars))
   end
 
-  defp escape_json_path({:^, _, [path]}) do
+  defp escape_json_path({:^, _, [path]}, _vars) do
     quote do
       path = Ecto.Query.Builder.json_path!(unquote(path))
       Enum.map(path, &Ecto.Query.Builder.json_path_element!/1)
     end
   end
 
-  defp escape_json_path(other) do
+  defp escape_json_path(other, _vars) do
     error!(
       "expected JSON path to be a literal list or interpolated value, got: `#{Macro.to_string(other)}`"
     )
   end
 
-  defp quoted_json_path_element!({:^, _, [expr]}),
+  defp quoted_json_path_element!({:^, _, [expr]}, _vars),
     do: quote(do: Ecto.Query.Builder.json_path_element!(unquote(expr)))
 
-  defp quoted_json_path_element!(binary) when is_binary(binary),
+  defp quoted_json_path_element!(binary, _vars) when is_binary(binary),
     do: binary
 
-  defp quoted_json_path_element!(integer) when is_integer(integer),
+  defp quoted_json_path_element!(integer, _vars) when is_integer(integer),
     do: integer
 
-  defp quoted_json_path_element!(other),
+  defp quoted_json_path_element!({{:., _, [callee, field]}, _, []}, vars) do
+    escape_field!(callee, field, vars)
+  end
+
+  defp quoted_json_path_element!({:field, _, [callee, field]}, vars) do
+    escape_field!(callee, field, vars)
+  end
+
+  defp quoted_json_path_element!(other, _vars),
     do:
       error!(
-        "expected JSON path to contain literal strings, literal integers, or interpolated values, got: " <>
+        "expected JSON path to contain literal strings, literal integers, fields, or interpolated values, got: " <>
           "`#{Macro.to_string(other)}`"
       )
 

Diff for: lib/ecto/query/planner.ex

@@ -1441,7 +1441,8 @@ defmodule Ecto.Query.Planner do
     {Enum.reverse(combinations), counter}
   end
 
-  defp validate_json_path!([path_field | rest], field, {:parameterized, {Ecto.Embedded, embed}}) do
+  defp validate_json_path!([path_field | rest], field, {:parameterized, {Ecto.Embedded, embed}})
+       when is_binary(path_field) or is_integer(path_field) do
     case embed do
       %{related: related, cardinality: :one} ->
         unless Enum.any?(related.__schema__(:fields), &(Atom.to_string(&1) == path_field)) do
@@ -1464,6 +1465,26 @@ defmodule Ecto.Query.Planner do
     end
   end
 
+  defp validate_json_path!([path_field | rest], field, {:parameterized, {Ecto.Embedded, embed}}) do
+    case embed do
+      %{related: _, cardinality: :one} ->
+        # A source field cannot be used to validate whether the next step in the
+        # path exists in the embedded schema, so we stop here. If there is an error
+        # later in the path it will be caught by the driver.
+        :ok
+
+      %{related: _, cardinality: :many} ->
+        # The source field may not be an integer but for the sake of validating
+        # the rest of the path, we assume it is. The error will be caught later
+        # by the driver if it is not.
+        updated_embed = %{embed | cardinality: :one}
+        validate_json_path!(rest, path_field, {:parameterized, {Ecto.Embedded, updated_embed}})
+
+      other ->
+        raise "expected field `#{field}` to be of type embed, got: `#{inspect(other)}`"
+    end
+  end
+
   defp validate_json_path!([_path_field | _rest] = path, field, other_type) do
     case Ecto.Type.type(other_type) do
       :any ->

Diff for: test/ecto/query/builder_test.exs

@@ -279,6 +279,19 @@ defmodule Ecto.Query.BuilderTest do
 
     assert actual == expected
 
+    expected = {Macro.escape(quote do: json_extract_path(&0.y(), [0, &0.z(), "a"])), []}
+
+    actual =
+      escape(
+        quote do
+          x.y[0][x.z]["a"]
+        end,
+        [x: 0],
+        __ENV__
+      )
+
+    assert actual == expected
+
     assert_raise Ecto.Query.CompileError, "`x` is not a valid json field", fn ->
       escape(
         quote do

Diff for: test/ecto/query/planner_test.exs

@@ -1663,6 +1663,18 @@ defmodule Ecto.Query.PlannerTest do
     query = from(Post, []) |> select([p], p.metas[0]["slug"])
     normalize(query)
 
+    query = from(Post, []) |> select([p], p.meta[p.title])
+    normalize(query)
+
+    query = from(Post, []) |> select([p], p.meta[p.title]["author"])
+    normalize(query)
+
+    query = from(Post, []) |> select([p], p.meta["author"][p.title])
+    normalize(query)
+
+    query = from(Post, []) |> select([p], p.metas[p.visits]["slug"])
+    normalize(query)
+
     query = from(Post, []) |> select([p], p.payload["unknown_field"])
     normalize(query)
 
@@ -1715,6 +1727,13 @@ defmodule Ecto.Query.PlannerTest do
                    normalize(query)
                  end
 
+    assert_raise RuntimeError,
+                 "field `unknown_field` does not exist in Ecto.Query.PlannerTest.PostMeta",
+                 fn ->
+                   query = from(Post, []) |> select([p], p.metas[p.visits]["unknown_field"])
+                   normalize(query)
+                 end
+
     assert_raise RuntimeError,
                  "field `0` does not exist in Ecto.Query.PlannerTest.PostMeta",
                  fn ->