From 5ccc643be2cf8b30159100ee1a048465c7ae9f6d Mon Sep 17 00:00:00 2001 From: Greg Rychlewski Date: Fri, 11 Apr 2025 20:37:36 -0400 Subject: [PATCH 1/7] postgres fields in json_extract_path --- integration_test/myxql/test_helper.exs | 4 ++- lib/ecto/adapters/postgres/connection.ex | 23 +++++++++++-- mix.exs | 2 +- mix.lock | 2 +- test/ecto/adapters/postgres_test.exs | 41 ++++++++++++++++++++---- 5 files changed, 60 insertions(+), 12 deletions(-) diff --git a/integration_test/myxql/test_helper.exs b/integration_test/myxql/test_helper.exs index 922f4d68..40a8aa73 100644 --- a/integration_test/myxql/test_helper.exs +++ b/integration_test/myxql/test_helper.exs @@ -119,7 +119,9 @@ excludes = [ # MySQL doesn't support specifying columns for ON DELETE SET NULL :on_delete_nilify_column_list, # MySQL doesnt' support anything except a single column in DISTINCT - :multicolumn_distinct + :multicolumn_distinct, + # need to update adapter to accept fields in paths from Ecto + :json_extract_path_with_field ] if Version.match?(version, ">= 8.0.0") do diff --git a/lib/ecto/adapters/postgres/connection.ex b/lib/ecto/adapters/postgres/connection.ex index 9a7c4783..53b71f13 100644 --- a/lib/ecto/adapters/postgres/connection.ex +++ b/lib/ecto/adapters/postgres/connection.ex @@ -1141,8 +1141,8 @@ if Code.ensure_loaded?(Postgrex) do end defp json_extract_path(expr, path, sources, query) do - path = Enum.map_intersperse(path, ?,, &escape_json/1) - [?(, expr(expr, sources, query), "#>'{", path, "}')"] + path = Enum.map_intersperse(path, ?,, &escape_json(&1, sources, query)) + [?(, expr(expr, sources, query), "#>array[", path, "]::text[])"] end defp values_list(types, idx, num_rows) do @@ -2022,6 +2022,25 @@ if Code.ensure_loaded?(Postgrex) do defp escape_json(true), do: ["true"] defp escape_json(false), do: ["false"] + # For unoptimized json_extract_path we allow columns to be + # a part of the path. To allow this, we use the array[...] syntax + # which requires special handling for strings and column references + defp escape_json(value, _, _) when is_binary(value) do + [?', escape_string(value), ?'] + end + + defp escape_json({{:., _, [{:&, _, [_]}, _]}, _, []} = expr, sources, query) do + expr(expr, sources, query) + end + + defp escape_json({{:., _, [{:parent_as, _, [_]}, _]}, _, []} = expr, sources, query) do + expr(expr, sources, query) + end + + defp escape_json(other, _, _) do + escape_json(other) + end + defp ecto_to_db({:array, t}), do: [ecto_to_db(t), ?[, ?]] defp ecto_to_db(:id), do: "integer" defp ecto_to_db(:identity), do: "bigint" diff --git a/mix.exs b/mix.exs index 59b79ffd..d6e537e4 100644 --- a/mix.exs +++ b/mix.exs @@ -76,7 +76,7 @@ defmodule EctoSQL.MixProject do if path = System.get_env("ECTO_PATH") do {:ecto, path: path} else - {:ecto, github: "elixir-ecto/ecto"} + {:ecto, github: "greg-rychlewski/ecto", branch: "json_path_with_fields"} end end diff --git a/mix.lock b/mix.lock index e41ac79d..0c109c62 100644 --- a/mix.lock +++ b/mix.lock @@ -4,7 +4,7 @@ "decimal": {:hex, :decimal, "2.3.0", "3ad6255aa77b4a3c4f818171b12d237500e63525c2fd056699967a3e7ea20f62", [:mix], [], "hexpm", "a4d66355cb29cb47c3cf30e71329e58361cfcb37c34235ef3bf1d7bf3773aeac"}, "deep_merge": {:hex, :deep_merge, "1.0.0", "b4aa1a0d1acac393bdf38b2291af38cb1d4a52806cf7a4906f718e1feb5ee961", [:mix], [], "hexpm", "ce708e5f094b9cd4e8f2be4f00d2f4250c4095be93f8cd6d018c753894885430"}, "earmark_parser": {:hex, :earmark_parser, "1.4.41", "ab34711c9dc6212dda44fcd20ecb87ac3f3fce6f0ca2f28d4a00e4154f8cd599", [:mix], [], "hexpm", "a81a04c7e34b6617c2792e291b5a2e57ab316365c2644ddc553bb9ed863ebefa"}, - "ecto": {:git, "https://github.com/elixir-ecto/ecto.git", "b02e921b7d2a2a4d5a73fe0ead6500a6bec9d207", []}, + "ecto": {:git, "https://github.com/greg-rychlewski/ecto.git", "193a1c0bee442f9ea79fdf1a7cf8e3af7dba58ac", [branch: "json_path_with_fields"]}, "ex_doc": {:hex, :ex_doc, "0.34.2", "13eedf3844ccdce25cfd837b99bea9ad92c4e511233199440488d217c92571e8", [:mix], [{:earmark_parser, "~> 1.4.39", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_c, ">= 0.1.0", [hex: :makeup_c, repo: "hexpm", optional: true]}, {:makeup_elixir, "~> 0.14 or ~> 1.0", [hex: :makeup_elixir, repo: "hexpm", optional: false]}, {:makeup_erlang, "~> 0.1 or ~> 1.0", [hex: :makeup_erlang, repo: "hexpm", optional: false]}, {:makeup_html, ">= 0.1.0", [hex: :makeup_html, repo: "hexpm", optional: true]}], "hexpm", "5ce5f16b41208a50106afed3de6a2ed34f4acfd65715b82a0b84b49d995f95c1"}, "jason": {:hex, :jason, "1.4.4", "b9226785a9aa77b6857ca22832cffa5d5011a667207eb2a0ad56adb5db443b8a", [:mix], [{:decimal, "~> 1.0 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "c5eb0cab91f094599f94d55bc63409236a8ec69a21a67814529e8d5f6cc90b3b"}, "makeup": {:hex, :makeup, "1.1.2", "9ba8837913bdf757787e71c1581c21f9d2455f4dd04cfca785c70bbfff1a76a3", [:mix], [{:nimble_parsec, "~> 1.2.2 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "cce1566b81fbcbd21eca8ffe808f33b221f9eee2cbc7a1706fc3da9ff18e6cac"}, diff --git a/test/ecto/adapters/postgres_test.exs b/test/ecto/adapters/postgres_test.exs index 90dbc495..02633427 100644 --- a/test/ecto/adapters/postgres_test.exs +++ b/test/ecto/adapters/postgres_test.exs @@ -963,16 +963,36 @@ defmodule Ecto.Adapters.PostgresTest do test "json_extract_path" do query = Schema |> select([s], json_extract_path(s.meta, [0, 1])) |> plan() - assert all(query) == ~s|SELECT (s0.\"meta\"#>'{0,1}') FROM "schema" AS s0| + assert all(query) == ~s|SELECT (s0.\"meta\"#>array[0,1]::text[]) FROM "schema" AS s0| query = Schema |> select([s], json_extract_path(s.meta, ["a", "b"])) |> plan() - assert all(query) == ~s|SELECT (s0.\"meta\"#>'{"a","b"}') FROM "schema" AS s0| + assert all(query) == ~s|SELECT (s0.\"meta\"#>array['a','b']::text[]) FROM "schema" AS s0| query = Schema |> select([s], json_extract_path(s.meta, ["'a"])) |> plan() - assert all(query) == ~s|SELECT (s0.\"meta\"#>'{"''a"}') FROM "schema" AS s0| + assert all(query) == ~s|SELECT (s0.\"meta\"#>array['''a']::text[]) FROM "schema" AS s0| query = Schema |> select([s], json_extract_path(s.meta, ["\"a"])) |> plan() - assert all(query) == ~s|SELECT (s0.\"meta\"#>'{"\\"a"}') FROM "schema" AS s0| + assert all(query) == ~s|SELECT (s0.\"meta\"#>array['\"a']::text[]) FROM "schema" AS s0| + + query = Schema |> select([s], json_extract_path(s.meta, [s.x])) |> plan() + assert all(query) == ~s|SELECT (s0.\"meta\"#>array[s0.\"x\"]::text[]) FROM "schema" AS s0| + + query = Schema |> select([s], json_extract_path(s.meta, ["a", s.x, 0])) |> plan() + + assert all(query) == + ~s|SELECT (s0.\"meta\"#>array['a',s0.\"x\",0]::text[]) FROM "schema" AS s0| + + squery = + Schema + |> where([s], is_nil(json_extract_path(s.meta, [parent_as(:s).x]))) + |> select([s], s.x) + + query = + Schema |> from(as: :s) |> where([s], s.x in subquery(squery)) |> select([s], s.x) |> plan() + + assert all(query) == + ~s|SELECT s0.\"x\" FROM "schema" AS s0 WHERE (s0.\"x\" IN | <> + ~s|(SELECT ss0.\"x\" FROM \"schema\" AS ss0 WHERE ((ss0.\"meta\"#>array[s0.\"x\"]::text[]) IS NULL)))| end test "optimized json_extract_path" do @@ -985,10 +1005,12 @@ defmodule Ecto.Adapters.PostgresTest do query = Schema |> where([s], s.meta["tags"][0]["name"] == "123") |> select(true) |> plan() assert all(query) == - ~s|SELECT TRUE FROM "schema" AS s0 WHERE (((s0."meta"#>'{"tags",0}')@>'{"name": "123"}'))| + ~s|SELECT TRUE FROM "schema" AS s0 WHERE (((s0."meta"#>array['tags',0]::text[])@>'{"name": "123"}'))| query = Schema |> where([s], s.meta[0] == "123") |> select(true) |> plan() - assert all(query) == ~s|SELECT TRUE FROM "schema" AS s0 WHERE ((s0.\"meta\"#>'{0}') = '123')| + + assert all(query) == + ~s|SELECT TRUE FROM "schema" AS s0 WHERE ((s0.\"meta\"#>array[0]::text[]) = '123')| query = Schema |> where([s], s.meta["enabled"] == true) |> select(true) |> plan() @@ -998,7 +1020,12 @@ defmodule Ecto.Adapters.PostgresTest do query = Schema |> where([s], s.meta["extra"][0]["enabled"] == false) |> select(true) |> plan() assert all(query) == - ~s|SELECT TRUE FROM "schema" AS s0 WHERE (((s0."meta"#>'{"extra",0}')@>'{"enabled": false}'))| + ~s|SELECT TRUE FROM "schema" AS s0 WHERE (((s0."meta"#>array['extra',0]::text[])@>'{"enabled": false}'))| + + query = Schema |> where([s], s.meta[s.x][0]["name"] == "123") |> select(true) |> plan() + + assert all(query) == + ~s|SELECT TRUE FROM "schema" AS s0 WHERE (((s0."meta"#>array[s0.\"x\",0]::text[])@>'{"name": "123"}'))| end test "nested expressions" do From 5c46f5f47fa339ded41eeddd3a0e15eaf15f25bc Mon Sep 17 00:00:00 2001 From: Greg Rychlewski Date: Fri, 11 Apr 2025 20:48:56 -0400 Subject: [PATCH 2/7] better comment --- lib/ecto/adapters/postgres/connection.ex | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/ecto/adapters/postgres/connection.ex b/lib/ecto/adapters/postgres/connection.ex index 53b71f13..bb045a54 100644 --- a/lib/ecto/adapters/postgres/connection.ex +++ b/lib/ecto/adapters/postgres/connection.ex @@ -2022,9 +2022,10 @@ if Code.ensure_loaded?(Postgrex) do defp escape_json(true), do: ["true"] defp escape_json(false), do: ["false"] - # For unoptimized json_extract_path we allow columns to be - # a part of the path. To allow this, we use the array[...] syntax - # which requires special handling for strings and column references + # To allow columns in json paths, we use the array[...] syntax + # which requires special handling for strings and column references. + # We still keep the escape_json/1 variant for strings because it is + # needed for the queries using @> defp escape_json(value, _, _) when is_binary(value) do [?', escape_string(value), ?'] end From b8995d725dff49badad13687eb0abc2c3a4a9588 Mon Sep 17 00:00:00 2001 From: Greg Rychlewski Date: Sat, 12 Apr 2025 18:11:58 -0400 Subject: [PATCH 3/7] string field for integration test --- integration_test/support/migration.exs | 1 + 1 file changed, 1 insertion(+) diff --git a/integration_test/support/migration.exs b/integration_test/support/migration.exs index f4f0229a..6ae53bc0 100644 --- a/integration_test/support/migration.exs +++ b/integration_test/support/migration.exs @@ -89,6 +89,7 @@ defmodule Ecto.Integration.Migration do end create table(:orders) do + add :label, :string add :item, :map add :items, :map add :meta, :map From 391060953d9ff60d4c09690c622c6ee638abe0ad Mon Sep 17 00:00:00 2001 From: Greg Rychlewski Date: Sat, 12 Apr 2025 18:18:02 -0400 Subject: [PATCH 4/7] raise for mysql --- lib/ecto/adapters/myxql/connection.ex | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/ecto/adapters/myxql/connection.ex b/lib/ecto/adapters/myxql/connection.ex index 43aaf2b8..92d36d0c 100644 --- a/lib/ecto/adapters/myxql/connection.ex +++ b/lib/ecto/adapters/myxql/connection.ex @@ -812,6 +812,12 @@ if Code.ensure_loaded?(MyXQL) do integer when is_integer(integer) -> "[#{integer}]" + + _ -> + error!( + query, + "MySQL adapter does not support references to source fields inside of `json_extract_path`" + ) end) ["json_extract(", expr(expr, sources, query), ", '$", path, "')"] From 148d2fcf42e4f6979e1900ba8b034892f05a6a37 Mon Sep 17 00:00:00 2001 From: Greg Rychlewski Date: Sat, 12 Apr 2025 18:21:28 -0400 Subject: [PATCH 5/7] update comment --- integration_test/myxql/test_helper.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration_test/myxql/test_helper.exs b/integration_test/myxql/test_helper.exs index 40a8aa73..1c2b4f5b 100644 --- a/integration_test/myxql/test_helper.exs +++ b/integration_test/myxql/test_helper.exs @@ -120,7 +120,7 @@ excludes = [ :on_delete_nilify_column_list, # MySQL doesnt' support anything except a single column in DISTINCT :multicolumn_distinct, - # need to update adapter to accept fields in paths from Ecto + # uncertain whether we can support this. needs more exploring :json_extract_path_with_field ] From 50004ae1ed22af89cccdc12ae3a5b5bfa6ca4a05 Mon Sep 17 00:00:00 2001 From: Greg Rychlewski Date: Sun, 13 Apr 2025 08:52:42 -0400 Subject: [PATCH 6/7] test against latest ecto changes --- mix.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mix.lock b/mix.lock index 0c109c62..e17c6783 100644 --- a/mix.lock +++ b/mix.lock @@ -4,7 +4,7 @@ "decimal": {:hex, :decimal, "2.3.0", "3ad6255aa77b4a3c4f818171b12d237500e63525c2fd056699967a3e7ea20f62", [:mix], [], "hexpm", "a4d66355cb29cb47c3cf30e71329e58361cfcb37c34235ef3bf1d7bf3773aeac"}, "deep_merge": {:hex, :deep_merge, "1.0.0", "b4aa1a0d1acac393bdf38b2291af38cb1d4a52806cf7a4906f718e1feb5ee961", [:mix], [], "hexpm", "ce708e5f094b9cd4e8f2be4f00d2f4250c4095be93f8cd6d018c753894885430"}, "earmark_parser": {:hex, :earmark_parser, "1.4.41", "ab34711c9dc6212dda44fcd20ecb87ac3f3fce6f0ca2f28d4a00e4154f8cd599", [:mix], [], "hexpm", "a81a04c7e34b6617c2792e291b5a2e57ab316365c2644ddc553bb9ed863ebefa"}, - "ecto": {:git, "https://github.com/greg-rychlewski/ecto.git", "193a1c0bee442f9ea79fdf1a7cf8e3af7dba58ac", [branch: "json_path_with_fields"]}, + "ecto": {:git, "https://github.com/greg-rychlewski/ecto.git", "c2df1d4488e722816d4d53e047cc25326e05a3ac", [branch: "json_path_with_fields"]}, "ex_doc": {:hex, :ex_doc, "0.34.2", "13eedf3844ccdce25cfd837b99bea9ad92c4e511233199440488d217c92571e8", [:mix], [{:earmark_parser, "~> 1.4.39", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_c, ">= 0.1.0", [hex: :makeup_c, repo: "hexpm", optional: true]}, {:makeup_elixir, "~> 0.14 or ~> 1.0", [hex: :makeup_elixir, repo: "hexpm", optional: false]}, {:makeup_erlang, "~> 0.1 or ~> 1.0", [hex: :makeup_erlang, repo: "hexpm", optional: false]}, {:makeup_html, ">= 0.1.0", [hex: :makeup_html, repo: "hexpm", optional: true]}], "hexpm", "5ce5f16b41208a50106afed3de6a2ed34f4acfd65715b82a0b84b49d995f95c1"}, "jason": {:hex, :jason, "1.4.4", "b9226785a9aa77b6857ca22832cffa5d5011a667207eb2a0ad56adb5db443b8a", [:mix], [{:decimal, "~> 1.0 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "c5eb0cab91f094599f94d55bc63409236a8ec69a21a67814529e8d5f6cc90b3b"}, "makeup": {:hex, :makeup, "1.1.2", "9ba8837913bdf757787e71c1581c21f9d2455f4dd04cfca785c70bbfff1a76a3", [:mix], [{:nimble_parsec, "~> 1.2.2 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "cce1566b81fbcbd21eca8ffe808f33b221f9eee2cbc7a1706fc3da9ff18e6cac"}, From af10aff39fb96872c89917658187ceff31e63218 Mon Sep 17 00:00:00 2001 From: Greg Rychlewski Date: Sun, 13 Apr 2025 08:59:45 -0400 Subject: [PATCH 7/7] point back to real ecto --- mix.exs | 2 +- mix.lock | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/mix.exs b/mix.exs index d6e537e4..59b79ffd 100644 --- a/mix.exs +++ b/mix.exs @@ -76,7 +76,7 @@ defmodule EctoSQL.MixProject do if path = System.get_env("ECTO_PATH") do {:ecto, path: path} else - {:ecto, github: "greg-rychlewski/ecto", branch: "json_path_with_fields"} + {:ecto, github: "elixir-ecto/ecto"} end end diff --git a/mix.lock b/mix.lock index e17c6783..883473fd 100644 --- a/mix.lock +++ b/mix.lock @@ -4,7 +4,7 @@ "decimal": {:hex, :decimal, "2.3.0", "3ad6255aa77b4a3c4f818171b12d237500e63525c2fd056699967a3e7ea20f62", [:mix], [], "hexpm", "a4d66355cb29cb47c3cf30e71329e58361cfcb37c34235ef3bf1d7bf3773aeac"}, "deep_merge": {:hex, :deep_merge, "1.0.0", "b4aa1a0d1acac393bdf38b2291af38cb1d4a52806cf7a4906f718e1feb5ee961", [:mix], [], "hexpm", "ce708e5f094b9cd4e8f2be4f00d2f4250c4095be93f8cd6d018c753894885430"}, "earmark_parser": {:hex, :earmark_parser, "1.4.41", "ab34711c9dc6212dda44fcd20ecb87ac3f3fce6f0ca2f28d4a00e4154f8cd599", [:mix], [], "hexpm", "a81a04c7e34b6617c2792e291b5a2e57ab316365c2644ddc553bb9ed863ebefa"}, - "ecto": {:git, "https://github.com/greg-rychlewski/ecto.git", "c2df1d4488e722816d4d53e047cc25326e05a3ac", [branch: "json_path_with_fields"]}, + "ecto": {:git, "https://github.com/elixir-ecto/ecto.git", "60120357088650119b6e1b0ee6277637bae943c1", []}, "ex_doc": {:hex, :ex_doc, "0.34.2", "13eedf3844ccdce25cfd837b99bea9ad92c4e511233199440488d217c92571e8", [:mix], [{:earmark_parser, "~> 1.4.39", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_c, ">= 0.1.0", [hex: :makeup_c, repo: "hexpm", optional: true]}, {:makeup_elixir, "~> 0.14 or ~> 1.0", [hex: :makeup_elixir, repo: "hexpm", optional: false]}, {:makeup_erlang, "~> 0.1 or ~> 1.0", [hex: :makeup_erlang, repo: "hexpm", optional: false]}, {:makeup_html, ">= 0.1.0", [hex: :makeup_html, repo: "hexpm", optional: true]}], "hexpm", "5ce5f16b41208a50106afed3de6a2ed34f4acfd65715b82a0b84b49d995f95c1"}, "jason": {:hex, :jason, "1.4.4", "b9226785a9aa77b6857ca22832cffa5d5011a667207eb2a0ad56adb5db443b8a", [:mix], [{:decimal, "~> 1.0 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "c5eb0cab91f094599f94d55bc63409236a8ec69a21a67814529e8d5f6cc90b3b"}, "makeup": {:hex, :makeup, "1.1.2", "9ba8837913bdf757787e71c1581c21f9d2455f4dd04cfca785c70bbfff1a76a3", [:mix], [{:nimble_parsec, "~> 1.2.2 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "cce1566b81fbcbd21eca8ffe808f33b221f9eee2cbc7a1706fc3da9ff18e6cac"},