Support select: map(x, fields), select_merge: %{...} in source query for insert_all

[nmk]

Elixir version

1.16.3

Database and Version

PostgreSQL 15.0

Ecto Versions

master

Database Adapter and Versions (postgrex, myxql, etc)

postgrex

Current behavior

Repo.insert_all(Foo, from x in Foo, select: %{foo: x.foo, bar: x.bar}, select_merge: %{baz: some_fun(x.baz)})

works as intended.

Repo.insert_all(Foo, from x in Foo, select: map(x, [:foo, :bar]), select_merge: %{baz: some_fun(x.baz)})

raises an ArgumentError stating that the source query does not select into a map.

Expected behavior

The above two snippets should both be supported as they should result in the same source query being produced.

I would be happy to take a stab at this if this is indeed the intended behaviour.

[josevalim]

Please take a stab at it. It may be that making it work is just too complex, and then we could improve the error message, but we should definitely try it!

[nmk]

Please take a stab at it. It may be that making it work is just too complex, and then we could improve the error message, but we should definitely try it!

I have made a PR which adds a test and the simplest code to make the test pass.

#4431

As far as I can see the test (as others in the same vein) only assert that a query would be successfully created, not that it would be correct. Is there a way to assert on the query itself?

[aglassman]

Using insert_all also fails with struct/2, and %{c | field: ^val}. These are all cases that work with select merge normally, but fail for insert_all. Just happened to dig into this today.

  # This does not work!
  def test_1() do
    # initial query
    query = from(c in Click, select: c, limit: 1)

    Ecto.Multi.new()
    |> Ecto.Multi.insert_all(:click, Click, query)
    |> Repo.transaction()
  end

  # This does not work!
  def test_2() do
    # initial query
    query = from(c in Click, select: struct(c, [:short_code]), limit: 1)

    Ecto.Multi.new()
    |> Ecto.Multi.insert_all(:click, Click, query)
    |> Repo.transaction()
  end

  # This does not work!
  def test_3(short_code_override \\ "default sc") do
    # initial query
    query = from(c in Click, select: %{c | short_code: ^short_code_override}, limit: 1)

    Ecto.Multi.new()
    |> Ecto.Multi.insert_all(:click, Click, query)
    |> Repo.transaction()
  end

[greg-rychlewski]

Hi @aglassman ,

Something like this will not work because the update syntax is essentially post processing. Whatever is returned from the query will later be updated in Elixir. Thus it can't work for insert_all where you need everything to happen in the database.

 query = from(c in Click, select: %{c | short_code: ^short_code_override}, limit: 1)

But this should work. I tested it:

query = from(c in Click, select: struct(c, [:short_code]), limit: 1)

Maybe your ecto version is old? If you try on the latest commit does it still happen?

[greg-rychlewski]

I thought maybe it would make sense to select the entire struct. But after playing around with it there are quite a few issues. Mostly because you will be selecting primary keys and foreign keys which will cause issues in a lot of cases.

[aglassman]

@greg-rychlewski Thanks for the explanation into why the update syntax does not work, that is good to know. As for struct/2 usage, I mistakingly assumed the error was related to the insert_all select using struct, when it was actually a constraint violation (inserted_at is required). My apologies on that one!