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

Inconsistency between empty_values and nil #4555

Open
josevalim opened this issue Dec 3, 2024 · 2 comments
Open

Inconsistency between empty_values and nil #4555

josevalim opened this issue Dec 3, 2024 · 2 comments
Labels

Comments

@josevalim
Copy link
Member

Elixir version

All

Database and Version

All

Ecto Versions

v3.12

Database Adapter and Versions (postgrex, myxql, etc)

All

Current behavior

Today there is some slight inconsistency between empty values and nil. When you pass an empty string to Ecto.Changeset.cast, it defaults to the field default value (which may not be nil). However, if you pass nil, it always sets it as nil.

Expected behavior

In my mind, empty_values should behave the exact same as nil, which is to set the field to nil.

On the other hand, we may have non-nullable fields in the future (it will be a type system requirement), so setting the field to nil should either be a cast error or rely on the default value. The important though is that empty_values and nil behave the same. One suggestion is to deprecate empty_values in favor of nil_values, which accept the same configuration, but behave as nil.

Thoughts?

@greg-rychlewski
Copy link
Member

I think for arrays we are removing entries that match empty values rather than turning them into nil. So if we go with this change we probably have to make the two behaviours consistent?

The current behaviours seem aligned to me since changing it to the default value is basically like removing an explicitly given change.

In terms of making the change or not, I don't really have use cases to fall back on. I can only say it seems theoretically useful to have both behaviours. Maybe some situations you want to treat values as if they are nil and some situations you want to treat them like they were not given.

@greg-rychlewski
Copy link
Member

greg-rychlewski commented Dec 4, 2024

Actually sorry I had a misunderstanding. I thought empty_values erased the change so that if the value didn't previously exist it saves to the default. But it makes it an explicit change to the default.

Then I think treating it like nil is a lot more intuitive. But still wondering if the current array behaviour is inconsistent with that.

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

2 participants