Fix nil map associations (#4631)

Author: greg-rychlewski

integration_test/cases/preload.exs

@@ -526,6 +526,51 @@ defmodule Ecto.Integration.PreloadTest do
     assert [] = sort_by_id(p3.comments)
   end
 
+  test "take with join nil maps (many association)" do
+    p = TestRepo.insert!(%Post{})
+
+    # many
+    query =
+      from p in Post,
+        left_join: c in Comment,
+        on: p.id == c.post_id,
+        select: map(p, [:id, comments: [:id, :post_id]]),
+        preload: [comments: c]
+
+    assert TestRepo.one(query) == %{id: p.id, comments: []}
+
+    query =
+      from p in Post,
+        left_join: c in Comment,
+        on: p.id == c.post_id,
+        select: map(p, [:id, comments: [:id, :post_id]]),
+        preload: [:comments]
+
+    assert TestRepo.one(query) == %{id: p.id, comments: []}
+  end
+
+  test "take with join nil maps (one association)" do
+    p = TestRepo.insert!(%Post{})
+
+    query =
+      from p in Post,
+        left_join: u in User,
+        on: p.author_id == u.id,
+        select: map(p, [:id, author: [:id, :name]]),
+        preload: [author: u]
+
+    assert TestRepo.one(query) == %{id: p.id, author: nil}
+
+    query =
+      from p in Post,
+        left_join: u in User,
+        on: p.author_id == u.id,
+        select: map(p, [:id, author: [:id, :name]]),
+        preload: [:author]
+
+    assert TestRepo.one(query) == %{id: p.id, author: nil}
+  end
+
   test "preload through with take" do
     %Post{id: pid1} = TestRepo.insert!(%Post{})
 

lib/ecto/repo/assoc.ex

@@ -33,14 +33,24 @@ defmodule Ecto.Repo.Assoc do
   end
 
   defp merge([struct|sub_structs], {primary_keys, cache, dict, sub_dicts}, parent_key) do
-    child_key =
+    {struct, child_key} =
       if struct do
-        for primary_key <- primary_keys do
-          case Map.get(struct, primary_key) do
-            nil -> raise Ecto.NoPrimaryKeyValueError, struct: struct
-            value -> value
-          end
-        end
+        {child_key, all_nil?} =
+          Enum.map_reduce(primary_keys, true, fn primary_key, all_nil? ->
+            case struct do
+              %_{^primary_key => nil} -> raise Ecto.NoPrimaryKeyValueError, struct: struct
+              # We allow maps to be returned with all `nil` values in queries without
+              # preloads. For preloads we have to treat maps with all `nil` values as
+              # `nil` instead of a map otherwise we can't associate the missing
+              # association to the parent struct
+              %{^primary_key => value} -> {value, all_nil? and value == nil}
+              %{} -> raise Ecto.NoPrimaryKeyValueError, struct: struct
+            end
+          end)
+
+        if all_nil?, do: {nil, nil}, else: {struct, child_key}
+      else
+        {nil, nil}
       end
 
     # Traverse sub_structs adding one by one to the tree.