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

Always keep map literal in select_merge if keys are distinct #4477

Merged

Conversation

greg-rychlewski
Copy link
Member

@greg-rychlewski greg-rychlewski commented Aug 8, 2024

Previously we discussed if insert_all can support select_merge with multiple map literals when one of the inner maps has interpolations. For example:

from p in "posts", select: %{title: ^"title"}, select_merge: %{visits: 1}

Because the inner map has an interpolation, we don't know whether it will be wiped out and our query params will be incorrect. So we turn the select expression into merge(%{...}, %{....}). And this merge expression doesn't work with insert_all due to difficulty extracting fields from merge.

There is one condition we can be certain the interpolation won't be wiped out: when all the merges have distinct keys. In this case we can keep the map literal and it will work with insert_all.

This would give a lot more flexibility to user. If they have dynamic keys/values they want to turn into multiple merge statements, they can always remove duplication in Elixir before going to Ecto land.

For example this:

conditions = [k1: v1, k2: v2, k3: v3, ...]
base = from p in "posts", select: %{k0: v0}

Enum.reduce(conditions, base, fn {k, v}, query -> from q in query, select_merge: %{^k => ^v}

They can ensure no duplications in conditions beforehand.

Copy link
Member

@josevalim josevalim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really nice improvement! ❤️

@greg-rychlewski greg-rychlewski merged commit 07f2529 into elixir-ecto:master Aug 9, 2024
6 checks passed
@greg-rychlewski greg-rychlewski deleted the select_merge_overlap branch August 9, 2024 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants