Allow select: map(x, fields), select_merge: %{} as source query for insert_all. Fixes #4430 #4431
Conversation
|
|
||
| TestRepo.insert_all(MySchema, query) | ||
|
|
||
| assert_received {:insert_all, %{source: "my_schema"}, {%Ecto.Query{}, _params}} |
There was a problem hiding this comment.
Is there a way to check the fields here? We need to check that both x and y are being sent.
| Enum.map(args, fn {field, _} -> extract_field(field, dumper) end) | ||
|
|
||
| %Ecto.Query.SelectExpr{expr: {:merge, _ctx, merge_args}, take: %{^ix => {_fun, take_fields}}} -> | ||
| [_, {_, _, merge_fields}] = merge_args |
There was a problem hiding this comment.
I believe it can be in the reverse order too like this
{:merge, [],
[
{:%{}, [], [y: {{:., [type: :binary], [{:&, [], [0]}, :yyy]}, [], []}]},
{:&, [], [0]}
]}
|
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 |
|
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 I can see in ecto/test/ecto/query/planner_test.exs Line 2311 in e0bb729 fields field on the Ecto.Query.SelectExpr struct. Would it be possible to use this field to extract the headers?
|
|
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. |
No description provided.