Repository: elixir-ecto/ecto
Pull request #2469 (2018-03-20 12:25:02)

Fix unsafe_validate_unique/3 for partially matching composite keys
See issue #2468 

ebert[bot]: Hello, @nikolailv! This is your first Pull Request that will be reviewed by [Ebert](https://ebertapp.io/), an automatic Code Review service. It will leave comments on this diff with potential issues and style violations found in the code as you push new commits. You can also see all the issues found on this Pull Request on its [review page](https://ebertapp.io/github/elixir-ecto/ecto/pulls/2469). Please check our [documentation](https://docs.ebertapp.io) for more information. 

Patch:
diff --git a/lib/ecto/changeset.ex b/lib/ecto/changeset.ex
index 16e5eb67..a4328628 100644
--- a/lib/ecto/changeset.ex
+++ b/lib/ecto/changeset.ex
@@ -1542,22 +1542,22 @@ defmodule Ecto.Changeset do
     else
       pk_pairs = pk_fields_and_values(changeset, struct)
 
-      query =
-        struct
-        |> Ecto.Query.where(^where_clause)
-        |> Ecto.Query.select(true)
-        |> Ecto.Query.limit(1)
-
-      query =
+      pk_query =
         # It should not conflict with itself for updates
         if Enum.any?(pk_pairs, &(&1 |> elem(1) |> is_nil())) do
-          query
+          struct
         else
-          Enum.reduce(pk_pairs, query, fn {field, value}, acc ->
-            Ecto.Query.where(acc, [q], field(q, ^field) != ^value)
+          Enum.reduce(pk_pairs, struct, fn {field, value}, acc ->
+            Ecto.Query.or_where(acc, [q], field(q, ^field) != ^value)
           end)
         end
 
+      query =
+        pk_query
+        |> Ecto.Query.where(^where_clause)
+        |> Ecto.Query.select(true)
+        |> Ecto.Query.limit(1)
+
       query =
         if prefix = opts[:prefix] do
           query


Test patch:
diff --git a/integration_test/cases/repo.exs b/integration_test/cases/repo.exs
index c37f27e1..6dd298e2 100644
--- a/integration_test/cases/repo.exs
+++ b/integration_test/cases/repo.exs
@@ -601,6 +601,25 @@ defmodule Ecto.Integration.RepoTest do
     assert changeset.errors[:title] == nil # cannot conflict with itself
   end
 
+  test "unsafe_validate_unique/3 with composite keys" do
+    {:ok, inserted_post} = TestRepo.insert(%CompositePk{a: 123, b: 456, name: "UniqueName"})
+
+    different_pk = CompositePk.changeset(%CompositePk{}, %{name: "UniqueName", a: 789, b: 321})
+    changeset = Ecto.Changeset.unsafe_validate_unique(different_pk, [:name], TestRepo)
+    assert changeset.errors[:name] ==
+      {"has already been taken", validation: :unsafe_unique, fields: [:name]}
+
+    partial_pk = CompositePk.changeset(%CompositePk{}, %{name: "UniqueName", a: 789, b: 456})
+    changeset = Ecto.Changeset.unsafe_validate_unique(partial_pk, [:name], TestRepo)
+    assert changeset.errors[:name] ==
+           {"has already been taken", validation: :unsafe_unique, fields: [:name]}
+
+    update_changeset = CompositePk.changeset(inserted_post, %{name: "NewName"})
+    changeset = Ecto.Changeset.unsafe_validate_unique(update_changeset, [:name], TestRepo)
+    assert changeset.valid?
+    assert changeset.errors[:name] == nil # cannot conflict with itself
+  end
+
   test "get(!)" do
     post1 = TestRepo.insert!(%Post{title: "1", text: "hai"})
     post2 = TestRepo.insert!(%Post{title: "2", text: "hai"})
diff --git a/integration_test/support/schemas.exs b/integration_test/support/schemas.exs
index fe10e342..68c17f66 100644
--- a/integration_test/support/schemas.exs
+++ b/integration_test/support/schemas.exs
@@ -238,6 +238,7 @@ defmodule Ecto.Integration.CompositePk do
 
   """
   use Ecto.Integration.Schema
+  import Ecto.Changeset
 
   @primary_key false
   schema "composite_pk" do
@@ -245,6 +246,9 @@ defmodule Ecto.Integration.CompositePk do
     field :b, :integer, primary_key: true
     field :name, :string
   end
+  def changeset(schema, params) do
+    cast(schema, params, ~w(a b name)a)
+  end
 end
 
 defmodule Ecto.Integration.CorruptedPk do