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

insert_all with source ( update syntax) #4469

Merged

Conversation

greg-rychlewski
Copy link
Member

Closes #4464

I also think we can make it work with selecting just a source. This could be useful if for instance you are trying to move data to a new table.

@greg-rychlewski
Copy link
Member Author

I messed up. fields is already translated to :source. I'll try to see if there's anything else I can do.

lib/ecto/repo/schema.ex Outdated Show resolved Hide resolved
lib/ecto/repo/schema.ex Outdated Show resolved Hide resolved
@greg-rychlewski
Copy link
Member Author

@josevalim thanks for the great feedback. i believe i addressed all of it :)

@josevalim
Copy link
Member

@greg-rychlewski I have been thinking about this PR and I am not really sure if we should support the update the syntax. The biggest issue is that, if you write this:

select: %{p | foo: 321}

It tells me you are selecting *all of p and updating the value of :foo. The fact we are selecting only a subset is a bit awkward to me, even though it is the most logical. What would we do if the user really wants to select all of them in the future? Sorry for not noticing this earlier, but we can probably keep the other warts of this PR (except the update)?

@greg-rychlewski greg-rychlewski merged commit 98ef18e into elixir-ecto:master Aug 3, 2024
6 checks passed
@greg-rychlewski greg-rychlewski deleted the insert_all_update branch August 3, 2024 23:04
@greg-rychlewski
Copy link
Member Author

@josevalim I'll get the read_only -> writeable change done this week so we are not stuck not being able to release for long. In case someone asks you to

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.

insert_all with query select and map update syntax produces error
2 participants