Introduce new multiple primary key error

Author: José Valim

integration_test/sql/sql.exs

@@ -43,21 +43,15 @@ defmodule Ecto.Integration.SQLTest do
     assert sql =~ "barebones"
   end
 
-  test "struct/7 raises when primary key is not unique" do
+  test "raises when primary key is not unique on struct operation" do
     schema = %CorruptedPk{a: "abc"}
-    for _ <- 1..10, do: TestRepo.insert!(schema)
+    TestRepo.insert!(schema)
+    TestRepo.insert!(schema)
+    TestRepo.insert!(schema)
 
-    assert_raise Ecto.MultipleResultsError, ~s|expected at most one result but got 10 in query:\n\n| <>
-                                            ~s|from c in Ecto.Integration.CorruptedPk,\n| <>
-                                            ~s|  where: c.a == ^\"abc\"\n|, fn ->
-      TestRepo.get(CorruptedPk, "abc")
-    end
-
-    assert %CorruptedPk{a: "abc"} = schema |> Ecto.Changeset.change(force: true) |> TestRepo.update!()
-
-    assert_raise Ecto.MultipleResultsError, fn ->
-      TestRepo.delete!(schema)
-    end
+    assert_raise Ecto.MultiplePrimaryKeyError,
+                 ~r|expected delete on corrupted_pk to return at most one entry but got 3 entries|,
+                 fn -> TestRepo.delete!(schema) end
   end
 
   test "Repo.insert! escape" do

lib/ecto/adapters/sql.ex

@@ -88,22 +88,22 @@ defmodule Ecto.Adapters.SQL do
                  {kind, conflict_params, _} = on_conflict, returning, opts) do
         {fields, values} = :lists.unzip(params)
         sql = @conn.insert(prefix, source, fields, [fields], on_conflict, returning)
-        Ecto.Adapters.SQL.struct(repo, @conn, sql, values ++ conflict_params, kind, returning, opts)
+        Ecto.Adapters.SQL.struct(repo, @conn, sql, {:insert, source, []}, values ++ conflict_params, kind, returning, opts)
       end
 
       @doc false
-      def update(repo, %{source: {prefix, source}}, fields, filter, returning, opts) do
+      def update(repo, %{source: {prefix, source}}, fields, params, returning, opts) do
         {fields, values1} = :lists.unzip(fields)
-        {filter, values2} = :lists.unzip(filter)
+        {filter, values2} = :lists.unzip(params)
         sql = @conn.update(prefix, source, fields, filter, returning)
-        Ecto.Adapters.SQL.struct(repo, @conn, sql, values1 ++ values2, :raise, returning, opts)
+        Ecto.Adapters.SQL.struct(repo, @conn, sql, {:update, source, params}, values1 ++ values2, :raise, returning, opts)
       end
 
       @doc false
-      def delete(repo, %{source: {prefix, source}}, filter, opts) do
-        {filter, values} = :lists.unzip(filter)
+      def delete(repo, %{source: {prefix, source}}, params, opts) do
+        {filter, values} = :lists.unzip(params)
         sql = @conn.delete(prefix, source, filter, [])
-        Ecto.Adapters.SQL.struct(repo, @conn, sql, values, :raise, [], opts)
+        Ecto.Adapters.SQL.struct(repo, @conn, sql, {:delete, source, params}, values, :raise, [], opts)
       end
 
       ## Transaction
@@ -551,16 +551,17 @@ defmodule Ecto.Adapters.SQL do
   end
 
   @doc false
-  def struct(repo, conn, sql, values, on_conflict, returning, opts) do
+  def struct(repo, conn, sql, {operation, source, params}, values, on_conflict, returning, opts) do
     case query(repo, sql, values, fn x -> x end, opts) do
       {:ok, %{rows: nil, num_rows: 1}} ->
         {:ok, []}
       {:ok, %{rows: [values], num_rows: 1}} ->
         {:ok, Enum.zip(returning, values)}
       {:ok, %{num_rows: 0}} ->
         if on_conflict == :nothing, do: {:ok, []}, else: {:error, :stale}
-      {:ok, %{rows: nil, num_rows: num_rows}} when num_rows > 1 ->
-        raise Ecto.MultipleResultsError, sql_query: sql, count: num_rows
+      {:ok, %{num_rows: num_rows}} when num_rows > 1 ->
+        raise Ecto.MultiplePrimaryKeyError,
+              source: source, params: params, count: num_rows, operation: operation
       {:error, err} ->
         case conn.to_constraints(err) do
           []          -> raise err

lib/ecto/exceptions.ex

@@ -204,22 +204,37 @@ defmodule Ecto.MultipleResultsError do
   defexception [:message]
 
   def exception(opts) do
-    sql_query = Keyword.get(opts, :sql_query)
+    query = Keyword.fetch!(opts, :queryable) |> Ecto.Queryable.to_query
     count = Keyword.fetch!(opts, :count)
-    query =
-      if sql_query do
-        sql_query
-      else
-        opts
-        |> Keyword.fetch!(:queryable)
-        |> Ecto.Queryable.to_query()
-        |> Inspect.Ecto.Query.to_string()
-      end
 
     msg = """
     expected at most one result but got #{count} in query:
 
-    #{query}
+    #{Inspect.Ecto.Query.to_string(query)}
+    """
+
+    %__MODULE__{message: msg}
+  end
+end
+
+defmodule Ecto.MultiplePrimaryKeyError do
+  defexception [:message]
+
+  def exception(opts) do
+    operation = Keyword.fetch!(opts, :operation)
+    source = Keyword.fetch!(opts, :source)
+    params = Keyword.fetch!(opts, :params)
+    count = Keyword.fetch!(opts, :count)
+
+    msg = """
+    expected #{operation} on #{source} to return at most one entry but got #{count} entries.
+
+    This typically means the field(s) set as primary_key in your schema/source
+    are not enough to uniquely identify entries in the repository.
+
+    Those are the parameters sent to the repository:
+
+    #{inspect params}
     """
 
     %__MODULE__{message: msg}