Query: Allow entities which are table splitting with derived type

DRY code to generate discriminator for a query for materialize & non-materialized case Find identifying relationship chain to root and add discriminator of relavent types Resolves #8973
dotnet · Feb 16, 2018 · 5f0b0c7 · 5f0b0c7
1 parent 92e50ee
commit 5f0b0c7
Show file tree

Hide file tree

Showing 15 changed files with 318 additions and 173 deletions.
diff --git a/src/EFCore.Relational.Specification.Tests/TableSplittingTestBase.cs b/src/EFCore.Relational.Specification.Tests/TableSplittingTestBase.cs
@@ -21,7 +21,7 @@ protected TableSplittingTestBase(ITestOutputHelper testOutputHelper)
             TestOutputHelper = testOutputHelper;
         }
 
-        [Fact(Skip = "#8973")]
+        [Fact]
         public virtual void Can_query_shared()
         {
             using (CreateTestStore(OnModelCreating))
@@ -33,7 +33,7 @@ public virtual void Can_query_shared()
             }
         }
 
-        [Fact(Skip = "#8973")]
+        [Fact(Skip = "Weird Model")]
         public virtual void Can_query_shared_derived()
         {
             using (CreateTestStore(OnModelCreating))
@@ -45,13 +45,13 @@ public virtual void Can_query_shared_derived()
             }
         }
 
-        [Fact(Skip = "#8973")]
+        [Fact]
         public virtual void Can_use_with_redundant_relationships()
         {
             Test_roundtrip(OnModelCreating);
         }
 
-        [Fact(Skip = "#8973")]
+        [Fact]
         public virtual void Can_use_with_chained_relationships()
         {
             Test_roundtrip(
@@ -62,7 +62,7 @@ public virtual void Can_use_with_chained_relationships()
                     });
         }
 
-        [Fact(Skip = "#8973")]
+        [Fact(Skip = "Issue#10971")]
         public virtual void Can_use_with_fanned_relationships()
         {
             Test_roundtrip(
@@ -181,6 +181,8 @@ public virtual void Can_change_principal_instance_non_derived()
 
         protected void AssertSql(params string[] expected)
             => TestSqlLoggerFactory.AssertBaseline(expected);
+        protected void AssertContainsSql(params string[] expected)
+            => TestSqlLoggerFactory.AssertBaseline(expected, assertOrder: false);
 
         protected virtual void OnModelCreating(ModelBuilder modelBuilder)
         {
@@ -207,6 +209,11 @@ protected TestStore CreateTestStore(Action<ModelBuilder> onModelCreating)
 
             TestStore.Initialize(ServiceProvider, CreateContext, c => ((TransportationContext)c).Seed());
 
+            TestSqlLoggerFactory.Clear();
+
+            // To enable logging
+            //TestSqlLoggerFactory.SetTestOutputHelper(TestOutputHelper);
+
             return TestStore;
         }
 

diff --git a/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs b/src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs
@@ -266,30 +266,11 @@ protected virtual void ValidateSharedTableCompatibility(
             }
         }
 
-        private static bool IsIdentifyingPrincipal(IEntityType dependEntityType, IEntityType principalEntityType)
+        private static bool IsIdentifyingPrincipal(IEntityType dependentEntityType, IEntityType principalEntityType)
         {
-            foreach (var foreignKey in dependEntityType.FindForeignKeys(dependEntityType.FindPrimaryKey().Properties))
-            {
-                if (!foreignKey.PrincipalKey.IsPrimaryKey()
-                    || foreignKey.PrincipalEntityType != principalEntityType)
-                {
-                    continue;
-                }
-
-                if (principalEntityType.BaseType != null)
-                {
-                    // #8973
-                    throw new InvalidOperationException(
-                        RelationalStrings.IncompatibleTableDerivedPrincipal(
-                            dependEntityType.Relational().TableName,
-                            dependEntityType.DisplayName(),
-                            principalEntityType.DisplayName(),
-                            principalEntityType.RootType().DisplayName()));
-                }
-                return true;
-            }
-
-            return false;
+            return dependentEntityType.FindForeignKeys(dependentEntityType.FindPrimaryKey().Properties)
+                .Any(fk => fk.PrincipalKey.IsPrimaryKey()
+                    && fk.PrincipalEntityType == principalEntityType);
         }
 
         /// <summary>
@@ -588,7 +569,7 @@ protected virtual void ValidateInheritanceMapping([NotNull] IModel model)
             }
         }
 
