From 5d684839acf47fe55011c19acb90ca1f9c51eef4 Mon Sep 17 00:00:00 2001 From: Lukasz Samson Date: Fri, 24 Jul 2026 17:28:49 +0200 Subject: [PATCH 1/2] Do not assume JoinExpr.on is a QueryExpr Ecto is changing interpolated join queries (join: x in ^query) to carry their "on" expression as a BooleanExpr instead of a QueryExpr, since folded wheres may hold subqueries (elixir-ecto/ecto#4765). Relax the join.on pattern matches to the map shape so both structs are accepted. This matters beyond the crash in join/2: the using_join comprehensions filtered joins by on: %QueryExpr{}, so a BooleanExpr "on" would have been silently dropped from the WHERE clause of update_all/delete_all. Add coverage for interpolated join queries in update_all/delete_all, which previously had none. The relaxed patterns remain compatible with current Ecto releases. Co-Authored-By: Claude Fable 5 --- lib/ecto/adapters/myxql/connection.ex | 4 ++-- lib/ecto/adapters/postgres/connection.ex | 6 +++--- lib/ecto/adapters/tds/connection.ex | 2 +- test/ecto/adapters/myxql_test.exs | 17 +++++++++++++++++ test/ecto/adapters/postgres_test.exs | 17 +++++++++++++++++ test/ecto/adapters/tds_test.exs | 17 +++++++++++++++++ 6 files changed, 57 insertions(+), 6 deletions(-) diff --git a/lib/ecto/adapters/myxql/connection.ex b/lib/ecto/adapters/myxql/connection.ex index 833c6d35..0c815268 100644 --- a/lib/ecto/adapters/myxql/connection.ex +++ b/lib/ecto/adapters/myxql/connection.ex @@ -502,7 +502,7 @@ if Code.ensure_loaded?(MyXQL) do end) wheres = - for %JoinExpr{on: %QueryExpr{expr: value} = expr} <- joins, + for %JoinExpr{on: %{expr: value} = expr} <- joins, value != true, do: expr |> Map.put(:__struct__, BooleanExpr) |> Map.put(:op, :and) @@ -513,7 +513,7 @@ if Code.ensure_loaded?(MyXQL) do defp join(%{joins: joins} = query, sources) do Enum.map(joins, fn - %JoinExpr{on: %QueryExpr{expr: expr}, qual: qual, ix: ix, source: source, hints: hints} -> + %JoinExpr{on: %{expr: expr}, qual: qual, ix: ix, source: source, hints: hints} -> {join, name} = get_source(query, sources, ix, source) [ diff --git a/lib/ecto/adapters/postgres/connection.ex b/lib/ecto/adapters/postgres/connection.ex index b6398531..5f1e30ee 100644 --- a/lib/ecto/adapters/postgres/connection.ex +++ b/lib/ecto/adapters/postgres/connection.ex @@ -705,7 +705,7 @@ if Code.ensure_loaded?(Postgrex) do join_clauses = join(%{query | joins: other_joins}, sources) wheres = - for %JoinExpr{on: %QueryExpr{expr: value} = expr} <- inner_joins, + for %JoinExpr{on: %{expr: value} = expr} <- inner_joins, value != true, do: expr |> Map.put(:__struct__, BooleanExpr) |> Map.put(:op, :and) @@ -724,7 +724,7 @@ if Code.ensure_loaded?(Postgrex) do end) wheres = - for %JoinExpr{on: %QueryExpr{expr: value} = expr} <- joins, + for %JoinExpr{on: %{expr: value} = expr} <- joins, value != true, do: expr |> Map.put(:__struct__, BooleanExpr) |> Map.put(:op, :and) @@ -738,7 +738,7 @@ if Code.ensure_loaded?(Postgrex) do ?\s | Enum.map_intersperse(joins, ?\s, fn %JoinExpr{ - on: %QueryExpr{expr: expr}, + on: %{expr: expr}, qual: qual, ix: ix, source: source, diff --git a/lib/ecto/adapters/tds/connection.ex b/lib/ecto/adapters/tds/connection.ex index be1cf229..6079e25c 100644 --- a/lib/ecto/adapters/tds/connection.ex +++ b/lib/ecto/adapters/tds/connection.ex @@ -542,7 +542,7 @@ if Code.ensure_loaded?(Tds) do [ ?\s, Enum.map_intersperse(joins, ?\s, fn - %JoinExpr{on: %QueryExpr{expr: expr}, qual: qual, ix: ix, source: source, hints: hints} -> + %JoinExpr{on: %{expr: expr}, qual: qual, ix: ix, source: source, hints: hints} -> {join, name} = get_source(query, sources, ix, source) qual_text = join_qual(qual, query) join = join || ["(", expr(source, sources, query) | ")"] diff --git a/test/ecto/adapters/myxql_test.exs b/test/ecto/adapters/myxql_test.exs index b062d7bb..fdcb7d97 100644 --- a/test/ecto/adapters/myxql_test.exs +++ b/test/ecto/adapters/myxql_test.exs @@ -1369,6 +1369,23 @@ defmodule Ecto.Adapters.MyXQLTest do "SELECT s0.`id`, s1.`id` FROM `schema` AS s0 LEFT OUTER JOIN `schema2` AS s1 ON TRUE" end + test "update all with interpolated join query" do + inner = from(s in Schema2, where: s.z > 10) + + query = + from(m in Schema, join: x in ^inner, on: m.x == x.z, update: [set: [x: 0]]) + |> plan(:update_all) + + assert update_all(query) == ~s{UPDATE `schema` AS s0, `schema2` AS s1 SET s0.`x` = 0 WHERE ((s1.`z` > 10) AND (s0.`x` = s1.`z`))} + end + + test "delete all with interpolated join query" do + inner = from(s in Schema2, where: s.z > 10) + query = from(m in Schema, join: x in ^inner, on: m.x == x.z) |> plan(:delete_all) + + assert delete_all(query) == ~s{DELETE s0.* FROM `schema` AS s0 INNER JOIN `schema2` AS s1 ON (s1.`z` > 10) AND (s0.`x` = s1.`z`)} + end + test "lateral join with fragment" do query = Schema diff --git a/test/ecto/adapters/postgres_test.exs b/test/ecto/adapters/postgres_test.exs index e877e7be..75c5f901 100644 --- a/test/ecto/adapters/postgres_test.exs +++ b/test/ecto/adapters/postgres_test.exs @@ -1733,6 +1733,23 @@ defmodule Ecto.Adapters.PostgresTest do "SELECT s0.\"id\", s1.\"id\" FROM \"schema\" AS s0 LEFT OUTER JOIN \"schema2\" AS s1 ON TRUE" end + test "update all with interpolated join query" do + inner = from(s in Schema2, where: s.z > 10) + + query = + from(m in Schema, join: x in ^inner, on: m.x == x.z, update: [set: [x: 0]]) + |> plan(:update_all) + + assert update_all(query) == ~s{UPDATE "schema" AS s0 SET "x" = 0 FROM "schema2" AS s1 WHERE ((s1."z" > 10) AND (s0."x" = s1."z"))} + end + + test "delete all with interpolated join query" do + inner = from(s in Schema2, where: s.z > 10) + query = from(m in Schema, join: x in ^inner, on: m.x == x.z) |> plan(:delete_all) + + assert delete_all(query) == ~s{DELETE FROM "schema" AS s0 USING "schema2" AS s1 WHERE ((s1."z" > 10) AND (s0."x" = s1."z"))} + end + test "lateral join with fragment" do query = Schema diff --git a/test/ecto/adapters/tds_test.exs b/test/ecto/adapters/tds_test.exs index 93413141..70c3e0bc 100644 --- a/test/ecto/adapters/tds_test.exs +++ b/test/ecto/adapters/tds_test.exs @@ -1215,6 +1215,23 @@ defmodule Ecto.Adapters.TdsTest do "SELECT s0.[id], s1.[id] FROM [schema] AS s0 LEFT OUTER JOIN [schema2] AS s1 ON 1 = 1" end + test "update all with interpolated join query" do + inner = from(s in Schema2, where: s.z > 10) + + query = + from(m in Schema, join: x in ^inner, on: m.x == x.z, update: [set: [x: 0]]) + |> plan(:update_all) + + assert update_all(query) == ~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])} + end + + test "delete all with interpolated join query" do + inner = from(s in Schema2, where: s.z > 10) + query = from(m in Schema, join: x in ^inner, on: m.x == x.z) |> plan(:delete_all) + + assert delete_all(query) == ~s{DELETE s0 FROM [schema] AS s0 INNER JOIN [schema2] AS s1 ON (s1.[z] > 10) AND (s0.[x] = s1.[z])} + end + test "join produces correct bindings" do query = from(p in Schema, join: c in Schema2, on: true) query = from(p in query, join: c in Schema2, on: true, select: {p.id, c.id}) From acbdce82a8ad15c9f7d68657a876b854fc6982e5 Mon Sep 17 00:00:00 2001 From: Lukasz Samson Date: Sat, 25 Jul 2026 18:10:33 +0200 Subject: [PATCH 2/2] Drop manual QueryExpr to BooleanExpr conversion in using_join Since elixir-ecto/ecto#4765, every JoinExpr.on is a BooleanExpr with op :and, so the using_join comprehensions no longer need to rewrite the struct by hand before appending the join conditions to WHERE. This requires ecto with the above change and must only ship once the minimum ecto version is bumped accordingly. Co-Authored-By: Claude Fable 5 --- lib/ecto/adapters/myxql/connection.ex | 4 ++-- lib/ecto/adapters/postgres/connection.ex | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/ecto/adapters/myxql/connection.ex b/lib/ecto/adapters/myxql/connection.ex index 0c815268..634ec426 100644 --- a/lib/ecto/adapters/myxql/connection.ex +++ b/lib/ecto/adapters/myxql/connection.ex @@ -502,9 +502,9 @@ if Code.ensure_loaded?(MyXQL) do end) wheres = - for %JoinExpr{on: %{expr: value} = expr} <- joins, + for %JoinExpr{on: %BooleanExpr{expr: value} = expr} <- joins, value != true, - do: expr |> Map.put(:__struct__, BooleanExpr) |> Map.put(:op, :and) + do: expr {[?,, ?\s | froms], wheres} end diff --git a/lib/ecto/adapters/postgres/connection.ex b/lib/ecto/adapters/postgres/connection.ex index 5f1e30ee..65a1dafd 100644 --- a/lib/ecto/adapters/postgres/connection.ex +++ b/lib/ecto/adapters/postgres/connection.ex @@ -705,9 +705,9 @@ if Code.ensure_loaded?(Postgrex) do join_clauses = join(%{query | joins: other_joins}, sources) wheres = - for %JoinExpr{on: %{expr: value} = expr} <- inner_joins, + for %JoinExpr{on: %BooleanExpr{expr: value} = expr} <- inner_joins, value != true, - do: expr |> Map.put(:__struct__, BooleanExpr) |> Map.put(:op, :and) + do: expr {[?\s, prefix, ?\s, froms | join_clauses], wheres} end @@ -724,9 +724,9 @@ if Code.ensure_loaded?(Postgrex) do end) wheres = - for %JoinExpr{on: %{expr: value} = expr} <- joins, + for %JoinExpr{on: %BooleanExpr{expr: value} = expr} <- joins, value != true, - do: expr |> Map.put(:__struct__, BooleanExpr) |> Map.put(:op, :and) + do: expr {[?\s, prefix, ?\s | froms], wheres} end