dotnet / efcore

EF Core is a modern object-database mapper for .NET. It supports LINQ queries, change tracking, updates, and schema migrations.
https://docs.microsoft.com/ef/
MIT License
13.66k stars 3.16k forks source link

Schema not set when configuring two Owned Types with table splitting #12954

Closed raffaeler closed 1 year ago

raffaeler commented 6 years ago

It's a while I am facing the following error while trying to configure owned types. Apparently, when configuring Owned Types, they sometimes misses to add the annotation "Relational:Schema".

Exception:

System.InvalidOperationException: Cannot use table 'dbo.Test' for entity type 'Test.ReferenceComplexTypeProp#ReferenceComplexType' since it is being used for entity type 'Test.ComplexTypeProp#ComplexType' and there is no relationship between their primary keys..
The exception happens in the RelationalModelValidator, it happens during the context creation, therefore there is still no "user-code" involved.

Stack trace:

   at Microsoft.EntityFrameworkCore.Infrastructure.RelationalModelValidator.ValidateSharedTableCompatibility(IReadOnlyList`1 mappedTypes, String tableName)
   at Microsoft.EntityFrameworkCore.Internal.SqlServerModelValidator.ValidateSharedTableCompatibility(IReadOnlyList`1 mappedTypes, String tableName)
   at Microsoft.EntityFrameworkCore.Infrastructure.RelationalModelValidator.ValidateSharedTableCompatibility(IModel model)
   at Microsoft.EntityFrameworkCore.Infrastructure.RelationalModelValidator.Validate(IModel model)
   at Microsoft.EntityFrameworkCore.Internal.SqlServerModelValidator.Validate(IModel model)
   at Microsoft.EntityFrameworkCore.Infrastructure.ModelSource.CreateModel(DbContext context, IConventionSetBuilder conventionSetBuilder, IModelValidator validator)
   at Microsoft.EntityFrameworkCore.Infrastructure.ModelSource.<>c__DisplayClass5_0.<GetModel>b__1()

Configuration: An entity type has two properties configured as Owned Types. The CLR type for the owned types is the same.

Apparently I solved the problem with the annotation but unfortunately, this solves just the smaller repro but not the main project.

Questions:

Further technical details

EF Core version: 2.1.1-rtm-30846 Database Provider: Microsoft.EntityFrameworkCore.SqlServer Operating system: Windows 10 IDE: Visual Studio 2017 15.6

Thanks

ajcvickers commented 6 years ago

@AndriySvyryd The only issue I can find that looks like a possible duplicate is #10911, but that is marked as fixed in 2.1-preview2.

AndriySvyryd commented 6 years ago

@raffaeler Could you show your model configuration code? Do you specify the schema anywhere without the workaround?

raffaeler commented 6 years ago

Hi @AndriySvyryd, the model is described with a very verbose code using reflection and expressions. This happens because the types are not known from the code building the model. The schema is always specified in the ToTable methods. In the specific case the schema string is "dbo" but in ohter cases it may be different.

The schema problem looks like a bug that misses to inherit the schema settings specified in the owning entity (I verified with the debugger that the schema is always set using ToTable).