-        private void ValidateDiscriminator(IEntityType entityType)
+        private static void ValidateDiscriminator(IEntityType entityType)
         {
             var annotations = entityType.Relational();
             if (annotations.DiscriminatorProperty == null)

diff --git a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs
diff --git a/src/EFCore.Relational/Properties/RelationalStrings.resx b/src/EFCore.Relational/Properties/RelationalStrings.resx
@@ -284,9 +284,6 @@
   <data name="IncompatibleTableNoRelationship" xml:space="preserve">
     <value>Cannot use table '{table}' for entity type '{entityType}' since it is being used for entity type '{otherEntityType}' and there is no relationship between their primary keys.</value>
   </data>
-  <data name="IncompatibleTableDerivedPrincipal" xml:space="preserve">
-    <value>Cannot use table '{table}' for entity type '{dependentType}' since it has a relationship to a derived entity type '{principalType}'. Either point the relationship to the base type '{rootType}' or map '{dependentType}' to a different table.</value>
-  </data>
   <data name="LogKeyHasDefaultValue" xml:space="preserve">
     <value>Property '{property}' on entity type '{entityType}' is part of a primary or alternate key but has a constant default value set. Constant default values are not useful for primary or alternate keys since these properties must always have non-null unqiue values.</value>
     <comment>Warning RelationalEventId.ModelValidationKeyDefaultValueWarning string string</comment>

diff --git a/src/EFCore.Relational/Query/ExpressionVisitors/Internal/IMaterializerFactory.cs b/src/EFCore.Relational/Query/ExpressionVisitors/Internal/IMaterializerFactory.cs
@@ -8,7 +8,6 @@
 using Microsoft.EntityFrameworkCore.Metadata;
 using Microsoft.EntityFrameworkCore.Query.Expressions;
 using Microsoft.EntityFrameworkCore.Storage;
-using Remotion.Linq.Clauses;
 
 namespace Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal
 {
@@ -26,7 +25,6 @@ Expression<Func<ValueBuffer, DbContext, object>> CreateMaterializer(
             [NotNull] IEntityType entityType,
             [NotNull] SelectExpression selectExpression,
             [NotNull] Func<IProperty, SelectExpression, int> projectionAdder,
-            [CanBeNull] IQuerySource querySource,
             out Dictionary<Type, int[]> typeIndexMap);
     }
 }
diff --git a/src/EFCore.Relational/Query/ExpressionVisitors/Internal/MaterializerFactory.cs b/src/EFCore.Relational/Query/ExpressionVisitors/Internal/MaterializerFactory.cs
@@ -13,7 +13,6 @@
 using Microsoft.EntityFrameworkCore.Query.Expressions;
 using Microsoft.EntityFrameworkCore.Storage;
 using Microsoft.EntityFrameworkCore.Utilities;
-using Remotion.Linq.Clauses;
 
 namespace Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal
 {
@@ -45,7 +44,6 @@ public virtual Expression<Func<ValueBuffer, DbContext, object>> CreateMaterializ
             IEntityType entityType,
             SelectExpression selectExpression,
             Func<IProperty, SelectExpression, int> projectionAdder,
-            IQuerySource querySource,
             out Dictionary<Type, int[]> typeIndexMap)
         {
             Check.NotNull(entityType, nameof(entityType));
@@ -59,7 +57,7 @@ public virtual Expression<Func<ValueBuffer, DbContext, object>> CreateMaterializ
             var rootEntityType = concreteEntityTypes[0];
             var indexMap = new int[rootEntityType.PropertyCount()];
             var contextParameter = Expression.Parameter(typeof(DbContext), "context");
-            
+
             foreach (var property in rootEntityType.GetProperties())
             {
                 indexMap[property.GetIndex()] = projectionAdder(property, selectExpression);
@@ -70,35 +68,18 @@ var materializer
                     .CreateMaterializeExpression(
                         rootEntityType, valueBufferParameter, contextParameter, indexMap);
 
-            if (concreteEntityTypes.Count == 1
-                && rootEntityType.RootType() == rootEntityType)
+            if (concreteEntityTypes.Count == 1)
             {
                 return Expression.Lambda<Func<ValueBuffer, DbContext, object>>(
                     materializer, valueBufferParameter, contextParameter);
             }
 
             var discriminatorProperty = rootEntityType.Relational().DiscriminatorProperty;
 
-            var discriminatorColumn
-                = selectExpression.Projection
-                    .Last(c => (c as ColumnExpression)?.Property == discriminatorProperty);
-
             var firstDiscriminatorValue
                 = Expression.Constant(
                     rootEntityType.Relational().DiscriminatorValue,
-                    discriminatorColumn.Type);
-
-            var discriminatorPredicate
-                = Expression.Equal(discriminatorColumn, firstDiscriminatorValue);
-
-            if (concreteEntityTypes.Count == 1)
-            {
-                selectExpression.Predicate
-                    = new DiscriminatorPredicateExpression(discriminatorPredicate, querySource);
-
-                return Expression.Lambda<Func<ValueBuffer, DbContext, object>>(
-                    materializer, valueBufferParameter, contextParameter);
-            }
+                    discriminatorProperty.ClrType);
 
             var discriminatorValueVariable
                 = Expression.Variable(discriminatorProperty.ClrType);
@@ -154,7 +135,7 @@ var blockExpressions
                 var discriminatorValue
                     = Expression.Constant(
                         concreteEntityType.Relational().DiscriminatorValue,
-                        discriminatorColumn.Type);
+                        discriminatorProperty.ClrType);
 
                 materializer
                     = _entityMaterializerSource
@@ -166,16 +147,8 @@ var discriminatorValue
                         Expression.Equal(discriminatorValueVariable, discriminatorValue),
                         Expression.Return(returnLabelTarget, materializer),
                         blockExpressions[1]);
-
-                discriminatorPredicate
-                    = Expression.OrElse(
-                        Expression.Equal(discriminatorColumn, discriminatorValue),
-                        discriminatorPredicate);
             }
 
-            selectExpression.Predicate
-                = new DiscriminatorPredicateExpression(discriminatorPredicate, querySource);
-
             return Expression.Lambda<Func<ValueBuffer, DbContext, object>>(
                 Expression.Block(new[] { discriminatorValueVariable }, blockExpressions),
                 valueBufferParameter,