ecto icon indicating copy to clipboard operation
ecto copied to clipboard

Allow select: map(x, fields), select_merge: %{} as source query for insert_all. Fixes #4430

Open nmk opened this issue 1 year ago • 4 comments

nmk avatar Jun 13 '24 15:06 nmk

We should be extra sure we are catching all the cases here https://github.com/elixir-ecto/ecto/blob/master/lib/ecto/query/builder/select.ex#L367

greg-rychlewski avatar Jun 13 '24 18:06 greg-rychlewski

Playing around with it more. This can get pretty dicey with nested merges. For example this query

from s in MySchema,
          where: s.x > ^threshold,
          select: %{y: s.y},
          select_merge: map(s, [:x]),
          select_merge: %{x: s.x}

turns into this

{:merge, [],
   [
     {:merge, [],
      [
        {:%{}, [], [y: {{:., [type: :binary], [{:&, [], [0]}, :yyy]}, [], []}]},
        {:&, [], [0]}
      ]},
     {:%{}, [], [x: {{:., [type: :string], [{:&, [], [0]}, :x]}, [], []}]}
   ]}

greg-rychlewski avatar Jun 14 '24 01:06 greg-rychlewski

@greg-rychlewski I can see in https://github.com/elixir-ecto/ecto/blob/e0bb7299fe0540998aeb6b10e5b87b63f66f61cb/test/ecto/query/planner_test.exs#L2311 that there is a fields field on the Ecto.Query.SelectExpr struct. Would it be possible to use this field to extract the headers?

nmk avatar Jun 14 '24 06:06 nmk

That will lose you the field names. However, in the planner module, we have a function for extracting fields of a subquery, maybe that can be used, but I think in this case it is probably better to improve the error message and have a single select with a map for simplicity, for full control over the database changes.

josevalim avatar Jun 14 '24 09:06 josevalim