Even if the error message is the same, I isolated the current problem by comparing again the DebugView output strings. The obfuscated relevant part of the working test is the following one:

  EntityType: FE_EFCoreTest Base: EWF
    Navigations: 
      ComplexTypeProp1 (<ComplexTypeProp1>k__BackingField, ComplexType) ToDependent ComplexType 1 -1 2 -1 -1
      ReferenceComplexTypeProp (<ReferenceComplexTypeProp>k__BackingField, ReferenceComplexType) ToDependent ReferenceComplexType 2 -1 3 -1 -1
    Annotations: 
      Relational:DiscriminatorValue: FE_EFCoreTest
      Relational:Schema: dbo
      Relational:TableName: vEWF_S_1949429316
      RelationshipDiscoveryConvention:NavigationCandidates: System.Collections.Immutable.ImmutableSortedDictionary`2[System.Reflection.PropertyInfo,System.Type]
  EntityType: ComplexType
    Properties: 
      Id.Shadow (no field, Guid) Shadow Required PK FK AfterSave:Throw 0 0 0 0 0
      Prop1 (_Prop1, string) 1 1 -1 -1 -1
        Annotations: 
          Relational:ColumnName: Prop1_ComplexTypePr_569187297
      Prop2 (_Prop2, Nullable<int>) 2 2 -1 -1 -1
        Annotations: 
          Relational:ColumnName: Prop2_ComplexTypeP_1295699952
    Keys: 
      Id.Shadow PK
    Foreign keys: 
      ComplexType {'Id.Shadow'} -> FE_EFCoreTest {'Id'} Unique Ownership ToDependent: ComplexTypeProp1
    Annotations: 
      Relational:Schema: dbo
      Relational:TableName: vEWF_S_1949429316
      RelationshipDiscoveryConvention:NavigationCandidates: System.Collections.Immutable.ImmutableSortedDictionary`2[System.Reflection.PropertyInfo,System.Type]
  EntityType: ReferenceComplexType
    Properties: 
      Id.Shadow (no field, Guid) Shadow Required PK FK AfterSave:Throw 0 0 0 0 0
      A (_A, string) 1 1 -1 -1 -1
        Annotations: 
          Relational:ColumnName: A_ReferenceComplex_1931649781
      Ref (_Ref, Nullable<Guid>) 2 2 -1 -1 -1
        Annotations: 
          Relational:ColumnName: Ref_ReferenceCompl_1577351122
    Keys: 
      Id.Shadow PK
    Foreign keys: 
      ReferenceComplexType {'Id.Shadow'} -> FE_EFCoreTest {'Id'} Unique Ownership ToDependent: ReferenceComplexTypeProp
    Annotations: 
      Relational:Schema: dbo
      Relational:TableName: vEWF_S_1949429316
      RelationshipDiscoveryConvention:NavigationCandidates: System.Collections.Immutable.ImmutableSortedDictionary`2[System.Reflection.PropertyInfo,System.Type]

The relevant differences of the DebugView output of the test failing are the following ones: Please note that the differences are just a number. From the source code I understand that the number that is different is the "RelationshipIndex" which does not help me so much.

ComplexTypeProp1 (<ComplexTypeProp1>k__BackingField, ComplexType) ToDependent ComplexType 1 -1 3 -1 -1
ReferenceComplexTypeProp (<ReferenceComplexTypeProp>k__BackingField, ReferenceComplexType) ToDependent ReferenceComplexType 2 -1 4 -1 -1

Notes:

Thanks for your help

raffaeler commented 6 years ago

@AndriySvyryd Here there are some more info I could extract from the code about the way the owned types are defined in the model. The first step is calling ToTable:

builder.ToTable(physicalTableName, "dbo");

The second step is to call OwnsOne. In this specific case I had to create the OwnsOne2 extension method accepting a PropertyInfo instead of the property name. The Relational:Schema entry was the 'missing one'.

var referenceOwnershipBuilder = builder
    .OwnsOne2(valueType, property)
    .HasEntityTypeAnnotation("Relational:Schema", Entity.PhysicalSchema);

For each property of the owned type I then call:

var propertyBuilder = referenceOwnershipBuilder.AddProperty2<TEntity>(
    valueType, propertyInfo);
propertyBuilder.HasColumnName(columnName);

I then had to add a shadow property with an arbitrary fixed name to avoid the case where a real property of the value type called "Id" (if any) collides with the one created from the conventions.

var propertyBuilder = referenceOwnershipBuilder
    .Property(typeof(Guid), $"Id.Shadow");
propertyBuilder.Metadata.IsPrimaryKey();
referenceOwnershipBuilder.HasForeignKey($"Id.Shadow");

As I said in a previous message, Relational:Schema is the one resolved partially the problem. But I still have the same exception and the only difference between a working use-case and a non-working one is the RelationshipIndex number.

Thanks

raffaeler commented 6 years ago

@AndriySvyryd I spent the entire day in debugging EF Core internals. The problem is caused by the owned type not being recognized as "inlined" from EFCore. The solution was to call .ToTable manually on the owned type too.

From the docs, I understood that mapping to the same table as the owning entity was the default, but apparently the table name and the schema are not inherited and do require a manual mapping. Since I now call ToTable taking both the table name and the schema, I could remove HasEntityTypeAnnotation entry.

Now I solved my problem but I leave to you the decision whether to use this issue to open a bug to fix the codebase, the docs or closing this issue and open another one.

AndriySvyryd commented 6 years ago

@raffaeler Thanks, I'll investigate.

raffaeler commented 6 years ago

@AndriySvyryd Of course, this just fixes one of the problems, but there are more. The context is the same of this thread. Just let me know if this thread is appropriate of I should one a new one.

Given a successful test, if I just add in the context constructor

this.ChangeTracker.QueryTrackingBehavior = QueryTrackingBehavior.NoTracking;

I get an exception on the model:

Unable to determine the relationship represented by navigation property 'Y.Parent' of type 'X'. Either manually configure the relationship, or ignore this property using the '[NotMapped]' attribute or by using 'EntityTypeBuilder.Ignore' in 'OnModelCreating'..

Beyond the way the model is built, I can't understand why the model does not validate anymore (with the above error) as soon as the tracking is disabled. How can these two things be related?

As I said at the beginning, the hardest part is the difficulty in understanding the real problem from the exceptions.

AndriySvyryd commented 6 years ago

It's even harder to understand the problem without seeing your source code 😄 Setting this.ChangeTracker.QueryTrackingBehavior can't possibly influence the model in any way.

raffaeler commented 6 years ago

@AndriySvyryd after many years, it sounds the closed source problem is inverted, LOL :D

Even if I could post the whole code (unfortunately the decision is not mine), it would be an extremely large codebase. I tried to reduce the scope in a small repro, but just reducing a bit the model does not show the problem anymore, it's really hard to understand.

In this latter case, I just comment out the this.ChangeTracker.QueryTrackingBehavior or anything else setting the ChangeTracker behavior, and the test passes.

smitpatel commented 5 years ago

I used following code to reproduce the issue

using System;
using System.Linq;
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Logging;

namespace EFSampleApp
{
    public class Program
    {
        public static void Main(string[] args)
        {
            using (var db = new MyContext())
            {
                // Recreate database
                db.Database.EnsureDeleted();
                db.Database.EnsureCreated();

                // Seed database

                db.SaveChanges();
            }

            using (var db = new MyContext())
            {
                // Run queries
                var query = db.Blogs.ToList();
            }

            Console.WriteLine("Program finished.");
        }
    }

    public class MyContext : DbContext
    {
        private static ILoggerFactory LoggerFactory => new LoggerFactory().AddConsole(LogLevel.Trace);

        // Declare DBSets
        public DbSet<Blog> Blogs { get; set; }

        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            // Select 1 provider
            optionsBuilder
                .UseSqlServer(@"Server=(localdb)\mssqllocaldb;Database=_ModelApp;Trusted_Connection=True;Connect Timeout=5;ConnectRetryCount=0")
                //.UseSqlite("filename=_modelApp.db")
                //.UseInMemoryDatabase(databaseName: "_modelApp")
                .EnableSensitiveDataLogging()
                .UseLoggerFactory(LoggerFactory);
        }

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            // Configure model
            modelBuilder.Entity<Blog>().ToTable("Blogging", "Custom");
            modelBuilder.Entity<Blog>().OwnsOne(e => e.HomeAddress);
            modelBuilder.Entity<Blog>().OwnsOne(e => e.WorkAddress);
        }
    }

    public class Blog
    {
        public int Id { get; set; }
        public Address HomeAddress { get; set; }
        public Address WorkAddress { get; set; }
    }

    public class Address
    {
        public string City { get; set; }
    }
}

Based on whatever said in the comments above, I tried to recreate similar model but it is working for me successfully in version 2.2.0-preview2-35157 (lastest on NuGet). I also verified that we set schema appropriately (not just the table) when configuring shared table. Beyond that, I believe, it is not possible for us to investigate without actual repro code. I am closing this issue. Feel free to re-open if you can provide us a self-contained repro code which demonstrate the issue you are seeing.

raffaeler commented 5 years ago

@smitpatel At the end the problem was a mix of two issues:

HTH

AndriySvyryd commented 5 years ago

@raffaeler Calling ToTable on the owned types is a workaround for your issue, it shouldn't be needed, that's why it's not documented.

raffaeler commented 5 years ago

@AndriySvyryd ok. But without it I could not make it work. I am not sure whether @smitpatel test cover the case when PhysicalSchema was initially specified by hand to the entity (and thus not propagated to the Owned Type)

smitpatel commented 5 years ago

@raffaeler - If you look at the code I posted, I have configured the owner type to map to different schema using ToTable call and yet it is propagated properly to owned entities. The code in SharedTableConvention also does processing for table and schema both. Hence the assumption that schema is not getting propagated is incorret. At this stage we will need repro to investigate further else there does not seem to be any issue in codebase or documentation.

raffaeler commented 5 years ago

@smitpatel I see but there is a huge difference between my code and yours. I have to manually specify all the mappings because in my case I don't know the entities. Therefore I have to use reflection to map them. Beyond the fact I created new extension methods to use reflection objects such as PropertyInfo, I manually add every single property without letting the conventions decide.

That said, I tried to add to your code the manual mappings, but I get an internal NullReferenceException:

var entityBuilder = modelBuilder.Entity<Blog>().ToTable("Blogging", "dbo");
var ownershipBuilder1 = modelBuilder.Entity<Blog>().OwnsOne(e => e.HomeAddress);
var ownershipBuilder2 = modelBuilder.Entity<Blog>().OwnsOne(e => e.WorkAddress);
var propertyBuilder = ownershipBuilder1.Property<Address>("City"); // explodes internally in EF

Let's start from this ... do you see that? Thanks

smitpatel commented 5 years ago

var propertyBuilder = ownershipBuilder1.Property<string>("City") Don't forget that the type of Property should match up. The cause of issue is more likely that you are specifying table manually but not the schema when configuring owned entities.

raffaeler commented 5 years ago

@smitpatel I am now using your example and I did not modify the entities you defined, so string is correct. I also tried this statement:

var propertyBuilder = ownershipBuilder1.Property("City");

but I still get a NullReferenceException. At home I did not configure the source code stepping so I am not understanding why this exception get thrown. (In any case I don't expect EFCore throwing a NullReferenceException).

For clarity this is the OnModelCreating method:

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
    // Configure model
    var entityBuilder = modelBuilder.Entity<Blog>().ToTable("Blogging", "Custom");
    var ownershipBuilder1 = modelBuilder.Entity<Blog>().OwnsOne(e => e.HomeAddress);
    var ownershipBuilder2 = modelBuilder.Entity<Blog>().OwnsOne(e => e.WorkAddress);

    var propertyBuilder = ownershipBuilder1.Property<Address>("City"); // throws NullReferenceException internally
    propertyBuilder.HasColumnName("City");

    //propertyBuilder = ownershipBuilder1.Property(typeof(Guid), $"Id.Shadow");
    //propertyBuilder.Metadata.IsPrimaryKey();
    //ownershipBuilder1.HasForeignKey($"Id.Shadow");
}
ajcvickers commented 5 years ago

Moved to #13519