Skip to content

Commit

Permalink
Fix select_merge on map with outer join (#3219)
Browse files Browse the repository at this point in the history
Resolves #3218

Three cases have been added to merge processing:

- If the merge argument (`right`) is `nil`, return `left` unmodified. This can
  happen with a `:left_join` or `:full_join`.

- If the merge source (`left`) is `nil`, return `right` unmodified. This can
  happen with a `:right_join` or `:full_join`.

- If both the source and argument are `nil`, return `nil`. This _probably_
  shouldn’t happen, but is currently included for completeness.
  • Loading branch information
halostatue committed Feb 3, 2020
1 parent 6c7afef commit 327a094
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 4 deletions.
32 changes: 31 additions & 1 deletion integration_test/cases/repo.exs
Original file line number Diff line number Diff line change
Expand Up @@ -1237,7 +1237,8 @@ defmodule Ecto.Integration.RepoTest do
end

test "merge" do
%Post{} = TestRepo.insert!(%Post{title: "1", counter: nil})
date = Date.utc_today()
%Post{id: post_id} = TestRepo.insert!(%Post{title: "1", counter: nil, posted: date, public: false})

# Merge on source
assert [%Post{title: "2"}] =
Expand All @@ -1256,6 +1257,35 @@ defmodule Ecto.Integration.RepoTest do
Post |> select([p], merge(%{title: p.title}, %{title: "2"})) |> TestRepo.all()
assert [%{title: "2"}] =
Post |> select([p], %{title: p.title}) |> select_merge([p], %{title: "2"}) |> TestRepo.all()

# Merge on outer join with map
%Permalink{} = TestRepo.insert!(%Permalink{post_id: post_id, url: "Q", title: "Z"})

# left join record is present
assert [%{url: "Q", title: "1", posted: date}] =
Permalink
|> join(:left, [l], p in Post, on: l.post_id == p.id)
|> select([l, p], merge(l, map(p, ^~w(title posted)a)))
|> TestRepo.all()

assert [%{url: "Q", title: "1", posted: date}] =
Permalink
|> join(:left, [l], p in Post, on: l.post_id == p.id)
|> select_merge([_l, p], map(p, ^~w(title posted)a))
|> TestRepo.all()

# left join record is not present
assert [%{url: "Q", title: "Z", posted: nil}] =
Permalink
|> join(:left, [l], p in Post, on: l.post_id == p.id and p.public)
|> select([l, p], merge(l, map(p, ^~w(title posted)a)))
|> TestRepo.all()

assert [%{url: "Q", title: "Z", posted: nil}] =
Permalink
|> join(:left, [l], p in Post, on: l.post_id == p.id and p.public)
|> select_merge([_l, p], map(p, ^~w(title posted)a))
|> TestRepo.all()
end

test "merge with update on self" do
Expand Down
10 changes: 7 additions & 3 deletions integration_test/support/schemas.exs
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
Code.require_file "types.exs", __DIR__
Code.require_file("types.exs", __DIR__)

defmodule Ecto.Integration.Schema do
defmacro __using__(_) do
quote do
use Ecto.Schema

type =
Application.get_env(:ecto, :primary_key_type) ||
raise ":primary_key_type not set in :ecto application"
raise ":primary_key_type not set in :ecto application"

@primary_key {:id, type, autogenerate: true}
@foreign_key_type type
end
Expand Down Expand Up @@ -104,14 +106,16 @@ defmodule Ecto.Integration.Permalink do

schema "permalinks" do
field :url, :string, source: :uniform_resource_locator
field :title, :string
field :posted, :date, virtual: true
belongs_to :post, Ecto.Integration.Post, on_replace: :nilify
belongs_to :update_post, Ecto.Integration.Post, on_replace: :update, foreign_key: :post_id, define_field: false
belongs_to :user, Ecto.Integration.User
has_many :post_comments_authors, through: [:post, :comments_authors]
end

def changeset(schema, params) do
Ecto.Changeset.cast(schema, params, [:url])
Ecto.Changeset.cast(schema, params, [:url, :title])
end
end

Expand Down
15 changes: 15 additions & 0 deletions lib/ecto/query.ex
Original file line number Diff line number Diff line change
Expand Up @@ -1232,6 +1232,21 @@ defmodule Ecto.Query do
being merged on must be a struct or a map. If it is a struct, the fields
merged later on must be part of the struct, otherwise an error is raised.
If the argument to `:select_merge` is a constructed struct (`struct/2`) or
map (`map/2`) where the source to `struct/2` or `map/2` may be a `nil` value
(as in an outer join), the source will be returned unmodified.
query =
Post
|> join(:left, [p], t in Post.Translation,
on: t.post_id == p.id and t.locale == ^"en"
)
|> select_merge([_p, t], map(t, ^~w(title summary)a))
If there is no English translation for the post, the untranslated post
`title` will be returned and `summary` will be `nil`. If there is, both
`title` and `summary` will be the value from `Post.Translation`.
`select_merge` cannot be used to set fields in associations, as
associations are always loaded later, overriding any previous value.
"""
Expand Down
9 changes: 9 additions & 0 deletions lib/ecto/repo/queryable.ex
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,15 @@ defmodule Ecto.Repo.Queryable do
{%{}, %{}} ->
Map.merge(left, right)

{%{}, nil} ->
left

{nil, %{}} ->
right

{nil, nil} ->
nil

{_, %{}} ->
raise ArgumentError,
"cannot merge because the left side is not a map, got: #{inspect(left)}"
Expand Down

0 comments on commit 327a094

Please sign in to comment.