Add {:nilify, columns} to on_delete reference option. (#474)

Author: greg-rychlewski

.github/workflows/ci.yml

@@ -26,6 +26,7 @@ jobs:
         elixirbase:
           - "1.11.4-erlang-21.3.8.24-alpine-3.13.3"
         postgres:
+          - "15.0"
           - "11.11"
           - "9.6"
           - "9.5"

Earthfile

@@ -24,6 +24,7 @@ test:
 integration-test-all:
     ARG ELIXIR_BASE=1.13.4-erlang-24.3.4.2-alpine-3.16.0
     BUILD \
+        --build-arg POSTGRES=15.0 \
         --build-arg POSTGRES=11.11 \
         --build-arg POSTGRES=9.6 \
         --build-arg POSTGRES=9.5 \
@@ -65,6 +66,14 @@ integration-test-postgres:
         # and in the 3.4 version, it is not included in postgresql-client but rather in postgresql
         RUN echo 'http://dl-cdn.alpinelinux.org/alpine/v3.4/main' >> /etc/apk/repositories
         RUN apk add postgresql=9.5.13-r0
+    ELSE IF [ "$POSTGRES" = "15.0" ]
+        # for 15.0 we need an upgraded version of pg_dump;
+        # alpine 3.17 does not come with the postgres 15 client by default;
+        # we must first update the public keys for the packages because they 
+        # might have been rotated since our image was built
+        RUN apk add -X https://dl-cdn.alpinelinux.org/alpine/v3.17/main -u alpine-keys
+        RUN echo 'http://dl-cdn.alpinelinux.org/alpine/v3.17/main' >> /etc/apk/repositories
+        RUN apk add postgresql15-client
     ELSE
         RUN apk add postgresql-client
     END
@@ -141,7 +150,7 @@ integration-test-mssql:
 
 
 setup-base:
-    ARG ELIXIR_BASE=1.13.4-erlang-24.3.4.2-alpine-3.16.0
+    ARG ELIXIR_BASE=1.13.4-erlang-24.3.4.2-alpine-3.17.0
     FROM hexpm/elixir:$ELIXIR_BASE
     RUN apk add --no-progress --update git build-base
     ENV ELIXIR_ASSERT_TIMEOUT=10000

integration_test/myxql/test_helper.exs

@@ -100,7 +100,9 @@ excludes = [
   # MySQL doesn't have a boolean type, so this ends up returning 0/1
   :map_boolean_in_expression,
   # MySQL doesn't support indexed parameters
-  :placeholders
+  :placeholders,
+  # MySQL doesn't support specifying columns for ON DELETE SET NULL
+  :on_delete_nilify_column_list
 ]
 
 if Version.match?(version, ">= 8.0.0") do

integration_test/pg/test_helper.exs

@@ -100,11 +100,19 @@ version =
 excludes = [:selected_as_with_having, :selected_as_with_order_by_expression]
 excludes_above_9_5 = [:without_conflict_target]
 excludes_below_9_6 = [:add_column_if_not_exists, :no_error_on_conditional_column_migration]
+excludes_below_15_0 = [:on_delete_nilify_column_list]
 
-if Version.match?(version, "< 9.6.0") do
-  ExUnit.configure(exclude: excludes ++ excludes_above_9_5 ++ excludes_below_9_6)
-else
-  ExUnit.configure(exclude: excludes ++ excludes_above_9_5)
+exclude_list = excludes ++ excludes_above_9_5
+
+cond do
+  Version.match?(version, "< 9.6.0") ->
+    ExUnit.configure(exclude: exclude_list ++ excludes_below_9_6 ++ excludes_below_15_0)
+
+  Version.match?(version, "< 15.0.0") ->
+    ExUnit.configure(exclude: exclude_list ++ excludes_below_15_0)
+
+  true ->
+    ExUnit.configure(exclude: exclude_list)
 end
 
 :ok = Ecto.Migrator.up(TestRepo, 0, Ecto.Integration.Migration, log: false)

integration_test/sql/migration.exs

@@ -226,6 +226,34 @@ defmodule Ecto.Integration.MigrationTest do
     end
   end
 
+  defmodule OnDeleteNilifyColumnsMigration do
+    use Ecto.Migration
+
+    def up do
+      create table(:parent) do
+        add :col1, :integer
+        add :col2, :integer
+      end
+
+      create unique_index(:parent, [:id, :col1, :col2])
+
+      create table(:ref) do
+        add :col1, :integer
+        add :col2, :integer
+        add :parent_id,
+            references(:parent,
+              with: [col1: :col1, col2: :col2],
+              on_delete: {:nilify, [:parent_id, :col2]}
+            )
+      end
+    end
+
+    def down do
+      drop table(:ref)
+      drop table(:parent)
+    end
+  end
+
   defmodule CompositeForeignKeyMigration do
     use Ecto.Migration
 
@@ -622,4 +650,18 @@ defmodule Ecto.Integration.MigrationTest do
     assert catch_error(PoolRepo.all from p in "drop_col_if_exists_migration", select: p.to_be_removed)
     :ok = down(PoolRepo, num, DropColumnIfExistsMigration, log: false)
   end
+
+  @tag :on_delete_nilify_column_list
+  test "nilify list of columns on_delete constraint", %{migration_number: num} do
+    assert :ok == up(PoolRepo, num, OnDeleteNilifyColumnsMigration, log: false)
+
+    PoolRepo.insert_all("parent", [%{col1: 1, col2: 2}])
+    assert [{id, col1, col2}] = PoolRepo.all from p in "parent", select: {p.id, p.col1, p.col2}
+
+    PoolRepo.insert_all("ref", [[parent_id: id, col1: col1, col2: col2]])
+    PoolRepo.delete_all("parent")
+    assert [{nil, col1, nil}] == PoolRepo.all from r in "ref", select: {r.parent_id, r.col1, r.col2}
+
+    :ok = down(PoolRepo, num, OnDeleteNilifyColumnsMigration, log: false)
+  end
 end

integration_test/tds/test_helper.exs

@@ -54,7 +54,9 @@ ExUnit.start(
     # MSSQL can't reference aliased columns in HAVING
     :selected_as_with_having,
     # MSSQL can't reference aliased columns in ORDER BY expressions
-    :selected_as_with_order_by_expression
+    :selected_as_with_order_by_expression,
+    # MSSQL doesn't support specifying columns for ON DELETE SET NULL
+    :on_delete_nilify_column_list
   ]
 )
 

