Skip to content

Commit 9c6a0c4

Browse files
lukaszsamsonclaude
andcommitted
Support BooleanExpr join ons
Since elixir-ecto/ecto#4765, the join builder emits every JoinExpr.on as a %BooleanExpr{op: :and} rather than a QueryExpr, so that wheres folded into the on of an interpolated join query can carry subqueries. Match on BooleanExpr when rendering joins, and drop the manual struct rewrite in the using_join comprehensions, which now receive a real BooleanExpr. Without this, join/2 raises for interpolated join queries, and the using_join comprehensions silently drop the join conditions from the WHERE clause of update_all/delete_all, as they filtered joins by on: %QueryExpr{}. Add coverage for interpolated join queries in update_all/delete_all, which had none. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 7908e75 commit 9c6a0c4

7 files changed

Lines changed: 73 additions & 10 deletions

File tree

lib/ecto/adapters/myxql/connection.ex

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -508,9 +508,9 @@ if Code.ensure_loaded?(MyXQL) do
508508
end)
509509

510510
wheres =
511-
for %JoinExpr{on: %QueryExpr{expr: value} = expr} <- joins,
511+
for %JoinExpr{on: %BooleanExpr{expr: value} = expr} <- joins,
512512
value != true,
513-
do: expr |> Map.put(:__struct__, BooleanExpr) |> Map.put(:op, :and)
513+
do: expr
514514

515515
{[?,, ?\s | froms], wheres}
516516
end
@@ -519,7 +519,7 @@ if Code.ensure_loaded?(MyXQL) do
519519

