Repository: elixir-ecto/ecto
Pull request #4148 (2023-04-07 22:07:31)

Use correct fields when select merging into a source
When select merging into a source there is a bit of a hiccup where you don't immediately know all of the source's fields. Last year there was a fix made for this: https://github.com/elixir-ecto/ecto/pull/3989.

The issue is that it always assumed the source you are merging into is the `from` source. I was playing around with this and saw there are situations where might have a `join` source (see the new test).

greg-rychlewski: The integration test failures look related to the ecto sql constraint changes and not here. I can look closer a bit later.

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5d6c7dc3..2c0fc7f0 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -20,7 +20,7 @@ Queries have also been improved, to support LIMIT WITH TIES as well as materiali
   * [Ecto.Changeset] Add `get_assoc`/`get_embed`
   * [Ecto.Changeset] Add `field_missing?/2`
   * [Ecto.Changeset] Add `changed?/2` and `changed?/3` with predicates support
-  * [Ecto.Changeset] Alow `Regex` to be used in constraint names for exact matches
+  * [Ecto.Changeset] Allow `Regex` to be used in constraint names for exact matches
   * [Ecto.Changeset] Allow `:empty_values` option in `cast/4` to include a function which must return true if the value is empty
   * [Ecto.Changeset] `cast/4` will by default consider strings made only of whitespace characters to be empty
   * [Ecto.Query] Support materialized option in CTEs
diff --git a/lib/ecto/query/builder/select.ex b/lib/ecto/query/builder/select.ex
index da9306d2..2e816d88 100644
--- a/lib/ecto/query/builder/select.ex
+++ b/lib/ecto/query/builder/select.ex
@@ -412,7 +412,7 @@ defmodule Ecto.Query.Builder.Select do
       select | expr: expr,
                params: old_params ++ bump_subquery_params(new_params, old_subqueries),
                subqueries: old_subqueries ++ new_subqueries,
-               take: merge_take(query.from.source, old_expr, old_take, new_take),
+               take: merge_take(query, old_expr, old_take, new_take),
                aliases: merge_aliases(old_aliases, new_aliases)
     }
 
@@ -470,7 +470,7 @@ defmodule Ecto.Query.Builder.Select do
     end)
   end
 
-  defp merge_take(source, old_expr, %{} = old_take, %{} = new_take) do
+  defp merge_take(query, old_expr, %{} = old_take, %{} = new_take) do
     Enum.reduce(new_take, old_take, fn {binding, {new_kind, new_fields} = new_value}, acc ->
       case acc do
         %{^binding => old_value} ->
@@ -480,12 +480,17 @@ defmodule Ecto.Query.Builder.Select do
           # If merging with a schema, add the schema's query fields. This comes in handy if the user
           # is merging fields with load_in_query = false.
           # If merging with a schemaless source, do nothing so the planner can take all the fields.
-          case {old_expr, source} do
-            {{:&, _, [^binding]}, {_source, schema}} when not is_nil(schema) ->
-              Map.put(acc, binding, {new_kind, Enum.uniq(new_fields ++ schema.__schema__(:query_fields))})
+          case old_expr do
+            {:&, _, [^binding]} ->
+              source = Enum.at([query.from | query.joins], binding).source
 
-            {{:&, _, [^binding]}, _} ->
-                acc
+              case source do
+                {_, schema} when schema != nil ->
+                  Map.put(acc, binding, {new_kind, Enum.uniq(new_fields ++ schema.__schema__(:query_fields))})
+
+                _ ->
+                  acc
+              end
 
             _ ->
               Map.put(acc, binding, new_value)


Test patch:
diff --git a/test/ecto/query/builder/select_test.exs b/test/ecto/query/builder/select_test.exs
index 4dbe50d5..7a8902ab 100644
--- a/test/ecto/query/builder/select_test.exs
+++ b/test/ecto/query/builder/select_test.exs
@@ -566,6 +566,12 @@ defmodule Ecto.Query.Builder.SelectTest do
       assert query.select.params == []
       assert query.select.take == %{0 => {:any, [:dislikes, :title, :likes]}}
 
+      query = from p in "posts", join: c in Comment, on: true, select: c, select_merge: map(c, [:dislikes])
+
+      assert Macro.to_string(query.select.expr) == "&1"
+      assert query.select.params == []
+      assert query.select.take == %{1 => {:map, [:dislikes, :title, :likes]}}
+
       # On take with schemaless source
       query = from c in "comments", select: [:title], select_merge: [:likes]