Ecto: select_merge ignores nested fields

[a12e]

Environment

• Elixir version (elixir -v): Elixir 1.9.0 (compiled with Erlang/OTP 22)
• Database and version (PostgreSQL 9.4, MongoDB 3.2, etc.): PostgreSQL 9.6
• Ecto version (mix deps): ecto 3.1.7
• Database adapter and version (mix deps): ecto_sql 3.1.6 & postgrex 0.15.0
• Operating system: Linux 5.1.16-1-default

Source

defmodule Migration do
  use Ecto.Migration

  def change do
    create table :posts do
      add :title, :text
    end

    create table :comments do
      add :content, :text
      add :post_id, references(:posts)
    end
  end
end

defmodule EctoBug.Post do
  use Ecto.Schema

  schema "posts" do
    field :title, :string
    has_many :comments, EctoBug.Comment
  end
end

defmodule EctoBug.Comment do
  use Ecto.Schema

  schema "comments" do
    field :content, :string
    field :like_count, :integer, virtual: true
    belongs_to :post, EctoBug.Post
  end
end

defmodule EctoBug do
alias EctoBug.{Migration, Repo, Post, Comment}
import Ecto.Query

def test do   
  Repo.start_link()
  Ecto.Migrator.up(Repo, 0, Migration)
  post = Repo.insert!(%Post{title: "My post"})
  Repo.insert! %Comment{post: post, content: "Great"}
  Repo.insert! %Comment{post: post, content: "Bad"}

  Post
  |> join(:left, [p], c in assoc(p, :comments))
  |> preload([_, c], comments: c)
  |> select_merge([...], %{comments: %{like_count: type(^10000, :integer)}}) # just select a constant for simplicity
  |> Repo.all
end
end

Current behavior

The nested field, the virtual field of the association, is not set by Ecto, while the SQL request correctly requested 10000 for the field like_count.

Output:

[
  %EctoBug.Post{
    __meta__: #Ecto.Schema.Metadata<:loaded, "posts">,
    comments: [
      %EctoBug.Comment{
        __meta__: #Ecto.Schema.Metadata<:loaded, "comments">, 
        content: "Great",
        id: 1,
        like_count: nil, # <- PROBLEM
        post: #Ecto.Association.NotLoaded<association :post is not loaded>,
        post_id: 4
      },
      %EctoBug.Comment{
        __meta__: #Ecto.Schema.Metadata<:loaded, "comments">,
        content: "Bad",
        id: 2,
        like_count: nil, # <- PROBLEM
        post: #Ecto.Association.NotLoaded<association :post is not loaded>,
        post_id: 4
      }
    ],
    id: 4,
    title: "My post"
  }
]

SQL request:

SELECT p0."id", p0."title", $1::bigint, c1."id", c1."content", c1."post_id" FROM "posts" AS p0 LEFT OUTER JOIN "comments" AS c1 ON c1."post_id" = p0."id" [10000]

Expected behavior

I expect to be able to merge my field like_count into the "nested" association comments

[
  %EctoBug.Post{
    __meta__: #Ecto.Schema.Metadata<:loaded, "posts">,
    comments: [
      %EctoBug.Comment{
        __meta__: #Ecto.Schema.Metadata<:loaded, "comments">, 
        content: "Great",
        id: 1,
        like_count: 10000, # <- OK
        post: #Ecto.Association.NotLoaded<association :post is not loaded>,
        post_id: 4
      },
      %EctoBug.Comment{
        __meta__: #Ecto.Schema.Metadata<:loaded, "comments">,
        content: "Bad",
        id: 2,
        like_count: 10000, # <- OK
        post: #Ecto.Association.NotLoaded<association :post is not loaded>,
        post_id: 4
      }
    ],
    id: 4,
    title: "My post"
  }
]

[josevalim]

@a12e can you please provide the steps above in a repository or in a failed test case we can integrate into the repository? That will make it much easier for us to fix this issue. Thank you.

[a12e]

@josevalim sure, i wrote the two following tests for the Ecto integration test suite:

  # Passes.
  test "select_merge for a root field" do
    TestRepo.insert!(%Post{title: "p1"})

    posts =
      from(p in Post)
      |> select_merge(%{title: type(^"override", :string)})
      |> TestRepo.all()

    assert [%{title: "override"}] = posts
  end

  # Fails. The text of the comment is "c1" instead of "override".
  # If we had selected a virtual field, it would have been set to nil.
  test "select_merge for a nested field" do
    p1 = TestRepo.insert!(%Post{title: "p1"})
    TestRepo.insert!(%Comment{text: "c1", post: p1})

    posts =
      from(p in Post)
      |> join(:left, [p], c in assoc(p, :comments))
      |> preload([_, c], comments: c)
      |> select_merge([_, c], %{comments: %{text: type(^"override", :string)}})
      |> TestRepo.all()

    assert [%{comments: [%{text: "override"}]}] = posts
  end