lib/ecto/adapters/postgres/connection.ex

@@ -1305,6 +1305,8 @@ if Code.ensure_loaded?(Postgrex) do
     defp reference_column_type(type, opts), do: column_type(type, opts)
 
     defp reference_on_delete(:nilify_all), do: " ON DELETE SET NULL"
+    defp reference_on_delete({:nilify, columns}),
+      do: [" ON DELETE SET NULL (", quote_names(columns), ")"]
     defp reference_on_delete(:delete_all), do: " ON DELETE CASCADE"
     defp reference_on_delete(:restrict), do: " ON DELETE RESTRICT"
     defp reference_on_delete(_), do: []

lib/ecto/migration.ex

@@ -1280,7 +1280,9 @@ defmodule Ecto.Migration do
       the example above), or `nil`.
     * `:type` - The foreign key type, which defaults to `:bigserial`.
     * `:on_delete` - What to do if the referenced entry is deleted. May be
-      `:nothing` (default), `:delete_all`, `:nilify_all`, or `:restrict`.
+      `:nothing` (default), `:delete_all`, `:nilify_all`, `{:nilify, columns}`,
+      or `:restrict`. `{:nilify, columns}` expects a list of atoms for `columns`
+      and is not supported by all databases.
     * `:on_update` - What to do if the referenced entry is updated. May be
       `:nothing` (default), `:update_all`, `:nilify_all`, or `:restrict`.
     * `:validate` - Whether or not to validate the foreign key constraint on
@@ -1302,14 +1304,8 @@ defmodule Ecto.Migration do
   def references(table, opts) when is_binary(table) and is_list(opts) do
     opts = Keyword.merge(foreign_key_repo_opts(), opts)
     reference = struct(%Reference{table: table}, opts)
-
-    unless reference.on_delete in [:nothing, :delete_all, :nilify_all, :restrict] do
-      raise ArgumentError, "unknown :on_delete value: #{inspect reference.on_delete}"
-    end
-
-    unless reference.on_update in [:nothing, :update_all, :nilify_all, :restrict] do
-      raise ArgumentError, "unknown :on_update value: #{inspect reference.on_update}"
-    end
+    check_on_delete!(reference.on_delete)
+    check_on_update!(reference.on_update)
 
     reference
   end
@@ -1323,6 +1319,31 @@ defmodule Ecto.Migration do
     |> Keyword.merge(Runner.repo_config(:migration_foreign_key, []))
   end
 
+  defp check_on_delete!(on_delete)
+      when on_delete in [:nothing, :delete_all, :nilify_all, :restrict],
+      do: :ok
+
+  defp check_on_delete!({:nilify, columns}) when is_list(columns) do
+    unless Enum.all?(columns, &is_atom/1) do
+      raise ArgumentError,
+            "expected `columns` in `{:nilify, columns}` to be a list of atoms, got: #{inspect columns}"
+    end
+
+    :ok
+  end
+
+  defp check_on_delete!(on_delete) do
+    raise ArgumentError, "unknown :on_delete value: #{inspect(on_delete)}"
+  end
+
+  defp check_on_update!(on_update)
+      when on_update in [:nothing, :update_all, :nilify_all, :restrict],
+      do: :ok
+
+  defp check_on_update!(on_update) do
+    raise ArgumentError, "unknown :on_update value: #{inspect(on_update)}"
+  end
+
   @doc ~S"""
   Defines a constraint (either a check constraint or an exclusion constraint)
   to be evaluated by the database when a row is inserted or updated.

test/ecto/adapters/postgres_test.exs

@@ -1501,7 +1501,8 @@ defmodule Ecto.Adapters.PostgresTest do
                {:add, :category_10, %Reference{table: :categories, on_update: :restrict}, []},
                {:add, :category_11, %Reference{table: :categories, prefix: "foo", on_update: :restrict}, []},
                {:add, :category_12, %Reference{table: :categories, with: [here: :there]}, []},
-               {:add, :category_13, %Reference{table: :categories, on_update: :restrict, with: [here: :there], match: :full}, []},]}
+               {:add, :category_13, %Reference{table: :categories, on_update: :restrict, with: [here: :there], match: :full}, []},
+               {:add, :category_14, %Reference{table: :categories, with: [here: :there, here2: :there2], on_delete: {:nilify, [:here, :here2]}}, []},]}
 
     assert execute_ddl(create) == ["""
     CREATE TABLE "posts" ("id" serial,
@@ -1519,6 +1520,7 @@ defmodule Ecto.Adapters.PostgresTest do
     "category_11" bigint, CONSTRAINT "posts_category_11_fkey" FOREIGN KEY ("category_11") REFERENCES "foo"."categories"("id") ON UPDATE RESTRICT,
     "category_12" bigint, CONSTRAINT "posts_category_12_fkey" FOREIGN KEY ("category_12","here") REFERENCES "categories"("id","there"),
     "category_13" bigint, CONSTRAINT "posts_category_13_fkey" FOREIGN KEY ("category_13","here") REFERENCES "categories"("id","there") MATCH FULL ON UPDATE RESTRICT,
+    "category_14" bigint, CONSTRAINT "posts_category_14_fkey" FOREIGN KEY ("category_14","here","here2") REFERENCES "categories"("id","there","there2") ON DELETE SET NULL ("here","here2"),
     PRIMARY KEY ("id"))
     """ |> remove_newlines]
   end