From 193a1c0bee442f9ea79fdf1a7cf8e3af7dba58ac Mon Sep 17 00:00:00 2001 From: Greg Rychlewski Date: Fri, 11 Apr 2025 20:34:01 -0400 Subject: [PATCH 1/5] allow fields in json_extract_path --- integration_test/cases/type.exs | 34 ++++++++++++++++++++++++++++++++ lib/ecto/query/builder.ex | 28 ++++++++++++++++---------- test/ecto/query/builder_test.exs | 13 ++++++++++++ 3 files changed, 65 insertions(+), 10 deletions(-) diff --git a/integration_test/cases/type.exs b/integration_test/cases/type.exs index f0b52b513d..004512afd3 100644 --- a/integration_test/cases/type.exs +++ b/integration_test/cases/type.exs @@ -431,6 +431,40 @@ 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, metadata: %{tags: [%{name: "red"}, %{name: "green"}]}} + order = TestRepo.insert!(order) + + 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 --git a/lib/ecto/query/builder.ex b/lib/ecto/query/builder.ex index f1e3256bcc..d3faacb384 100644 --- a/lib/ecto/query/builder.ex +++ b/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, "ed_json_path_element!/1) + defp escape_json_path(path, vars) when is_list(path) do + Enum.map(path, "ed_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 --git a/test/ecto/query/builder_test.exs b/test/ecto/query/builder_test.exs index 0e871244aa..0e164a6c77 100644 --- a/test/ecto/query/builder_test.exs +++ b/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 From 8ad722a9f04cbda75f38e7e56e73e96c4ca9c487 Mon Sep 17 00:00:00 2001 From: Greg Rychlewski Date: Fri, 11 Apr 2025 23:00:19 -0400 Subject: [PATCH 2/5] update embed validation --- lib/ecto/query/planner.ex | 23 ++++++++++++++++++++++- test/ecto/query/planner_test.exs | 19 +++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/lib/ecto/query/planner.ex b/lib/ecto/query/planner.ex index 0f1a7a050b..2b7b2ff5a0 100644 --- a/lib/ecto/query/planner.ex +++ b/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 --git a/test/ecto/query/planner_test.exs b/test/ecto/query/planner_test.exs index 06c84e25d7..5e32c12704 100644 --- a/test/ecto/query/planner_test.exs +++ b/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 -> From 26c630e37c046840ede15f528d7b006c9666d04c Mon Sep 17 00:00:00 2001 From: Greg Rychlewski Date: Sat, 12 Apr 2025 09:19:25 -0400 Subject: [PATCH 3/5] add to docs --- lib/ecto/query/api.ex | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/ecto/query/api.ex b/lib/ecto/query/api.ex index 984074037e..c6dc96efcd 100644 --- a/lib/ecto/query/api.ex +++ b/lib/ecto/query/api.ex @@ -765,11 +765,19 @@ defmodule Ecto.Query.API do from(post in Post, select: post.meta["tags"][0]["name"]) + Path elements can be references to fields on query sources + + 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. From 7e00ae49a16ab7d76911a2605ae4fb07a42d9eda Mon Sep 17 00:00:00 2001 From: Greg Rychlewski Date: Sat, 12 Apr 2025 18:05:22 -0400 Subject: [PATCH 4/5] test strings as well as ints --- integration_test/cases/type.exs | 3 ++- integration_test/support/schemas.exs | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/integration_test/cases/type.exs b/integration_test/cases/type.exs index 004512afd3..47d08b0036 100644 --- a/integration_test/cases/type.exs +++ b/integration_test/cases/type.exs @@ -435,9 +435,10 @@ defmodule Ecto.Integration.TypeTest do @tag :json_extract_path_with_field @tag :json_extract_path test "json_extract_path with fields in path" do - order = %Order{id: 1, metadata: %{tags: [%{name: "red"}, %{name: "green"}]}} + 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"]) == diff --git a/integration_test/support/schemas.exs b/integration_test/support/schemas.exs index 5dbb78f707..e5b6559945 100644 --- a/integration_test/support/schemas.exs +++ b/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 From c2df1d4488e722816d4d53e047cc25326e05a3ac Mon Sep 17 00:00:00 2001 From: Greg Rychlewski Date: Sun, 13 Apr 2025 08:46:06 -0400 Subject: [PATCH 5/5] small doc change --- lib/ecto/query/api.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ecto/query/api.ex b/lib/ecto/query/api.ex index c6dc96efcd..eb942c925f 100644 --- a/lib/ecto/query/api.ex +++ b/lib/ecto/query/api.ex @@ -765,7 +765,7 @@ defmodule Ecto.Query.API do from(post in Post, select: post.meta["tags"][0]["name"]) - Path elements can be references to fields on query sources + 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])