upserts: offer guidance when :replace_all or :replace_all_except produce an empty-list of fields to replace (#4623)

Author: milmazz

lib/ecto/repo/schema.ex

@@ -914,11 +914,19 @@ defmodule Ecto.Repo.Schema do
         # since the values don't change and this allows postgres to
         # possibly perform a HOT optimization: https://www.postgresql.org/docs/current/storage-hot.html
         to_remove = List.wrap(conflict_target)
-        {{replace_all_fields!(:replace_all, schema, to_remove), [], conflict_target}, []}
+        replace = replace_all_fields!(:replace_all, schema, to_remove)
+
+        if replace == [], do: raise(ArgumentError, "empty list of fields to update, use the `:replace` option instead")
+
+        {{replace, [], conflict_target}, []}
 
       {:replace_all_except, fields} ->
         to_remove = List.wrap(conflict_target) ++ fields
-        {{replace_all_fields!(:replace_all_except, schema, to_remove), [], conflict_target}, []}
+        replace = replace_all_fields!(:replace_all_except, schema, to_remove)
+
+        if replace == [], do: raise(ArgumentError, "empty list of fields to update, use the `:replace` option instead")
+
+        {{replace, [], conflict_target}, []}
 
       [_ | _] = on_conflict ->
         from = if schema, do: {source, schema}, else: source

test/ecto/repo_test.exs

@@ -181,6 +181,15 @@ defmodule Ecto.RepoTest do
     end
   end
 
+  defmodule MySchemaOneField do
+     use Ecto.Schema
+
+    @primary_key false
+    schema "my_schema" do
+      field :n, :integer
+    end
+  end
+
   test "defines child_spec/1" do
     assert TestRepo.child_spec([]) == %{
              id: TestRepo,
@@ -1929,12 +1938,34 @@ defmodule Ecto.RepoTest do
       assert_received {:insert, %{source: "my_schema", on_conflict: {^fields, [], [:id]}}}
     end
 
+    test "raises on empty-list of fields to update when :replace_all_except is given" do
+      msg = "empty list of fields to update, use the `:replace` option instead"
+
+      assert_raise ArgumentError, msg, fn ->
+        TestRepo.insert(%MySchema{id: 1},
+          on_conflict: {:replace_all_except, [:array, :map, :z, :y, :x]},
+          conflict_target: [:id]
+        )
+      end
+    end
+
     test "excludes conflict target from :replace_all" do
       fields = [:map, :array, :z, :yyy, :x]
       TestRepo.insert(%MySchema{id: 1}, on_conflict: :replace_all, conflict_target: [:id])
       assert_received {:insert, %{source: "my_schema", on_conflict: {^fields, [], [:id]}}}
     end
 
+    test "raises on empty-list of fields to update when :replace_all is given" do
+      msg = "empty list of fields to update, use the `:replace` option instead"
+
+      assert_raise ArgumentError, msg, fn ->
+        TestRepo.insert(%MySchemaOneField{n: 1},
+          on_conflict: :replace_all,
+          conflict_target: [:n]
+        )
+      end
+    end
+
     test "converts keyword list into query" do
       TestRepo.insert(%MySchema{id: 1}, on_conflict: [set: [x: "123", y: "456"]])
       assert_received {:insert, %{source: "my_schema", on_conflict: {query, ["123", "456"], []}}}