Migrations: Adding a required property on owned type makes existing data to appear deleted from EF Core

[vzarskus]

Description and example

When getting an entity from the database that owns another entity, EF Core returns the whole owned entity as null if one of it's properties has null value in the database.

I have created a small project that reproduces the issue: https://github.com/vzarskus/EFCorePossibleBug. It mimics the real world scenario that me and me colleagues encountered when working with EF Core.

Here is a quick step by step example of how one might encounter this issue:

1. Create an owned entity (do not make it required as the business case does not require it).
2. Set values to its properties.
3. Later decide to expand the owned entity by adding a few more properties to it (do not make them required as the business case does not require it).
4. Run the migration and update the database. The new properties should have null values in the database columns that represent them.
5. Run some code that gets the entity from the database and tries to manipulate the owned entity.
6. Realize that the whole owned entity is returned as null, even though only some of it's properties have null values in the database.

Provider and version information

EF Core version: 5.0.8
Database provider: Microsoft.EntityFrameworkCore.SqlServer, however Microsoft.EntityFrameworkCore.Sqlite is used in the example that reproduces the issue, so the problem is probably not related to the database provider.
Target framework: .NET 5.0
Operating system: Windows 10
IDE: Visual Studio Code 1.58

[smitpatel]

Duplicate of #23564

[vzarskus]

In the issue that you are refering to the second rule of retrieving optional owned entity states:
"If first condition fails. we look for non-pk optional properties. if any of them have value we get data from server and materialize instance. At least one non-null value would indicate that instance exists."
However, my example shows the opposite - that if at least one of the properties do not have value, the instance is not materialized.

[smitpatel]

Refer the first rule

Find non-pk required properties, if all of them have value then we get data from server and materialize instance. We need value for all required properties else creating instance would throw.

Both the properties - Score and DeliveryTypeScore are required properties in EF Core metadata. Not all of them have value since DeliveryTypeScore has null value. We cannot initialize an instance of owned type because that means assigning null to DeliveryTypeScore which is non-nullable enum type. So from the perspective of query this is working as expected.

From migrations perspective, I am not sure if this should be allowed. Effectively adding a required property in owned type deletes the owned entity. cc: @bricelam

[bricelam]

So the issue is that the new required columns contain NULL? I wonder if something #2595 would improve the Migrations experience by ensuring that a value is provided for already-existing owned entities.

[smitpatel]

Yes, owned type is adding a required property and we generate migration which puts null values for existing columns which should be non-null.

[vzarskus]

Thank you for the answer.

I am not sure if I am missing something, but do I understand correctly that by convention, if the property type is non-nullable (in value types), the generated migration should set the columns as non-nullable as well?
When I am generating migrations for the owned entity in the example, the migration sets columns to nullable.

Or is it that if the property pointing to the owned entity type is no set as required, the owned entity's properties need to be explicitly set to required in order for them to be mapped to the required db columns, but inside EF Core metadata they are still treated the same as per convention?

[smitpatel]

There are 2 different things at play here (probably which causes the bug).

• Properties in owned type can be required/optional in C# model but they would be nullable in database (unless owned type is marked as required dependent) because a null value indicates that owned type associated with owner is null. Hence what you are seeing from metadata perspective is correct.
• Required properties (in c# model) on owned type must have value in database for those columns when owned type exists. That means the default value for a required column which is being added later gets complicated
  • If owned type doesn't exist then default value can/should be null.
  • If owned type does exist then default value must be non-null.

Currently we don't have a way to set default value conditionally like that hence currently it sets just null value which makes existing owned type to get "marked" as no longer exist.

[bricelam]

Note, you can manually add a step to the migration to supply default values for existing owned entities:

migrationBuilder.Sql(
@"
    UPDATE Offers
    SET Score_DeliveryTypeScore = 0.0
    WHERE Score_Score IS NOT NULL;
");

[ajcvickers]

Note from triage: putting this on the backlog to consider doing something better in generated migrations.

[IndigoHealth]

do I understand correctly that by convention, if the property type is non-nullable (in value types), the generated migration should set the columns as non-nullable as well? When I am generating migrations for the owned entity in the example, the migration sets columns to nullable.

I'm trying to understand what's really happening here and what I can do to work around it. I added a Boolean property to an owned entity, and the migration creates it as a nullable column. So now, when I retrieve existing data, the owned entity is null, apparently because EF can't assign the null value to the Boolean property.

1. Is there some reason why this isn't a serious bug in the way migrations are scaffolded? In other words, is there some condition in which it might be correct to create a nullable column for a Boolean property?
2. Can I solve the problem in my code by editing the scaffolded migration to simply create the column as non-nullable? Or, even better, is there some fluent configuration I could add to force EF to correctly understand that the Boolean property should be mapped to a non-nullable column?

[IndigoHealth]

So, there are apparently several situations when required columns are created as nullable. This discussion focuses on an "owned entities" case, where "table splitting" causes required columns to be created as nullable. Another scenario involves TPH inheritance (that might just be another "table splitting" scenario - I'm not expert enough to know). From a user perspective, it would be really helpful if we got a yellow highlighted warning in Visual Studio whenever a migration is created that includes mapping of a non-nullable C# type to a nullable column. If there is some reason why code can't (or shouldn't) be scaffolded to insert default (non-null) values in existing data, at least the warning would let us users know that we need to add that code manually.

[ajcvickers]

@sbsw This issue is specifically about optional dependents sharing a table with the principal where the presence of a non-null value in a column indicates that the dependent exists. It's not about the column being null, but about the impact this has on existing data. This does not apply to other cases, such as TPH columns.