Owned Types: SaveChanges fails on Relational but succeeds for In-memory

[anpete]

Given the following:

public class OwnedAddress
{
    public OwnedCountry Country { get; set; }
}

public class OwnedCountry
{
    public string Name { get; set; }
}

public class OwnedPerson
{
    public int Id { get; set; }
    public OwnedAddress PersonAddress { get; set; }
}

public class Branch : OwnedPerson
{
    public OwnedAddress BranchAddress { get; set; }
}

public class LeafA : Branch
{
    public OwnedAddress LeafAAddress { get; set; }
}

public class LeafB : OwnedPerson
{
    public OwnedAddress LeafBAddress { get; set; }
}

protected virtual void OnModelCreating(ModelBuilder modelBuilder)
{
    modelBuilder.Entity<OwnedPerson>().OwnsOne(p => p.PersonAddress).OwnsOne(a => a.Country);
    modelBuilder.Entity<Branch>().OwnsOne(p => p.BranchAddress).OwnsOne(a => a.Country);
    modelBuilder.Entity<LeafA>().OwnsOne(p => p.LeafAAddress).OwnsOne(a => a.Country);
    modelBuilder.Entity<LeafB>().OwnsOne(p => p.LeafBAddress).OwnsOne(a => a.Country);
}

protected static void AddTestData(DbContext context)
{
    context.Set<OwnedPerson>().AddRange(
        new OwnedPerson
        {
            PersonAddress = new OwnedAddress
            {
                Country = new OwnedCountry { Name = "USA" }
            }
        },
        new Branch
        {
            PersonAddress = new OwnedAddress
            {
                Country = new OwnedCountry { Name = "USA" }
            },
            BranchAddress = new OwnedAddress
            {
                Country = new OwnedCountry { Name = "Canada" }
            }
        },
        new LeafA
        {
            PersonAddress = new OwnedAddress
            {
                Country = new OwnedCountry { Name = "USA" }
            },
            BranchAddress = new OwnedAddress
            {
                Country = new OwnedCountry { Name = "Canada" }
            },
            LeafAAddress = new OwnedAddress
            {
                Country = new OwnedCountry { Name = "Mexico" }
            }
        },
        new LeafB
        {
            PersonAddress = new OwnedAddress
            {
                Country = new OwnedCountry { Name = "USA" }
            },
            LeafBAddress = new OwnedAddress
            {
                Country = new OwnedCountry { Name = "Panama" }
            }
        });

    context.SaveChanges();
}

The SaveChanges call throws on relational:

---- System.InvalidOperationException : There are '9' entity types sharing the table 'OwnedPerson', but only '3' entries with the same key values have been marked as 'Added'. The missing entities should be assignable to {'Branch.BranchAddress#OwnedAddress', 'LeafA.LeafAAddress#OwnedAddress', 'LeafB.LeafBAddress#OwnedAddress', 'Branch.BranchAddress#OwnedAddress.Country#OwnedCountry', 'LeafA.LeafAAddress#OwnedAddress.Country#OwnedCountry', 'LeafB.LeafBAddress#OwnedAddress.Country#OwnedCountry'}. Consider using 'DbContextOptionsBuilder.EnableSensitiveDataLogging' to see the key values.
---- The following constructor parameters did not have matching fixture data: OwnedQuerySqlServerFixture fixture
	
	----- Inner Stack Trace #1 (System.InvalidOperationException) -----
	C:\dev\github\aspnet\EntityFramework\src\EFCore.Relational\Update\Internal\ModificationCommandIdentityMap.cs(129,0): at Microsoft.EntityFrameworkCore.Update.Internal.ModificationCommandIdentityMap.Validate(Boolean sensitiveLoggingEnabled)
	C:\dev\github\aspnet\EntityFramework\src\EFCore.Relational\Update\Internal\CommandBatchPreparer.cs(140,0): at Microsoft.EntityFrameworkCore.Update.Internal.CommandBatchPreparer.CreateModificationCommands(IReadOnlyList`1 entries, Func`1 generateParameterName)
	C:\dev\github\aspnet\EntityFramework\src\EFCore.Relational\Update\Internal\CommandBatchPreparer.cs(67,0): at Microsoft.EntityFrameworkCore.Update.Internal.CommandBatchPreparer.<BatchCommands>d__8.MoveNext()
	C:\dev\github\aspnet\EntityFramework\src\EFCore.Relational\Update\Internal\BatchExecutor.cs(71,0): at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.Execute(Tuple`2 parameters)
	C:\dev\github\aspnet\EntityFramework\src\EFCore\Extensions\ExecutionStrategyExtensions.cs(331,0): at Microsoft.EntityFrameworkCore.ExecutionStrategyExtensions.<>c__DisplayClass12_0`2.<Execute>b__0(DbContext c, TState s)
	C:\dev\github\aspnet\EntityFramework\src\EFCore.SqlServer\Storage\Internal\SqlServerExecutionStrategy.cs(39,0): at Microsoft.EntityFrameworkCore.Storage.Internal.SqlServerExecutionStrategy.Execute[TState,TResult](TState state, Func`3 operation, Func`3 verifySucceeded)
	C:\dev\github\aspnet\EntityFramework\src\EFCore\Extensions\ExecutionStrategyExtensions.cs(329,0): at Microsoft.EntityFrameworkCore.ExecutionStrategyExtensions.Execute[TState,TResult](IExecutionStrategy strategy, Func`2 operation, Func`2 verifySucceeded, TState state)
	C:\dev\github\aspnet\EntityFramework\src\EFCore\Extensions\ExecutionStrategyExtensions.cs(279,0): at Microsoft.EntityFrameworkCore.ExecutionStrategyExtensions.Execute[TState,TResult](IExecutionStrategy strategy, TState state, Func`2 operation)
	C:\dev\github\aspnet\EntityFramework\src\EFCore.Relational\Update\Internal\BatchExecutor.cs(49,0): at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.Execute(IEnumerable`1 commandBatches, IRelationalConnection connection)
	C:\dev\github\aspnet\EntityFramework\src\EFCore.Relational\Storage\RelationalDatabase.cs(51,0): at Microsoft.EntityFrameworkCore.Storage.RelationalDatabase.SaveChanges(IReadOnlyList`1 entries)
	C:\dev\github\aspnet\EntityFramework\src\EFCore\ChangeTracking\Internal\StateManager.cs(787,0): at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(IReadOnlyList`1 entriesToSave)
	C:\dev\github\aspnet\EntityFramework\src\EFCore\ChangeTracking\Internal\StateManager.cs(712,0): at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(Boolean acceptAllChangesOnSuccess)
	C:\dev\github\aspnet\EntityFramework\src\EFCore\DbContext.cs(355,0): at Microsoft.EntityFrameworkCore.DbContext.SaveChanges(Boolean acceptAllChangesOnSuccess)
	C:\dev\github\aspnet\EntityFramework\src\EFCore\DbContext.cs(323,0): at Microsoft.EntityFrameworkCore.DbContext.SaveChanges()

But, In-memory is able to persist the data correctly.

[julielerman]

oh I think this is what I've been beating my head on today! InMemory was ok but integration tests against SQL Server db fail

Using preview2-final and trying to add an object (samurai) that has a SecrectIdentity property that is valueobject defined in fluent API as OwnsOne. Error is almost the same as the error in this issue:

There are '2' entity types sharing the table 'Samurais', but only '1' entries
with the same key values 'Id:-2147482647' have been marked as 'Added'. The missing entities should be assignable t
o {'Samurai.SecretIdentity#Identity'}

Is that what I'm hitting? The Owned type isn't an entity so there should only be one entity.
I wanted to avoid nightly builds for my demos at a conference coming up in a few days so will decide if I want to use nightlies or not show off this feature. Hmmmm..... :)

