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

Allow value types as optional properties, using the sentinel value to detect when they're set #34943

Open
roji opened this issue Oct 21, 2024 · 3 comments

Comments

@roji
Copy link
Member

roji commented Oct 21, 2024

We currently disallow value type properties to be marked as optional:

Unhandled exception. System.InvalidOperationException: The property 'Blog.Foo' cannot be marked as nullable/optional because the type of the property is 'int' which is not a nullable type. Any property can be marked as non-nullable/required, but only properties of nullable types can be marked as nullable/optional.
   at Microsoft.EntityFrameworkCore.Metadata.Internal.Property.SetIsNullable(Nullable`1 nullable, ConfigurationSource configurationSource)
   at Microsoft.EntityFrameworkCore.Metadata.Internal.InternalPropertyBuilder.IsRequired(Nullable`1 required, ConfigurationSource configurationSource)
   at Microsoft.EntityFrameworkCore.Metadata.Builders.PropertyBuilder.IsRequired(Boolean required)
   at Microsoft.EntityFrameworkCore.Metadata.Builders.PropertyBuilder`1.IsRequired(Boolean required)
   at BlogContext.OnModelCreating(ModelBuilder modelBuilder) in /Users/roji/projects/test/Program.cs:line 28

On the other hand, for the purpose of value generation, we have the concept of a sentinel (CLR default by default) which informs us whether a property is set or not (if unset and value generation is configured, we don't send the value to the database). We've also been discussing how to handle missing properties in documents (JSON), and the ability to read such documents back (especially for scenarios where a JSON "schema" evolves to include a new property, at which point all existing documents are missing it). It seems we could use the same principles and materialize a database null to the property default/sentinel, allowing optional value type properties.

/cc @AndriySvyryd

Raised by @burikella in #34940 for the Option<T> type.

@AndriySvyryd
Copy link
Member

Related to #24685, should probably be fixed together
Related to #26981 and #21006, we should have a consistent design across providers

@ajcvickers
Copy link
Member

@roji What happens when the entity is saved? Are the sentinel values written to the database? Or will properties set to the sentinel now be excluded when SaveChanges is called? Also, we only use the sentinel currently for value-generated properties. This means the full value space is always available for a property unless you need to do value generation.

In general, I'm very much in favor of supporting missing properties, but I'm not sure that this is going to be sufficient to do that.

@roji
Copy link
Member Author

roji commented Oct 30, 2024

@ajcvickers we definitely haven't thought this through yet, but I think the idea here is to support optional properties, i.e. write a null to the database when the property has the sentinel type (or leave it the default, am not sure).

Yeah, we need to take a more general, holistic view at all of these related issues.

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

No branches or pull requests

3 participants