520520
defp join(%{joins: joins} = query, sources) do
521521
Enum.map(joins, fn
522-
%JoinExpr{on: %QueryExpr{expr: expr}, qual: qual, ix: ix, source: source, hints: hints} ->
522+
%JoinExpr{on: %BooleanExpr{expr: expr}, qual: qual, ix: ix, source: source, hints: hints} ->
523523
{join, name} = get_source(query, sources, ix, source)
524524

525525
[

lib/ecto/adapters/postgres/connection.ex

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -717,9 +717,9 @@ if Code.ensure_loaded?(Postgrex) do
717717
join_clauses = join(%{query | joins: other_joins}, sources)
718718

719719
wheres =
720-
for %JoinExpr{on: %QueryExpr{expr: value} = expr} <- inner_joins,
720+
for %JoinExpr{on: %BooleanExpr{expr: value} = expr} <- inner_joins,
721721
value != true,
722-
do: expr |> Map.put(:__struct__, BooleanExpr) |> Map.put(:op, :and)
722+
do: expr
723723

724724
{[?\s, prefix, ?\s, froms | join_clauses], wheres}
725725
end
@@ -736,9 +736,9 @@ if Code.ensure_loaded?(Postgrex) do
736736
end)
737737

738738
wheres =
739-
for %JoinExpr{on: %QueryExpr{expr: value} = expr} <- joins,
739+
for %JoinExpr{on: %BooleanExpr{expr: value} = expr} <- joins,
740740
value != true,
741-
do: expr |> Map.put(:__struct__, BooleanExpr) |> Map.put(:op, :and)
741+
do: expr
742742

743743
{[?\s, prefix, ?\s | froms], wheres}
744744
end
@@ -750,7 +750,7 @@ if Code.ensure_loaded?(Postgrex) do
750750
?\s
751751
| Enum.map_intersperse(joins, ?\s, fn
752752
%JoinExpr{
753-
on: %QueryExpr{expr: expr},
753+
on: %BooleanExpr{expr: expr},
754754
qual: qual,
755755
ix: ix,
756756
source: source,

lib/ecto/adapters/tds/connection.ex

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -542,7 +542,13 @@ if Code.ensure_loaded?(Tds) do
542542
[
543543
?\s,
544544
Enum.map_intersperse(joins, ?\s, fn
545-
%JoinExpr{on: %QueryExpr{expr: expr}, qual: qual, ix: ix, source: source, hints: hints} ->
545+
%JoinExpr{
546+
on: %BooleanExpr{expr: expr},
547+
qual: qual,
548+
ix: ix,
549+
source: source,
550+
hints: hints
551+
} ->
546552
{join, name} = get_source(query, sources, ix, source)
547553
qual_text = join_qual(qual, query)
548554
join = join || ["(", expr(source, sources, query) | ")"]

mix.lock

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
"decimal": {:hex, :decimal, "3.1.1", "430d87b04011ce6cbd4fd205be758311a81f87d552d40904abd00f015935b1d0", [:mix], [], "hexpm", "c5f25f2ced74a0587d03e6023f595db8e924c9d3922c8c8ffd9edfc4498cf1f6"},
77
"deep_merge": {:hex, :deep_merge, "1.0.0", "b4aa1a0d1acac393bdf38b2291af38cb1d4a52806cf7a4906f718e1feb5ee961", [:mix], [], "hexpm", "ce708e5f094b9cd4e8f2be4f00d2f4250c4095be93f8cd6d018c753894885430"},
88
"earmark_parser": {:hex, :earmark_parser, "1.4.44", "f20830dd6b5c77afe2b063777ddbbff09f9759396500cdbe7523efd58d7a339c", [:mix], [], "hexpm", "4778ac752b4701a5599215f7030989c989ffdc4f6df457c5f36938cc2d2a2750"},
9-
"ecto": {:git, "https://github.com/elixir-ecto/ecto.git", "d4add51346eb282f69eefd387503be49064a6304", []},
9+
"ecto": {:git, "https://github.com/elixir-ecto/ecto.git", "8959c439c7618ed88be98fe7741ba9986a57a7e5", []},
1010
"ex_doc": {:hex, :ex_doc, "0.40.2", "f50edec428c4b0a457a167de42414c461122a3585a99515a69d09fff19e5597e", [:mix], [{:earmark_parser, "~> 1.4.44", [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", "4fa426e2beb47854a162e2c488727fdec51cd4692e319b23810c2804cb1a40fe"},
1111
"jason": {:hex, :jason, "1.4.5", "2e3a008590b0b8d7388c20293e9dcc9cf3e5d642fd2a114e4cbbb52e595d940a", [:mix], [{:decimal, "~> 1.0 or ~> 2.0 or ~> 3.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "b0c823996102bcd0239b3c2444eb00409b72f6a140c1950bc8b457d836b30684"},
1212
"makeup": {:hex, :makeup, "1.2.1", "e90ac1c65589ef354378def3ba19d401e739ee7ee06fb47f94c687016e3713d1", [:mix], [{:nimble_parsec, "~> 1.4", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "d36484867b0bae0fea568d10131197a4c2e47056a6fbe84922bf6ba71c8d17ce"},

test/ecto/adapters/myxql_test.exs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1377,6 +1377,25 @@ defmodule Ecto.Adapters.MyXQLTest do
13771377
"SELECT s0.`id`, s1.`id` FROM `schema` AS s0 LEFT OUTER JOIN `schema2` AS s1 ON TRUE"
13781378
end
13791379

1380+
test "update all with interpolated join query" do
1381+
inner = from(s in Schema2, where: s.z > 10)
1382+
1383+
query =
1384+
from(m in Schema, join: x in ^inner, on: m.x == x.z, update: [set: [x: 0]])
1385+
|> plan(:update_all)
1386+
1387+
assert update_all(query) ==
1388+
~s{UPDATE `schema` AS s0, `schema2` AS s1 SET s0.`x` = 0 WHERE ((s1.`z` > 10) AND (s0.`x` = s1.`z`))}
1389+
end
1390+
1391+
test "delete all with interpolated join query" do
1392+
inner = from(s in Schema2, where: s.z > 10)
1393+
query = from(m in Schema, join: x in ^inner, on: m.x == x.z) |> plan(:delete_all)
1394+
1395+
assert delete_all(query) ==
1396+
~s{DELETE s0.* FROM `schema` AS s0 INNER JOIN `schema2` AS s1 ON (s1.`z` > 10) AND (s0.`x` = s1.`z`)}
1397+
end
1398+
13801399
test "lateral join with fragment" do
13811400
query =
13821401
Schema

test/ecto/adapters/postgres_test.exs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1757,6 +1757,25 @@ defmodule Ecto.Adapters.PostgresTest do
17571757
"SELECT s0.\"id\", s1.\"id\" FROM \"schema\" AS s0 LEFT OUTER JOIN \"schema2\" AS s1 ON TRUE"
17581758
end
17591759

1760+
test "update all with interpolated join query" do
1761+
inner = from(s in Schema2, where: s.z > 10)
1762+
1763+
query =
1764+
from(m in Schema, join: x in ^inner, on: m.x == x.z, update: [set: [x: 0]])
1765+
|> plan(:update_all)
1766+
1767+
assert update_all(query) ==
1768+
~s{UPDATE "schema" AS s0 SET "x" = 0 FROM "schema2" AS s1 WHERE ((s1."z" > 10) AND (s0."x" = s1."z"))}
1769+
end
1770+
1771+
test "delete all with interpolated join query" do
1772+
inner = from(s in Schema2, where: s.z > 10)
1773+
query = from(m in Schema, join: x in ^inner, on: m.x == x.z) |> plan(:delete_all)
1774+
1775+
assert delete_all(query) ==
1776+
~s{DELETE FROM "schema" AS s0 USING "schema2" AS s1 WHERE ((s1."z" > 10) AND (s0."x" = s1."z"))}
1777+
end
1778+
17601779
test "lateral join with fragment" do
17611780
query =
17621781
Schema

test/ecto/adapters/tds_test.exs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1223,6 +1223,25 @@ defmodule Ecto.Adapters.TdsTest do
12231223
"SELECT s0.[id], s1.[id] FROM [schema] AS s0 LEFT OUTER JOIN [schema2] AS s1 ON 1 = 1"
12241224
end
12251225

1226+
test "update all with interpolated join query" do
1227+
inner = from(s in Schema2, where: s.z > 10)
1228+
1229+
query =
1230+
from(m in Schema, join: x in ^inner, on: m.x == x.z, update: [set: [x: 0]])
1231+
|> plan(:update_all)
1232+
1233+
assert update_all(query) ==
1234+
~s{UPDATE s0 SET s0.[x] = 0 FROM [schema] AS s0 INNER JOIN [schema2] AS s1 ON (s1.[z] > 10) AND (s0.[x] = s1.[z])}
1235+
end
1236+
1237+
test "delete all with interpolated join query" do
1238+
inner = from(s in Schema2, where: s.z > 10)
1239+
query = from(m in Schema, join: x in ^inner, on: m.x == x.z) |> plan(:delete_all)
1240+
1241+
assert delete_all(query) ==
1242+
~s{DELETE s0 FROM [schema] AS s0 INNER JOIN [schema2] AS s1 ON (s1.[z] > 10) AND (s0.[x] = s1.[z])}
1243+
end
1244+
12261245
test "join produces correct bindings" do
12271246
query = from(p in Schema, join: c in Schema2, on: true)
12281247
query = from(p in query, join: c in Schema2, on: true, select: {p.id, c.id})

0 commit comments

Comments
 (0)