Skip to content

Commit

Permalink
Permit outer joins when using update_all with PostreSQL (#457)
Browse files Browse the repository at this point in the history
The PostgreSQL UPDATE command permits specifying multiple 'from_item'
expressions to allow columns from other tables to appear in the update
expression. In particular, the documentation explains (cf.
https://www.postgresql.org/docs/current/sql-update.html):

> This uses the same syntax as the FROM clause of a SELECT statement

Thus, a statement such as

  UPDATE x
  SET a = z.a
  FROM y
  LEFT_JOIN z ON z.i = y.i

is legal.

However, executing query like this this via Ecto would fail, raising an
error:

  ** (Ecto.QueryError) PostgreSQL supports only inner joins on update_all, got: `left` in query:

  from x0 in "x",
    join: y1 in "y",
    on: true,
    left_join: z2 in "z",
    on: true,
    update: [set: [a: 1]]

This PR fixes the issue by adding a `Postgres.Connection.using_join/4`
clause for the :update_all case: in this scenario, at least one inner
join has to be specified, but additional other joins are permissible.
  • Loading branch information
frerich committed Nov 11, 2022
1 parent ddf4fb5 commit 5ab355a
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 0 deletions.
25 changes: 25 additions & 0 deletions lib/ecto/adapters/postgres/connection.ex
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,31 @@ if Code.ensure_loaded?(Postgrex) do
end

defp using_join(%{joins: []}, _kind, _prefix, _sources), do: {[], []}

defp using_join(%{joins: joins} = query, :update_all, prefix, sources) do
{inner_joins, other_joins} = Enum.split_while(joins, & &1.qual == :inner)

if inner_joins == [] and other_joins != [] do
error!(query, "Need at least one inner join at the beginning to use other joins with update_all")
end

froms =
intersperse_map(inner_joins, ", ", fn
%JoinExpr{qual: :inner, ix: ix, source: source} ->
{join, name} = get_source(query, sources, ix, source)
[join, " AS " | [name]]
end)

join_clauses = join(%{query | joins: other_joins}, sources)

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

{[?\s, prefix, ?\s, froms | join_clauses], wheres}
end

defp using_join(%{joins: joins} = query, kind, prefix, sources) do
froms =
intersperse_map(joins, ", ", fn
Expand Down
13 changes: 13 additions & 0 deletions test/ecto/adapters/postgres_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -920,6 +920,19 @@ defmodule Ecto.Adapters.PostgresTest do
~s{UPDATE "first"."schema" AS s0 SET "x" = 0}
end

test "update all with left join" do
query = from(m in Schema, join: x in assoc(m, :comments), left_join: p in assoc(m, :permalink), update: [set: [w: m.list2]]) |> plan(:update_all)
assert update_all(query) ==
~s{UPDATE "schema" AS s0 SET "w" = s0."list2" FROM "schema2" AS s1 LEFT OUTER JOIN "schema3" AS s2 ON s2."id" = s0."y" WHERE (s1."z" = s0."x")}
end

test "update all with left join but no inner join" do
query = from(m in Schema, left_join: p in assoc(m, :permalink), left_join: x in assoc(m, :permalink), update: [set: [w: m.list2]]) |> plan(:update_all)
assert_raise Ecto.QueryError, ~r/Need at least one inner join at the beginning to use other joins with update_all/, fn ->
update_all(query)
end
end

test "delete all" do
query = Schema |> Queryable.to_query |> plan()
assert delete_all(query) == ~s{DELETE FROM "schema" AS s0}
Expand Down

0 comments on commit 5ab355a

Please sign in to comment.