From 8d28101384dc5e57ed15e545abbf2b2a15809cbb Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Wed, 28 Aug 2024 00:08:14 +0200 Subject: [PATCH] Properly implement support for Cosmos hierarchical partition keys Fixes #34553 --- .../Extensions/CosmosQueryableExtensions.cs | 94 +++++++++--- .../Properties/CosmosStrings.Designer.cs | 8 - .../Properties/CosmosStrings.resx | 3 - ...yableMethodTranslatingExpressionVisitor.cs | 16 +- ...CosmosReadItemAndPartitionKeysExtractor.cs | 140 +++++++++++------- ...osShapedQueryCompilingExpressionVisitor.cs | 29 +--- .../Internal/Expressions/SelectExpression.cs | 2 +- ...mPartitionKeyQueryDiscriminatorInIdTest.cs | 43 ++++-- ...temPartitionKeyQueryInheritanceTestBase.cs | 16 +- ...artitionKeyQueryNoDiscriminatorInIdTest.cs | 43 ++++-- ...titionKeyQueryRootDiscriminatorInIdTest.cs | 43 ++++-- .../Query/ReadItemPartitionKeyQueryTest.cs | 30 +++- .../ReadItemPartitionKeyQueryTestBase.cs | 19 +-- 13 files changed, 311 insertions(+), 175 deletions(-) diff --git a/src/EFCore.Cosmos/Extensions/CosmosQueryableExtensions.cs b/src/EFCore.Cosmos/Extensions/CosmosQueryableExtensions.cs index 53857ed63f8..70bc240dfb8 100644 --- a/src/EFCore.Cosmos/Extensions/CosmosQueryableExtensions.cs +++ b/src/EFCore.Cosmos/Extensions/CosmosQueryableExtensions.cs @@ -18,14 +18,24 @@ namespace Microsoft.EntityFrameworkCore; /// public static class CosmosQueryableExtensions { - internal static readonly MethodInfo WithPartitionKeyMethodInfo + internal static readonly MethodInfo WithPartitionKeyMethodInfo1 + = typeof(CosmosQueryableExtensions).GetTypeInfo() + .GetDeclaredMethods(nameof(WithPartitionKey)) + .Single(mi => mi.GetParameters().Length == 2); + + internal static readonly MethodInfo WithPartitionKeyMethodInfo2 = typeof(CosmosQueryableExtensions).GetTypeInfo() .GetDeclaredMethods(nameof(WithPartitionKey)) .Single(mi => mi.GetParameters().Length == 3); + internal static readonly MethodInfo WithPartitionKeyMethodInfo3 + = typeof(CosmosQueryableExtensions).GetTypeInfo() + .GetDeclaredMethods(nameof(WithPartitionKey)) + .Single(mi => mi.GetParameters().Length == 4); + /// - /// Specify the partition key value for partition used for the query. Required when using - /// a resource token that provides permission based on a partition key for authentication. + /// Specify the partition key for partition used for the query. + /// Required when using a resource token that provides permission based on a partition key for authentication, /// /// /// See Querying data with EF Core, and @@ -33,15 +43,27 @@ internal static readonly MethodInfo WithPartitionKeyMethodInfo /// /// The type of entity being queried. /// The source query. - /// The partition key value. + /// The partition key value. /// A new query with the set partition key. - public static IQueryable WithPartitionKey(this IQueryable source, string partitionKey) + public static IQueryable WithPartitionKey(this IQueryable source, object partitionKeyValue) where TEntity : class - => WithPartitionKey(source, partitionKey, []); + { + Check.NotNull(partitionKeyValue, nameof(partitionKeyValue)); + + return + source.Provider is EntityQueryProvider + ? source.Provider.CreateQuery( + Expression.Call( + instance: null, + method: WithPartitionKeyMethodInfo1.MakeGenericMethod(typeof(TEntity)), + source.Expression, + Expression.Constant(partitionKeyValue, typeof(object)))) + : source; + } /// - /// Specify the partition key for partition used for the query. Required when using - /// a resource token that provides permission based on a partition key for authentication, + /// Specify the partition key for partition used for the query. + /// Required when using a resource token that provides permission based on a partition key for authentication, /// /// /// See Querying data with EF Core, and @@ -49,27 +71,65 @@ public static IQueryable WithPartitionKey(this IQueryable /// The type of entity being queried. /// The source query. - /// The partition key value. - /// Additional values for hierarchical partitions. + /// The first value in a hierarchical partition key. + /// The second value in a hierarchical partition key. /// A new query with the set partition key. public static IQueryable WithPartitionKey( this IQueryable source, - object partitionKeyValue, - params object[] additionalPartitionKeyValues) + object partitionKeyValue1, + object partitionKeyValue2) where TEntity : class { - Check.NotNull(partitionKeyValue, nameof(partitionKeyValue)); - Check.HasNoNulls(additionalPartitionKeyValues, nameof(additionalPartitionKeyValues)); + Check.NotNull(partitionKeyValue1, nameof(partitionKeyValue1)); + Check.NotNull(partitionKeyValue2, nameof(partitionKeyValue2)); + + return + source.Provider is EntityQueryProvider + ? source.Provider.CreateQuery( + Expression.Call( + instance: null, + method: WithPartitionKeyMethodInfo2.MakeGenericMethod(typeof(TEntity)), + source.Expression, + Expression.Constant(partitionKeyValue1, typeof(object)), + Expression.Constant(partitionKeyValue2, typeof(object)))) + : source; + } + + /// + /// Specify the partition key for partition used for the query. + /// Required when using a resource token that provides permission based on a partition key for authentication, + /// + /// + /// See Querying data with EF Core, and + /// Accessing Azure Cosmos DB with EF Core for more information and examples. + /// + /// The type of entity being queried. + /// The source query. + /// The first value in a hierarchical partition key. + /// The second value in a hierarchical partition key. + /// The third value in a hierarchical partition key. + /// A new query with the set partition key. + public static IQueryable WithPartitionKey( + this IQueryable source, + object partitionKeyValue1, + object partitionKeyValue2, + object partitionKeyValue3) + where TEntity : class + { + Check.NotNull(partitionKeyValue1, nameof(partitionKeyValue1)); + Check.NotNull(partitionKeyValue2, nameof(partitionKeyValue2)); + Check.NotNull(partitionKeyValue3, nameof(partitionKeyValue3)); return source.Provider is EntityQueryProvider ? source.Provider.CreateQuery( Expression.Call( instance: null, - method: WithPartitionKeyMethodInfo.MakeGenericMethod(typeof(TEntity)), + method: WithPartitionKeyMethodInfo3.MakeGenericMethod(typeof(TEntity)), source.Expression, - Expression.Constant(partitionKeyValue, typeof(object)), - Expression.Constant(additionalPartitionKeyValues, typeof(object[])))) + Expression.Constant(partitionKeyValue1, typeof(object)), + Expression.Constant(partitionKeyValue2, typeof(object)), + Expression.Constant(partitionKeyValue3, typeof(object)))) : source; } diff --git a/src/EFCore.Cosmos/Properties/CosmosStrings.Designer.cs b/src/EFCore.Cosmos/Properties/CosmosStrings.Designer.cs index fcde6e3de80..d9571fc2d05 100644 --- a/src/EFCore.Cosmos/Properties/CosmosStrings.Designer.cs +++ b/src/EFCore.Cosmos/Properties/CosmosStrings.Designer.cs @@ -163,14 +163,6 @@ public static string IdNonStringStoreType(object? idProperty, object? entityType GetString("IdNonStringStoreType", nameof(idProperty), nameof(entityType), nameof(propertyType)), idProperty, entityType, propertyType); - /// - /// {actual} partition key values were provided, but the entity type '{entityType}' has {expected} partition key values defined. - /// - public static string IncorrectPartitionKeyNumber(object? entityType, object? actual, object? expected) - => string.Format( - GetString("IncorrectPartitionKeyNumber", nameof(entityType), nameof(actual), nameof(expected)), - entityType, actual, expected); - /// /// The entity type '{entityType}' has an index defined over properties '{properties}'. The Azure Cosmos DB provider for EF Core currently does not support index definitions. /// diff --git a/src/EFCore.Cosmos/Properties/CosmosStrings.resx b/src/EFCore.Cosmos/Properties/CosmosStrings.resx index 59db5844f85..171d85f852c 100644 --- a/src/EFCore.Cosmos/Properties/CosmosStrings.resx +++ b/src/EFCore.Cosmos/Properties/CosmosStrings.resx @@ -174,9 +174,6 @@ The type of the '{idProperty}' property on '{entityType}' is '{propertyType}'. All 'id' properties must be strings or have a string value converter. - - {actual} partition key values were provided, but the entity type '{entityType}' has {expected} partition key values defined. - The entity type '{entityType}' has an index defined over properties '{properties}'. The Azure Cosmos DB provider for EF Core currently does not support index definitions. diff --git a/src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs b/src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs index 7e7c7095699..9b446a78152 100644 --- a/src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs +++ b/src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs @@ -172,23 +172,15 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp var innerQueryable = Visit(methodCallExpression.Arguments[0]); - var firstValue = _sqlTranslator.Translate(methodCallExpression.Arguments[1], applyDefaultTypeMapping: false); - if (firstValue is not SqlConstantExpression and not SqlParameterExpression) + for (var i = 1; i < methodCallExpression.Arguments.Count; i++) { - throw new InvalidOperationException(CosmosStrings.WithPartitionKeyNotConstantOrParameter); - } - - _queryCompilationContext.PartitionKeyPropertyValues.Add(firstValue); - - if (methodCallExpression.Arguments.Count == 3) - { - var remainingValuesArray = _sqlTranslator.Translate(methodCallExpression.Arguments[2], applyDefaultTypeMapping: false); - if (remainingValuesArray is not SqlParameterExpression) + var value = _sqlTranslator.Translate(methodCallExpression.Arguments[i], applyDefaultTypeMapping: false); + if (value is not SqlConstantExpression and not SqlParameterExpression) { throw new InvalidOperationException(CosmosStrings.WithPartitionKeyNotConstantOrParameter); } - _queryCompilationContext.PartitionKeyPropertyValues.Add(remainingValuesArray); + _queryCompilationContext.PartitionKeyPropertyValues.Add(value); } return innerQueryable; diff --git a/src/EFCore.Cosmos/Query/Internal/CosmosReadItemAndPartitionKeysExtractor.cs b/src/EFCore.Cosmos/Query/Internal/CosmosReadItemAndPartitionKeysExtractor.cs index 2b06a3f2e44..a0311a5c457 100644 --- a/src/EFCore.Cosmos/Query/Internal/CosmosReadItemAndPartitionKeysExtractor.cs +++ b/src/EFCore.Cosmos/Query/Internal/CosmosReadItemAndPartitionKeysExtractor.cs @@ -6,7 +6,8 @@ namespace Microsoft.EntityFrameworkCore.Cosmos.Query.Internal; /// -/// Identifies Cosmos queries that can be transformed to optimized ReadItem form and performs the transformation. +/// Identifies Cosmos queries that can be transformed to optimized ReadItem form and performs the transformation; also extracts out +/// partition key comparisons from the predicate. /// /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -23,7 +24,7 @@ public class CosmosReadItemAndPartitionKeysExtractor : ExpressionVisitor private bool _discriminatorHandled; private string? _discriminatorJsonPropertyName; private Dictionary _jsonIdPropertyValues = null!; - private Dictionary _partitionKeyPropertyValues = null!; + private Dictionary _partitionKeyPropertyValues = null!; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -74,51 +75,48 @@ public virtual Expression ExtractPartitionKeysAndId( _jsonIdPropertyValues = jsonIdProperties.ToDictionary(p => p, _ => (Expression?)null); var partitionKeyProperties = _entityType.GetPartitionKeyProperties(); - _partitionKeyPropertyValues = partitionKeyProperties.ToDictionary(p => p, _ => (Expression?)null); + _partitionKeyPropertyValues = partitionKeyProperties.ToDictionary( + p => p, _ => (ValueExpression: (Expression?)null, (Expression?)null)); + _discriminatorJsonPropertyName = _entityType.FindDiscriminatorProperty()?.GetJsonPropertyName(); // Visit the predicate. - // This will populate _jsonIdPropertyValues and _partitionKeyPropertyValues with comparisons found in the predicate, and return - // a rewritten predicate where the partition key comparisons have been removed. - var predicateWithoutPartitionKeyComparisons = (SqlExpression)Visit(predicate); + // This will populate _jsonIdPropertyValues and _partitionKeyPropertyValues with comparisons found in the predicate. + // It does not modify the predicate (this may happen below if we lift our partition key comparisons). + var samePredicate = (SqlExpression)Visit(predicate); + Check.DebugAssert(ReferenceEquals(samePredicate, predicate), "Visitation shouldn't have changed the predicate."); var allIdPropertiesSpecified = _jsonIdPropertyValues.Values.All(p => p is not null) && _jsonIdPropertyValues.Count > 0; - var allPartitionKeyPropertiesSpecified = _partitionKeyPropertyValues.Values.All(p => p is not null); - - // First, take care of the partition key properties; if the visitation above returned a different predicate, that means that some - // partition key comparisons were extracted (and therefore found). Lift these up to the query compilation context and rewrite - // the SelectExpression with the new, reduced predicate. - // Note that if the user called WithPartitionKey(), we'll have already populated the partition key property values from there, and - // we skip lifting the predicate comparisons. - if (allPartitionKeyPropertiesSpecified - && queryCompilationContext.PartitionKeyPropertyValues.Count == 0) + + // First, go over the partition key properties and lift them from the predicate to the query compilation context, as possible. + // We do this only as long as all partition key values are provided; the moment there's a gap we stop (so if PK1 and PK3 are + // provided but not PK2, only PK1 will be lifted out). + // Note that if the user called WithPartitionKey(), we'll have already populated the partition key property values from there; for + // this case, we skip lifting the predicate comparisons and leave the predicate exactly as it is (it may conflict with the values + // given in WithPartitionKey and return zero results - that's the expected behavior). + var liftPartitionKeys = queryCompilationContext.PartitionKeyPropertyValues.Count == 0; + foreach (var property in partitionKeyProperties) { - foreach (var partitionKeyProperty in partitionKeyProperties) + if (liftPartitionKeys && _partitionKeyPropertyValues[property].ValueExpression is Expression valueExpression) { - queryCompilationContext.PartitionKeyPropertyValues.Add(_partitionKeyPropertyValues[partitionKeyProperty]!); + queryCompilationContext.PartitionKeyPropertyValues.Add(valueExpression); + } + else + { + // We either have a gap in the partition key comparisons in the predicate (so we can't lift later ones), or the user + // specified a partition key value via WithPartitionKey. In either case, we need to not lift out comparisons and null out + // _partitionKeyPropertyValues, to prevent us removing the comparisons from the predicate below. + liftPartitionKeys = false; + _partitionKeyPropertyValues[property] = (null, null); } - - select = select.Update( - select.Sources.ToList(), - predicateWithoutPartitionKeyComparisons is SqlConstantExpression { Value: true } - ? null - : predicateWithoutPartitionKeyComparisons, - select.Projection.ToList(), - select.Orderings.ToList(), - select.Offset, - select.Limit); - - shapedQuery = shapedQuery.UpdateQueryExpression(select); } - // Now, attempt to also transform the query to ReadItem form if possible. + // Now, attempt to also transform the query to ReadItem form; this is only possible if all JSON ID properties were compared in the + // predicate, and *all* partition key values are specified(in the predicate or via WithPartitionKey) if (_isPredicateCompatibleWithReadItem && allIdPropertiesSpecified - // Note that queryCompilationContext.PartitionKeyPropertyValues may have been populated with WithPartitionKey(), which has - // a params object[] argument that gets parameterized as a single array. So the number of property values may not match the - // number of partition key properties. - && (partitionKeyProperties.Count == 0 || queryCompilationContext.PartitionKeyPropertyValues.Count > 0) + && queryCompilationContext.PartitionKeyPropertyValues.Count == partitionKeyProperties.Count && select is { Offset: null or SqlConstantExpression { Value: 0 }, @@ -132,6 +130,28 @@ public virtual Expression ExtractPartitionKeysAndId( return shapedQuery.UpdateQueryExpression(select.WithReadItemInfo(new ReadItemInfo(_jsonIdPropertyValues!))); } + // We couldn't transform to ReadItem - some JSON ID or partition key property comparison was missing in the predicate. + // However, comparisons might still be there for some (or all) of the partition key properties. These have already been lifted + // up to the query compilation context (above), but we still need to remove them from the predicate. + if (partitionKeyProperties.Count > 0 && _partitionKeyPropertyValues[partitionKeyProperties[0]].ValueExpression is not null) + { + var predicateWithoutPartitionKeyComparisons = (SqlExpression)new PredicateComparisonRemover( + _sqlExpressionFactory, + _partitionKeyPropertyValues.Values.Select(p => p.OriginalExpression).OfType().ToList()) + .Visit(predicate); + Check.DebugAssert(!ReferenceEquals(predicateWithoutPartitionKeyComparisons, predicate), "Predicate should have changed."); + + select = select.Update( + select.Sources.ToList(), + predicateWithoutPartitionKeyComparisons, + select.Projection.ToList(), + select.Orderings.ToList(), + select.Offset, + select.Limit); + + shapedQuery = shapedQuery.UpdateQueryExpression(select); + } + return shapedQuery; Expression Unwrap(Expression shaper) @@ -194,6 +214,7 @@ protected override Expression VisitExtension(Expression node) _isPredicateCompatibleWithReadItem = false; return node; } + case SqlBinaryExpression { OperatorType: ExpressionType.Equal, Left: var left, Right: var right } binary: { // TODO: Handle property accesses into complex types/owned entity types, #25548 @@ -209,7 +230,8 @@ left is ScalarAccessExpression leftScalarAccess if (scalarAccess?.Object is ObjectReferenceExpression { Name: var referencedSourceAlias } && referencedSourceAlias == _rootAlias) { - return ProcessPropertyComparison(scalarAccess.PropertyName, propertyValue!, binary); + ProcessPropertyComparison(scalarAccess.PropertyName, propertyValue!, binary); + return node; } _isPredicateCompatibleWithReadItem = false; @@ -218,7 +240,8 @@ left is ScalarAccessExpression leftScalarAccess // Bool property access (e.g. Where(b => b.BoolPartitionKey)) case ScalarAccessExpression { PropertyName: var propertyName } scalarAccess: - return ProcessPropertyComparison(propertyName, _sqlExpressionFactory.Constant(true), scalarAccess); + ProcessPropertyComparison(propertyName, _sqlExpressionFactory.Constant(true), scalarAccess); + return node; // Negated bool property access (e.g. Where(b => !b.BoolPartitionKey)) case SqlUnaryExpression @@ -226,15 +249,11 @@ left is ScalarAccessExpression leftScalarAccess OperatorType: ExpressionType.Not, Operand: ScalarAccessExpression { PropertyName: var propertyName } } unary: - return ProcessPropertyComparison(propertyName, _sqlExpressionFactory.Constant(false), unary); + ProcessPropertyComparison(propertyName, _sqlExpressionFactory.Constant(false), unary); + return node; case SqlBinaryExpression { OperatorType: ExpressionType.AndAlso } binary: - return _sqlExpressionFactory.MakeBinary( - ExpressionType.AndAlso, - (SqlExpression)Visit(binary.Left), - (SqlExpression)Visit(binary.Right), - binary.TypeMapping, - binary)!; + return binary.Update((SqlExpression)Visit(binary.Left), (SqlExpression)Visit(binary.Right)); default: // Anything else in the predicate, e.g. an OR, immediately disqualifies it from being a ReadItem query, and means we @@ -243,7 +262,7 @@ left is ScalarAccessExpression leftScalarAccess return node; } - SqlExpression ProcessPropertyComparison(string propertyName, SqlExpression propertyValue, SqlExpression originalExpression) + void ProcessPropertyComparison(string propertyName, SqlExpression propertyValue, SqlExpression originalExpression) { // We assume that the comparison is incompatible with ReadItem until proven otherwise, i.e. the comparison is for a JSON ID // property, a partition key property, or certain cases involving the discriminator property. @@ -283,11 +302,11 @@ SqlExpression ProcessPropertyComparison(string propertyName, SqlExpression prope // Extract its value expression and elide the comparison from the predicate - it'll be lifted out to the Cosmos SDK // call. Note that this is always considered a compatible comparison for ReadItem. if (propertyName == property.GetJsonPropertyName() - && _partitionKeyPropertyValues.TryGetValue(property, out var previousValue) - && (previousValue is null || previousValue.Equals(propertyValue))) + && _partitionKeyPropertyValues.TryGetValue(property, out var previousValues) + && (previousValues.ValueExpression is null || previousValues.Equals(propertyValue))) { - _partitionKeyPropertyValues[property] = propertyValue; - return _sqlExpressionFactory.Constant(true); + _partitionKeyPropertyValues[property] = (ValueExpression: propertyValue, OriginalExpression: originalExpression); + return; } } @@ -295,8 +314,29 @@ SqlExpression ProcessPropertyComparison(string propertyName, SqlExpression prope { _isPredicateCompatibleWithReadItem = false; } - - return originalExpression; } } + + private sealed class PredicateComparisonRemover(ISqlExpressionFactory sqlExpressionFactory, List comparisonsToRemove) + : ExpressionVisitor + { + protected override Expression VisitExtension(Expression node) + => node switch + { + _ when comparisonsToRemove.Contains(node) + => sqlExpressionFactory.Constant(true), + + // This elides `AND true` from the predicate. + // TODO: We shouldn't need to do this explicitly, see #34556. + SqlBinaryExpression { OperatorType: ExpressionType.AndAlso } binary + => sqlExpressionFactory.MakeBinary( + ExpressionType.AndAlso, + (SqlExpression)Visit(binary.Left), + (SqlExpression)Visit(binary.Right), + binary.TypeMapping, + binary)!, + + _ => base.VisitExtension(node) + }; + } } diff --git a/src/EFCore.Cosmos/Query/Internal/CosmosShapedQueryCompilingExpressionVisitor.cs b/src/EFCore.Cosmos/Query/Internal/CosmosShapedQueryCompilingExpressionVisitor.cs index 2ad551a6521..e8aa6e16db2 100644 --- a/src/EFCore.Cosmos/Query/Internal/CosmosShapedQueryCompilingExpressionVisitor.cs +++ b/src/EFCore.Cosmos/Query/Internal/CosmosShapedQueryCompilingExpressionVisitor.cs @@ -145,8 +145,7 @@ private static PartitionKey GeneratePartitionKey( var partitionKeyProperties = rootEntityType.GetPartitionKeyProperties(); - int i; - for (i = 0; i < partitionKeyPropertyValues.Count && i < partitionKeyProperties.Count; i++) + for (var i = 0; i < partitionKeyPropertyValues.Count && i < partitionKeyProperties.Count; i++) { var property = partitionKeyProperties[i]; @@ -156,25 +155,6 @@ private static PartitionKey GeneratePartitionKey( builder.Add(constant.Value, property); continue; - // If WithPartitionKey() was used, its second argument is a params object[] array, which gets parameterized as a single - // parameter. Extract the object[] and iterate over the values within here. - case SqlParameterExpression parameter when parameter.Type == typeof(object[]): - { - if (!parameterValues.TryGetValue(parameter.Name, out var value) - || value is not object[] remainingValuesArray - || i != 1) - { - throw new UnreachableException("Couldn't find partition key parameter value"); - } - - for (var j = 0; j < remainingValuesArray.Length; j++, i++) - { - builder.Add(remainingValuesArray[j], partitionKeyProperties[i]); - } - - goto End; - } - case SqlParameterExpression parameter: { builder.Add( @@ -190,13 +170,6 @@ private static PartitionKey GeneratePartitionKey( } } - End: - if (i != partitionKeyProperties.Count) - { - throw new InvalidOperationException( - CosmosStrings.IncorrectPartitionKeyNumber(rootEntityType.DisplayName(), i, partitionKeyProperties.Count)); - } - return builder.Build(); } } diff --git a/src/EFCore.Cosmos/Query/Internal/Expressions/SelectExpression.cs b/src/EFCore.Cosmos/Query/Internal/Expressions/SelectExpression.cs index b3c10865b23..3f31abdf5a8 100644 --- a/src/EFCore.Cosmos/Query/Internal/Expressions/SelectExpression.cs +++ b/src/EFCore.Cosmos/Query/Internal/Expressions/SelectExpression.cs @@ -48,7 +48,7 @@ public SelectExpression( SqlExpression? limit) { _sources = sources; - Predicate = predicate; + Predicate = predicate is SqlConstantExpression { Value: true } ? null : predicate; _projection = projections; IsDistinct = distinct; _orderings = orderings; diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryDiscriminatorInIdTest.cs b/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryDiscriminatorInIdTest.cs index 7b16a7e4de1..6e098c92d28 100644 --- a/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryDiscriminatorInIdTest.cs +++ b/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryDiscriminatorInIdTest.cs @@ -78,7 +78,20 @@ public override async Task Predicate_with_partial_values_in_hierarchical_partiti """ SELECT VALUE c FROM root c -WHERE (c["$type"] IN ("HierarchicalPartitionKeyEntity", "DerivedHierarchicalPartitionKeyEntity") AND ((c["PartitionKey1"] = "PK1") AND (c["PartitionKey2"] = 1))) +WHERE c["$type"] IN ("HierarchicalPartitionKeyEntity", "DerivedHierarchicalPartitionKeyEntity") +"""); + } + + public override async Task Predicate_with_partial_values_and_gap_in_hierarchical_partition_key() + { + await base.Predicate_with_partial_values_and_gap_in_hierarchical_partition_key(); + + // Not ReadItem because no primary key value + AssertSql( + """ +SELECT VALUE c +FROM root c +WHERE (c["$type"] IN ("HierarchicalPartitionKeyEntity", "DerivedHierarchicalPartitionKeyEntity") AND c["PartitionKey3"]) """); } @@ -91,7 +104,7 @@ public override async Task Predicate_with_partial_values_in_only_hierarchical_pa """ SELECT VALUE c FROM root c -WHERE (c["$type"] IN ("OnlyHierarchicalPartitionKeyEntity", "DerivedOnlyHierarchicalPartitionKeyEntity") AND ((c["PartitionKey1"] = "PK1a") AND (c["PartitionKey2"] = 1))) +WHERE c["$type"] IN ("OnlyHierarchicalPartitionKeyEntity", "DerivedOnlyHierarchicalPartitionKeyEntity") """); } @@ -175,11 +188,16 @@ FROM root c """); } - public override async Task WithPartitionKey_with_missing_value_in_hierarchical_partition_key() + public override async Task WithPartitionKey_with_partial_value_in_hierarchical_partition_key() { - await base.WithPartitionKey_with_missing_value_in_hierarchical_partition_key(); + await base.WithPartitionKey_with_partial_value_in_hierarchical_partition_key(); - AssertSql(); + AssertSql( + """ +SELECT VALUE c +FROM root c +WHERE c["$type"] IN ("HierarchicalPartitionKeyEntity", "DerivedHierarchicalPartitionKeyEntity") +"""); } public override async Task Both_WithPartitionKey_and_predicate_comparisons_with_different_values() @@ -568,7 +586,7 @@ public override async Task Predicate_with_partial_values_in_hierarchical_partiti """ SELECT VALUE c FROM root c -WHERE ((c["$type"] = "DerivedHierarchicalPartitionKeyEntity") AND ((c["PartitionKey1"] = "PK1") AND (c["PartitionKey2"] = 1))) +WHERE (c["$type"] = "DerivedHierarchicalPartitionKeyEntity") """); } @@ -581,7 +599,7 @@ public override async Task Predicate_with_partial_values_in_only_hierarchical_pa """ SELECT VALUE c FROM root c -WHERE ((c["$type"] = "DerivedOnlyHierarchicalPartitionKeyEntity") AND ((c["PartitionKey1"] = "PK1c") AND (c["PartitionKey2"] = 1))) +WHERE (c["$type"] = "DerivedOnlyHierarchicalPartitionKeyEntity") """); } @@ -665,11 +683,16 @@ FROM root c """); } - public override async Task WithPartitionKey_with_missing_value_in_hierarchical_partition_key_leaf() + public override async Task WithPartitionKey_with_partial_value_in_hierarchical_partition_key_leaf() { - await base.WithPartitionKey_with_missing_value_in_hierarchical_partition_key(); + await base.WithPartitionKey_with_partial_value_in_hierarchical_partition_key(); - AssertSql(); + AssertSql( + """ +SELECT VALUE c +FROM root c +WHERE c["$type"] IN ("HierarchicalPartitionKeyEntity", "DerivedHierarchicalPartitionKeyEntity") +"""); } public override async Task Both_WithPartitionKey_and_predicate_comparisons_with_different_values_leaf() diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryInheritanceTestBase.cs b/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryInheritanceTestBase.cs index a3192ebd864..f79173d7eda 100644 --- a/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryInheritanceTestBase.cs +++ b/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryInheritanceTestBase.cs @@ -108,17 +108,11 @@ public virtual Task WithPartitionKey_with_only_single_partition_key_leaf() ss => ss.Set().Where(e => e.PartitionKey == "PK1c")); [ConditionalFact] - public virtual async Task WithPartitionKey_with_missing_value_in_hierarchical_partition_key_leaf() - { - var message = await Assert.ThrowsAsync( - () => AssertQuery( - async: true, - ss => ss.Set().WithPartitionKey("PK1", 1), - ss => ss.Set() - .Where(e => e.PartitionKey1 == "PK1" && e.PartitionKey2 == 1 && e.PartitionKey3))); - - Assert.Equal(CosmosStrings.IncorrectPartitionKeyNumber(nameof(DerivedHierarchicalPartitionKeyEntity), 2, 3), message.Message); - } + public virtual Task WithPartitionKey_with_partial_value_in_hierarchical_partition_key_leaf() + => AssertQuery( + async: true, + ss => ss.Set().WithPartitionKey("PK1", 1), + ss => ss.Set().Where(e => e.PartitionKey1 == "PK1" && e.PartitionKey2 == 1)); [ConditionalFact] public virtual Task Both_WithPartitionKey_and_predicate_comparisons_with_different_values_leaf() diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryNoDiscriminatorInIdTest.cs b/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryNoDiscriminatorInIdTest.cs index 56bf7ee34cd..f672784f717 100644 --- a/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryNoDiscriminatorInIdTest.cs +++ b/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryNoDiscriminatorInIdTest.cs @@ -66,7 +66,20 @@ public override async Task Predicate_with_partial_values_in_hierarchical_partiti """ SELECT VALUE c FROM root c -WHERE (c["$type"] IN ("HierarchicalPartitionKeyEntity", "DerivedHierarchicalPartitionKeyEntity") AND ((c["PartitionKey1"] = "PK1") AND (c["PartitionKey2"] = 1))) +WHERE c["$type"] IN ("HierarchicalPartitionKeyEntity", "DerivedHierarchicalPartitionKeyEntity") +"""); + } + + public override async Task Predicate_with_partial_values_and_gap_in_hierarchical_partition_key() + { + await base.Predicate_with_partial_values_and_gap_in_hierarchical_partition_key(); + + // Not ReadItem because no primary key value + AssertSql( + """ +SELECT VALUE c +FROM root c +WHERE (c["$type"] IN ("HierarchicalPartitionKeyEntity", "DerivedHierarchicalPartitionKeyEntity") AND c["PartitionKey3"]) """); } @@ -79,7 +92,7 @@ public override async Task Predicate_with_partial_values_in_only_hierarchical_pa """ SELECT VALUE c FROM root c -WHERE (c["$type"] IN ("OnlyHierarchicalPartitionKeyEntity", "DerivedOnlyHierarchicalPartitionKeyEntity") AND ((c["PartitionKey1"] = "PK1a") AND (c["PartitionKey2"] = 1))) +WHERE c["$type"] IN ("OnlyHierarchicalPartitionKeyEntity", "DerivedOnlyHierarchicalPartitionKeyEntity") """); } @@ -161,11 +174,16 @@ FROM root c """); } - public override async Task WithPartitionKey_with_missing_value_in_hierarchical_partition_key() + public override async Task WithPartitionKey_with_partial_value_in_hierarchical_partition_key() { - await base.WithPartitionKey_with_missing_value_in_hierarchical_partition_key(); + await base.WithPartitionKey_with_partial_value_in_hierarchical_partition_key(); - AssertSql(); + AssertSql( + """ +SELECT VALUE c +FROM root c +WHERE c["$type"] IN ("HierarchicalPartitionKeyEntity", "DerivedHierarchicalPartitionKeyEntity") +"""); } public override async Task Both_WithPartitionKey_and_predicate_comparisons_with_different_values() @@ -443,7 +461,7 @@ public override async Task Predicate_with_partial_values_in_hierarchical_partiti """ SELECT VALUE c FROM root c -WHERE ((c["$type"] = "DerivedHierarchicalPartitionKeyEntity") AND ((c["PartitionKey1"] = "PK1") AND (c["PartitionKey2"] = 1))) +WHERE (c["$type"] = "DerivedHierarchicalPartitionKeyEntity") """); } @@ -456,7 +474,7 @@ public override async Task Predicate_with_partial_values_in_only_hierarchical_pa """ SELECT VALUE c FROM root c -WHERE ((c["$type"] = "DerivedOnlyHierarchicalPartitionKeyEntity") AND ((c["PartitionKey1"] = "PK1c") AND (c["PartitionKey2"] = 1))) +WHERE (c["$type"] = "DerivedOnlyHierarchicalPartitionKeyEntity") """); } @@ -538,11 +556,16 @@ FROM root c """); } - public override async Task WithPartitionKey_with_missing_value_in_hierarchical_partition_key_leaf() + public override async Task WithPartitionKey_with_partial_value_in_hierarchical_partition_key_leaf() { - await base.WithPartitionKey_with_missing_value_in_hierarchical_partition_key_leaf(); + await base.WithPartitionKey_with_partial_value_in_hierarchical_partition_key_leaf(); - AssertSql(); + AssertSql( + """ +SELECT VALUE c +FROM root c +WHERE (c["$type"] = "DerivedHierarchicalPartitionKeyEntity") +"""); } public override async Task Both_WithPartitionKey_and_predicate_comparisons_with_different_values_leaf() diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryRootDiscriminatorInIdTest.cs b/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryRootDiscriminatorInIdTest.cs index f87617a8687..8e0e574a193 100644 --- a/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryRootDiscriminatorInIdTest.cs +++ b/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryRootDiscriminatorInIdTest.cs @@ -66,7 +66,20 @@ public override async Task Predicate_with_partial_values_in_hierarchical_partiti """ SELECT VALUE c FROM root c -WHERE (c["$type"] IN ("HierarchicalPartitionKeyEntity", "DerivedHierarchicalPartitionKeyEntity") AND ((c["PartitionKey1"] = "PK1") AND (c["PartitionKey2"] = 1))) +WHERE c["$type"] IN ("HierarchicalPartitionKeyEntity", "DerivedHierarchicalPartitionKeyEntity") +"""); + } + + public override async Task Predicate_with_partial_values_and_gap_in_hierarchical_partition_key() + { + await base.Predicate_with_partial_values_and_gap_in_hierarchical_partition_key(); + + // Not ReadItem because no primary key value + AssertSql( + """ +SELECT VALUE c +FROM root c +WHERE (c["$type"] IN ("HierarchicalPartitionKeyEntity", "DerivedHierarchicalPartitionKeyEntity") AND c["PartitionKey3"]) """); } @@ -79,7 +92,7 @@ public override async Task Predicate_with_partial_values_in_only_hierarchical_pa """ SELECT VALUE c FROM root c -WHERE (c["$type"] IN ("OnlyHierarchicalPartitionKeyEntity", "DerivedOnlyHierarchicalPartitionKeyEntity") AND ((c["PartitionKey1"] = "PK1a") AND (c["PartitionKey2"] = 1))) +WHERE c["$type"] IN ("OnlyHierarchicalPartitionKeyEntity", "DerivedOnlyHierarchicalPartitionKeyEntity") """); } @@ -163,11 +176,16 @@ FROM root c """); } - public override async Task WithPartitionKey_with_missing_value_in_hierarchical_partition_key() + public override async Task WithPartitionKey_with_partial_value_in_hierarchical_partition_key() { - await base.WithPartitionKey_with_missing_value_in_hierarchical_partition_key(); + await base.WithPartitionKey_with_partial_value_in_hierarchical_partition_key(); - AssertSql(); + AssertSql( + """ +SELECT VALUE c +FROM root c +WHERE c["$type"] IN ("HierarchicalPartitionKeyEntity", "DerivedHierarchicalPartitionKeyEntity") +"""); } public override async Task Both_WithPartitionKey_and_predicate_comparisons_with_different_values() @@ -445,7 +463,7 @@ public override async Task Predicate_with_partial_values_in_hierarchical_partiti """ SELECT VALUE c FROM root c -WHERE ((c["$type"] = "DerivedHierarchicalPartitionKeyEntity") AND ((c["PartitionKey1"] = "PK1") AND (c["PartitionKey2"] = 1))) +WHERE (c["$type"] = "DerivedHierarchicalPartitionKeyEntity") """); } @@ -458,7 +476,7 @@ public override async Task Predicate_with_partial_values_in_only_hierarchical_pa """ SELECT VALUE c FROM root c -WHERE ((c["$type"] = "DerivedOnlyHierarchicalPartitionKeyEntity") AND ((c["PartitionKey1"] = "PK1c") AND (c["PartitionKey2"] = 1))) +WHERE (c["$type"] = "DerivedOnlyHierarchicalPartitionKeyEntity") """); } @@ -542,11 +560,16 @@ FROM root c """); } - public override async Task WithPartitionKey_with_missing_value_in_hierarchical_partition_key_leaf() + public override async Task WithPartitionKey_with_partial_value_in_hierarchical_partition_key_leaf() { - await base.WithPartitionKey_with_missing_value_in_hierarchical_partition_key_leaf(); + await base.WithPartitionKey_with_partial_value_in_hierarchical_partition_key_leaf(); - AssertSql(); + AssertSql( + """ +SELECT VALUE c +FROM root c +WHERE (c["$type"] = "DerivedHierarchicalPartitionKeyEntity") +"""); } public override async Task Both_WithPartitionKey_and_predicate_comparisons_with_different_values_leaf() diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryTest.cs b/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryTest.cs index e0af34c54ac..b31eb485e6e 100644 --- a/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryTest.cs +++ b/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryTest.cs @@ -60,12 +60,25 @@ public override async Task Predicate_with_partial_values_in_hierarchical_partiti { await base.Predicate_with_partial_values_in_hierarchical_partition_key(); - // Not ReadItem because no primary key value + // Not ReadItem because no primary key value, but partial partition key value is extracted + AssertSql( + """ +SELECT VALUE c +FROM root c +WHERE (c["$type"] = "HierarchicalPartitionKeyEntity") +"""); + } + + public override async Task Predicate_with_partial_values_and_gap_in_hierarchical_partition_key() + { + await base.Predicate_with_partial_values_and_gap_in_hierarchical_partition_key(); + + // Not ReadItem because no primary key value, but partial partition key value is extracted AssertSql( """ SELECT VALUE c FROM root c -WHERE ((c["$type"] = "HierarchicalPartitionKeyEntity") AND ((c["PartitionKey1"] = "PK1") AND (c["PartitionKey2"] = 1))) +WHERE ((c["$type"] = "HierarchicalPartitionKeyEntity") AND c["PartitionKey3"]) """); } @@ -79,7 +92,7 @@ public override async Task Predicate_with_partial_values_in_only_hierarchical_pa """ SELECT VALUE c FROM root c -WHERE ((c["$type"] = "OnlyHierarchicalPartitionKeyEntity") AND ((c["PartitionKey1"] = "PK1a") AND (c["PartitionKey2"] = 1))) +WHERE (c["$type"] = "OnlyHierarchicalPartitionKeyEntity") """); } @@ -159,11 +172,16 @@ FROM root c """); } - public override async Task WithPartitionKey_with_missing_value_in_hierarchical_partition_key() + public override async Task WithPartitionKey_with_partial_value_in_hierarchical_partition_key() { - await base.WithPartitionKey_with_missing_value_in_hierarchical_partition_key(); + await base.WithPartitionKey_with_partial_value_in_hierarchical_partition_key(); - AssertSql(); + AssertSql( + """ +SELECT VALUE c +FROM root c +WHERE (c["$type"] = "HierarchicalPartitionKeyEntity") +"""); } public override async Task Both_WithPartitionKey_and_predicate_comparisons_with_different_values() diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryTestBase.cs b/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryTestBase.cs index 97c63f9a6ed..5a0bedaf279 100644 --- a/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryTestBase.cs +++ b/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryTestBase.cs @@ -48,6 +48,13 @@ public virtual Task Predicate_with_partial_values_in_hierarchical_partition_key( ss => ss.Set() .Where(e => e.PartitionKey1 == "PK1" && e.PartitionKey2 == 1)); + [ConditionalFact] + public virtual Task Predicate_with_partial_values_and_gap_in_hierarchical_partition_key() + => AssertQuery( + async: true, + ss => ss.Set() + .Where(e => e.PartitionKey1 == "PK1" && e.PartitionKey3)); + [ConditionalFact] public virtual Task Predicate_with_partial_values_in_only_hierarchical_partition_key() => AssertQuery( @@ -108,17 +115,11 @@ public virtual Task WithPartitionKey_with_only_single_partition_key() ss => ss.Set().Where(e => e.PartitionKey == "PK1a")); [ConditionalFact] - public virtual async Task WithPartitionKey_with_missing_value_in_hierarchical_partition_key() - { - var message = await Assert.ThrowsAsync( - () => AssertQuery( + public virtual Task WithPartitionKey_with_partial_value_in_hierarchical_partition_key() + => AssertQuery( async: true, ss => ss.Set().WithPartitionKey("PK1", 1), - ss => ss.Set() - .Where(e => e.PartitionKey1 == "PK1" && e.PartitionKey2 == 1 && e.PartitionKey3))); - - Assert.Equal(CosmosStrings.IncorrectPartitionKeyNumber(nameof(HierarchicalPartitionKeyEntity), 2, 3), message.Message); - } + ss => ss.Set().Where(e => e.PartitionKey1 == "PK1" && e.PartitionKey2 == 1)); [ConditionalFact] public virtual Task Both_WithPartitionKey_and_predicate_comparisons_with_different_values()