Here's my stack trace ...

Stack Trace:

   at Microsoft.EntityFrameworkCore.Update.Internal.ModificationCommandIdentityMap.Validate(Boolean sensitiveLoggingEnabled)
   at Microsoft.EntityFrameworkCore.Update.Internal.CommandBatchPreparer.CreateModificationCommands(IReadOnlyList`
1 entries, Func`1 generateParameterName)
   at Microsoft.EntityFrameworkCore.Update.Internal.CommandBatchPreparer.<BatchCommands>d__8.MoveNext()
   at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.Execute(Tuple`2 parameters)
   at Microsoft.EntityFrameworkCore.Storage.Internal.SqlServerExecutionStrategy.Execute[TState,TResult](TState sta
te, Func`3 operation, Func`3 verifySucceeded)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(IReadOnlyList`1 entriesToSave
)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(Boolean acceptAllChangesOnSuc
cess)
   at Microsoft.EntityFrameworkCore.DbContext.SaveChanges(Boolean acceptAllChangesOnSuccess)
   at SamuraiApp.Data.SamuraiContext.SaveChanges() in 

[julielerman]

updated packages to preview3-* and still having this problem. Now I'm wondering if it's related to #8986 which isn't merged yet? If not, I can create a new issue.

[ajcvickers]

@julielerman Could you file an issue with a full listing or project that reproduces what you are seeing?

[julielerman]

will do! May have to wait until end of week (conference) I'll also recheck after 8986 is merged.

[AndriySvyryd]

@julielerman Owned entities are still entities. And owned entities are not optional when using table splitting (the default)

[julielerman]

Sorry, I thought that without an ID, it isn't an entity. (DDD & old-EF complex type assumptions)
However, this happens when I create the owned object. Will be better when I provide a repo. Just out of time right now having spent a lot of time on this over the weekend.

[julielerman]

I just retested with preview3-* and rtm-* (currently rtm-26180) and still getting this problem. The owned entity is not null as mentioned earlier. I will open a separate repo and link to this for ref. Just have to create a trimmed down, focused solution.

[julielerman]

@AndriySvyryd re your explanation that "owned entities are not optional when using table splitting (the default)" What alternatives are there (non default)? I do expect the values of the value object to be stored in the same table as the entity that uses that object as a property so not sure how else to do that without table splitting. The fact that the owned entity is not optional creates some other types of problems for how I would design my domain model following DDD guidance. For one there's no chance to have null objects. Even if I default the property to having each of it's values null, I would have to do some unconventional logic be able to test for a null object (e.g. test that every property of the value object is null and that creates more complexities with value types like int).

Am I missing something or is this just the way it is right now? Is there any plan to change this going forward? Shall I create an issue that you can mark as enhancement?

[AndriySvyryd]

@julielerman The fundamental problem with optional dependents using table splitting is differentiating between a non-existent dependent and one with all non-PK properties having default values.
Suppose we have these classes:

public class Customer
{
    public int Id { get; set; }
    public Address Address { get; set; }
}
public class Address
{
    public int Id { get; set; }
    public string Street { get; set; }
}

And they are mapped to the same table with columns Id, Street.
If we query a row with values 1, null should we create an Address with Street set to null or leave Customer.Address as null?
We are going to investigate this in #9005

For now any dependents using table splitting are required. To make a navigation to an owned type optional you would have to map it to a different table.

[julielerman]

I hadn't discovered #9005. That is most excellent! I totally grok the challenges you have! Now I can use my little workaround but also share with folks know this will get easier! Thanks @AndriySvyryd ! :)