Cosmos: Stop generating discriminator clause when not needed (dotnet#…

…34145) Closes dotnet#20268
ranma42 · Jul 4, 2024 · 20edb63 · 20edb63
1 parent ec42520
commit 20edb63
Show file tree

Hide file tree

Showing 20 changed files with 334 additions and 385 deletions.
diff --git a/src/EFCore.Cosmos/Infrastructure/Internal/CosmosModelValidator.cs b/src/EFCore.Cosmos/Infrastructure/Internal/CosmosModelValidator.cs
@@ -110,6 +110,8 @@ protected virtual void ValidateSharedContainerCompatibility(
         int? defaultTtl = null;
         ThroughputProperties? throughput = null;
         IEntityType? firstEntityType = null;
+        bool? isDiscriminatorMappingComplete = null;
+
         foreach (var entityType in mappedTypes)
         {
             Check.DebugAssert(entityType.IsDocumentRoot(), "Only document roots expected here.");
@@ -177,6 +179,19 @@ protected virtual void ValidateSharedContainerCompatibility(
                 }
 
                 discriminatorValues[discriminatorValue] = entityType;
+
+                var currentIsDiscriminatorMappingComplete = entityType.GetIsDiscriminatorMappingComplete();
+                if (isDiscriminatorMappingComplete == null)
+                {
+                    isDiscriminatorMappingComplete = currentIsDiscriminatorMappingComplete;
+                }
+                else if (currentIsDiscriminatorMappingComplete != isDiscriminatorMappingComplete)
+                {
+                    throw new InvalidOperationException(
+                        CosmosStrings.IsDiscriminatorMappingCompleteMismatch(
+                            isDiscriminatorMappingComplete, firstEntityType.DisplayName(), entityType.DisplayName(),
+                            currentIsDiscriminatorMappingComplete, container));
+                }
             }
 
             var currentAnalyticalTtl = entityType.GetAnalyticalStoreTimeToLive();

diff --git a/src/EFCore.Cosmos/Properties/CosmosStrings.Designer.cs b/src/EFCore.Cosmos/Properties/CosmosStrings.Designer.cs
diff --git a/src/EFCore.Cosmos/Properties/CosmosStrings.resx b/src/EFCore.Cosmos/Properties/CosmosStrings.resx
@@ -165,6 +165,9 @@
   <data name="InvalidResourceId" xml:space="preserve">
     <value>Unable to generate a valid 'id' value to execute a 'ReadItem' query. This usually happens when the value provided for one of the properties is 'null' or an empty string. Please supply a value that's not 'null' or an empty string.</value>
   </data>
+  <data name="IsDiscriminatorMappingCompleteMismatch" xml:space="preserve">
+    <value>The IsDiscriminatorMappingComplete setting was configured to '{isDiscriminatorMappingComplete1}' on '{entityType1}', but on '{entityType2}' it was configured to '{isDiscriminatorMappingComplete2}'. All entity types mapped to the same container '{container}' must be configured with the same IsDiscriminatorMappingComplete value.</value>
+  </data>
   <data name="JsonPropertyCollision" xml:space="preserve">
     <value>Both properties '{property1}' and '{property2}' on entity type '{entityType}' are mapped to '{storeName}'. Map one of the properties to a different JSON property.</value>
   </data>

diff --git a/src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs b/src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs
@@ -234,6 +234,7 @@ public override Expression Translate(Expression expression)
                                 (property, parameter) => (property, parameter))
                             .ToDictionary(tuple => tuple.property, tuple => tuple.parameter);
 
+                        // TODO: Reimplement ReadItem properly: #34157
                         _readItemInfo = new ReadItemInfo(entityType, propertyParameterList, clrType);
                     }
                 }
@@ -427,38 +428,34 @@ protected override ShapedQueryExpression CreateShapedQueryExpression(IEntityType
 
         // Add discriminator predicate
         var concreteEntityTypes = entityType.GetConcreteDerivedTypesInclusive().ToList();
-        if (concreteEntityTypes.Count == 1)
+        if (concreteEntityTypes is [var singleEntityType]
+            && singleEntityType.GetIsDiscriminatorMappingComplete()
+            && entityType.GetContainer() is var container
+            && !entityType.Model.GetEntityTypes().Any(e => e.GetContainer() == container && e != singleEntityType))
         {
-            var concreteEntityType = concreteEntityTypes[0];
-            var discriminatorProperty = concreteEntityType.FindDiscriminatorProperty();
-            if (discriminatorProperty != null)
+            // There's a single entity type mapped to the container and the discriminator mapping is complete; we can skip the
+            // discriminator predicate.
+        }
+        else
+        {
+            var discriminatorProperty = concreteEntityTypes[0].FindDiscriminatorProperty();
+            Check.DebugAssert(
+                discriminatorProperty is not null || concreteEntityTypes.Count == 1,
+                "Missing discriminator property in hierarchy");
+            if (discriminatorProperty is not null)
             {
                 var discriminatorColumn = ((EntityProjectionExpression)selectExpression.GetMappedProjection(new ProjectionMember()))
                     .BindProperty(discriminatorProperty, clientEval: false);
 
                 var success = TryApplyPredicate(
                     selectExpression,
-                    _sqlExpressionFactory.Equal(
+                    _sqlExpressionFactory.In(
                         (SqlExpression)discriminatorColumn,
-                        _sqlExpressionFactory.Constant(concreteEntityType.GetDiscriminatorValue(), discriminatorColumn.Type)));
+                        concreteEntityTypes.Select(et => _sqlExpressionFactory.Constant(et.GetDiscriminatorValue(), discriminatorColumn.Type))
+                        .ToArray()));
                 Check.DebugAssert(success, "Couldn't apply predicate when creating a new ShapedQueryExpression");
             }
         }
-        else
-        {
-            var discriminatorProperty = concreteEntityTypes[0].FindDiscriminatorProperty();
-            Check.DebugAssert(discriminatorProperty is not null, "Missing discriminator property in hierarchy");
-            var discriminatorColumn = ((EntityProjectionExpression)selectExpression.GetMappedProjection(new ProjectionMember()))
-                .BindProperty(discriminatorProperty, clientEval: false);
-
-            var success = TryApplyPredicate(
-                selectExpression,
-                _sqlExpressionFactory.In(
-                    (SqlExpression)discriminatorColumn,
-                    concreteEntityTypes.Select(et => _sqlExpressionFactory.Constant(et.GetDiscriminatorValue(), discriminatorColumn.Type))
-                        .ToArray()));
-            Check.DebugAssert(success, "Couldn't apply predicate when creating a new ShapedQueryExpression");
-        }
 
         return CreateShapedQueryExpression(entityType, selectExpression);
     }
@@ -834,7 +831,7 @@ protected override ShapedQueryExpression TranslateCast(ShapedQueryExpression sou
             return null;
         }
 
-        if (select is { Predicate: null, Orderings: [] })
+        if (select is { Orderings: [], Predicate: null, ReadItemInfo: null })
         {
             _queryCompilationContext.Logger.FirstWithoutOrderByAndFilterWarning();
         }

diff --git a/src/EFCore.Cosmos/Query/Internal/Expressions/SelectExpression.cs b/src/EFCore.Cosmos/Query/Internal/Expressions/SelectExpression.cs
@@ -29,6 +29,7 @@ public sealed class SelectExpression : Expression, IPrintableExpression
     ///     any release. You should only use it directly in your code with extreme caution and knowing that
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
+    // TODO: Reimplement ReadItem properly: #34157
     public ReadItemInfo? ReadItemInfo { get; init; }
 
     /// <summary>