Output:

root@fc6a06705653:/app/ecto_sql# ECTO_PATH=../ecto ECTO_ADAPTER=pg PG_URL="postgres:[email protected]:5432" mix test ../ecto/integration_test/cases/preload.exs:14:27
Excluding tags: [:test]
Including tags: [line: "14", line: "27"]

.

  1) test select_merge for a nested field (Ecto.Integration.PreloadTest)
     /app/ecto/integration_test/cases/preload.exs:27
     match (=) failed
     code:  assert [%{comments: [%{text: "override"}]}] = posts
     right: [
              %Ecto.Integration.Post{
                __meta__: #Ecto.Schema.Metadata<:loaded, "posts">,
                author: #Ecto.Association.NotLoaded<association :author is not loaded>,
                author_id: nil,
                bid: nil,
                comments: [
                  %Ecto.Integration.Comment{
                    __meta__: #Ecto.Schema.Metadata<:loaded, "comments">,
                    author: #Ecto.Association.NotLoaded<association :author is not loaded>,
                    author_id: nil,
                    id: 1,
                    lock_version: 1,
                    post: #Ecto.Association.NotLoaded<association :post is not loaded>,
                    post_id: 2,
                    post_permalink: #Ecto.Association.NotLoaded<association :post_permalink is not loaded>,
                    text: "c1"
                  }
                ],
                comments_authors: #Ecto.Association.NotLoaded<association :comments_authors is not loaded>,
                comments_authors_permalinks: #Ecto.Association.NotLoaded<association :comments_authors_permalinks is not loaded>,
                constraint_users: #Ecto.Association.NotLoaded<association :constraint_users is not loaded>,
                cost: nil,
                counter: nil,
                id: 2,
                inserted_at: ~N[2019-07-26 09:55:06],
                intensities: nil,
                intensity: nil,
                links: nil,
                meta: nil,
                permalink: #Ecto.Association.NotLoaded<association :permalink is not loaded>,
                post_user_composite_pk: #Ecto.Association.NotLoaded<association :post_user_composite_pk is not loaded>,
                posted: nil,
                public: true,
                temp: "temp",
                text: nil,
                title: "p1",
                unique_users: #Ecto.Association.NotLoaded<association :unique_users is not loaded>,
                update_permalink: #Ecto.Association.NotLoaded<association :update_permalink is not loaded>,
                updated_at: ~N[2019-07-26 09:55:06],
                users: #Ecto.Association.NotLoaded<association :users is not loaded>,
                users_comments: #Ecto.Association.NotLoaded<association :users_comments is not loaded>,
                uuid: "a19757ff-debd-4812-b175-7d926216244c",
                visits: nil
              }
            ]
     stacktrace:
       /app/ecto/integration_test/cases/preload.exs:38: (test)



Finished in 1.4 seconds
35 tests, 1 failure, 33 excluded

Randomized with seed 78540

It seems that Ecto doesn't "fill" the nested structs it receives from Postgres, when using select_merge. :wink:

[josevalim]

Thanks but unfortunately everything is merged before we compute associations and it would require a lot of work in Ecto for us to compute that the merge applies to the association and postpone its handling. It is likely doable but it sounds like an uncommon case. If someone wants to investigate it, we can review it, but be warned it is not straight-forward. :)

The best course of action in this case is to load both posts and comments separately. You can then group comments by post_id so they can be easily accessible.

[a12e]

Thank you for the answer @josevalim !

For the record, we'll finally regular select for this.

select_merge would have been nicer, because we have a lot of joins and preloads in the same query, without a select statement. Ecto was doing the magic of assembling the tree of associations for us. With regular select, it forces us to know and write explicitly the complete tree of the query.

So, we're falling back to something like this:

    posts =
      from(p in Post)
      |> join(:left, [p], c in assoc(p, :comments))
      |> preload([_, c], comments: c)
      |> select([p, c], %{p | comments: [%{c | text: type(^"override", :string)}]})
      |> TestRepo.all()

Thanks again for your quick replies and your great work on these projects :+1: