Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

select_merge regression #4564

Open
JamesKentDemcon opened this issue Jan 9, 2025 · 8 comments
Open

select_merge regression #4564

JamesKentDemcon opened this issue Jan 9, 2025 · 8 comments
Labels

Comments

@JamesKentDemcon
Copy link

Elixir version

1.15.4

Database and Version

MySQL 8.0.40

Ecto Versions

ecto 3.10.3, ecto_sql 3.10.2

Database Adapter and Versions (postgrex, myxql, etc)

myxql 0.6.4

Current behavior

Merging with maps which are nil throws an exception. The example below is taken from #3218, where this problem was initially mentioned. As described, if the translation t is missing, an error is thrown.

def product_with_translations(sku_code, locale, translated_fields \\ ~w(name)a) do
  Product
  |> from(as: :product)
  |> where(sku_code: ^sku_code)
  |> join(
    :left, [product: p],
    t in Product.Translation,
    on: t.product_id == p.id and t.locale == ^locale,
    as: :translation
  )
  |> select([product: p], p)
  |> select_merge([translation: t], map(t, ^translated_fields))
end

results in the error:

 ** (ArgumentError) cannot merge because the left side is not a map, got: nil

This issue was supposedly solved with #3219. Looking at git blame, the relevant lines were merged.

However, when I look at a git blame of the most recent version of Ecto, the relevant lines are missing and only a subset of the fix is merged.

Expected behavior

I would expect no exception to be thrown as is intended by the fix in #3219.

@JamesKentDemcon
Copy link
Author

I was doing some experimenting, and interestingly enough, I am able to get this to work if I call the functions in IEx. It seems just not to work in my testing environment. I'll have to do some more digging.

@josevalim
Copy link
Member

Can you try removing MIX_ENV=test mix clean and, if that doesn't fix it, rm -rf _build/test, to rule out it being a build issue?

@JamesKentDemcon
Copy link
Author

If you meant running MIX_ENV=test mix clean, then neither of those steps worked.

@greg-rychlewski
Copy link
Member

it might be your test database is empty but whatever is being called in iex is not

@JamesKentDemcon
Copy link
Author

The test database is the one controlled by ExUnit so its cleaned up after every test.

@v0idpwn
Copy link
Member

v0idpwn commented Jan 9, 2025

Part of it was removed right after the merge: d1910c4

I'll try to write a test that reproduces the behaviour, and potentially restore these clauses.

@v0idpwn
Copy link
Member

v0idpwn commented Jan 9, 2025

Not able to reproduce here either :)

@greg-rychlewski
Copy link
Member

From the description it seems it was happening when the left side of the join is nil. So if it's a left join I think we won't even get to the part of the code where this error can raise because the result is empty.

Maybe we improved that after Ecto 3.10. @JamesKentDemcon does using the latest Ecto solve the issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants