diff --git a/src/EFCore.Cosmos/Query/Internal/CosmosReadItemAndPartitionKeysExtractor.cs b/src/EFCore.Cosmos/Query/Internal/CosmosReadItemAndPartitionKeysExtractor.cs index c0221b87e81..06648b516e0 100644 --- a/src/EFCore.Cosmos/Query/Internal/CosmosReadItemAndPartitionKeysExtractor.cs +++ b/src/EFCore.Cosmos/Query/Internal/CosmosReadItemAndPartitionKeysExtractor.cs @@ -21,6 +21,7 @@ public class CosmosReadItemAndPartitionKeysExtractor : ExpressionVisitor private IEntityType _entityType = null!; private string _rootAlias = null!; private bool _isPredicateCompatibleWithReadItem; + private bool _partitionKeyValuesInPredicateConflictWithReadItem; private bool _nonRootDiscriminatorInJsonId; private string? _discriminatorJsonPropertyName; private Dictionary _jsonIdPropertyValues = null!; @@ -60,6 +61,7 @@ public virtual Expression ExtractPartitionKeysAndId( // from the tree (either constants or parameters). // We also want to ignore the discriminator property if it's compared to our entity type's discriminator value (see below). _isPredicateCompatibleWithReadItem = true; + _partitionKeyValuesInPredicateConflictWithReadItem = false; var jsonIdDefinition = _entityType.GetJsonIdDefinition(); var jsonIdProperties = jsonIdDefinition?.Properties ?? []; @@ -95,6 +97,33 @@ public virtual Expression ExtractPartitionKeysAndId( // 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; + + if (!liftPartitionKeys) // partition key values provided via WithPartitionKey() + { + // WithPartitionKey() values are translated with applyDefaultTypeMapping:false (TypeMapping is null), so we apply the + // predicate value's TypeMapping before comparing. A literal WithPartitionKey argument is also parameterized by the + // funcletizer (outside a lambda) while the same literal in Where() stays a constant — when all PK properties are in + // the JSON ID, ReadItem is safe in that case: a wrong partition key returns null, not the wrong document. + var pkPropertiesInJsonId = partitionKeyProperties.All(p => jsonIdProperties.Contains(p)); + + for (var i = 0; i < partitionKeyProperties.Count; i++) + { + if (_partitionKeyPropertyValues[partitionKeyProperties[i]].ValueExpression is not SqlExpression predicateValue) + { + continue; + } + + if (i >= queryCompilationContext.PartitionKeyPropertyValues.Count + || queryCompilationContext.PartitionKeyPropertyValues[i] is not SqlExpression withPkValue + || (!predicateValue.Equals(_sqlExpressionFactory.ApplyTypeMapping(withPkValue, predicateValue.TypeMapping)) + && !(pkPropertiesInJsonId && withPkValue is SqlParameterExpression && predicateValue is SqlConstantExpression))) + { + _partitionKeyValuesInPredicateConflictWithReadItem = true; + break; + } + } + } + foreach (var property in partitionKeyProperties) { if (liftPartitionKeys && _partitionKeyPropertyValues[property].ValueExpression is { } valueExpression) @@ -115,6 +144,10 @@ public virtual Expression ExtractPartitionKeysAndId( // predicate, and *all* partition key values are specified(in the predicate or via WithPartitionKey) if (_isPredicateCompatibleWithReadItem && allIdPropertiesSpecified + // If partition key values were provided via WithPartitionKey() and the predicate specifies different values, we don't + // transform to ReadItem: that would silently ignore the predicate and return a result for the WithPartitionKey() value. + // Executing as a regular query instead correctly yields zero results on conflict (see #38238). + && !_partitionKeyValuesInPredicateConflictWithReadItem && queryCompilationContext.PartitionKeyPropertyValues.Count == partitionKeyProperties.Count && select is { @@ -301,7 +334,7 @@ void ProcessPropertyComparison(string propertyName, SqlExpression propertyValue, if (propertyName == property.GetJsonPropertyName()) { if (_jsonIdPropertyValues.TryGetValue(property, out var previousValue) - && (previousValue is null || previousValue.Equals(propertyValue))) + && (previousValue is null || ExpressionEqualityComparer.Instance.Equals(previousValue, propertyValue))) { _jsonIdPropertyValues[property] = propertyValue; isCompatibleComparisonForReadItem = true; @@ -320,7 +353,8 @@ void ProcessPropertyComparison(string propertyName, SqlExpression propertyValue, // call. Note that this is always considered a compatible comparison for ReadItem. if (propertyName == property.GetJsonPropertyName() && _partitionKeyPropertyValues.TryGetValue(property, out var previousValues) - && (previousValues.ValueExpression is null || previousValues.Equals(propertyValue))) + && (previousValues.ValueExpression is null + || ExpressionEqualityComparer.Instance.Equals(previousValues.ValueExpression, propertyValue))) { _partitionKeyPropertyValues[property] = (ValueExpression: propertyValue, OriginalExpression: originalExpression); return; diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryDiscriminatorInIdTest.cs b/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryDiscriminatorInIdTest.cs index 2d142ed1fe3..5d9e249d3fd 100644 --- a/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryDiscriminatorInIdTest.cs +++ b/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryDiscriminatorInIdTest.cs @@ -456,6 +456,22 @@ FROM root c """); } + public override async Task WithPartitionKey_and_predicate_with_id_two_locals_same_value() + { + await base.WithPartitionKey_and_predicate_with_id_two_locals_same_value(); + + // Not ReadItem because pkForWith and pkForWhere are different parameter expressions (different names), + // so they can't be compared at translate time even though they have the same runtime value. + AssertSql( + """ +@pkForWhere='PK1' + +SELECT VALUE c +FROM root c +WHERE (c["$type"] IN ("SinglePartitionKeyEntity", "DerivedSinglePartitionKeyEntity") AND ((c["Id"] = "b29bced8-e1e5-420e-82d7-1c7a51703d34") AND (c["PartitionKey"] = @pkForWhere))) +"""); + } + public override async Task Multiple_incompatible_predicate_comparisons_cause_no_ReadItem() { await base.Multiple_incompatible_predicate_comparisons_cause_no_ReadItem(); @@ -894,6 +910,30 @@ public override async Task ReadItem_with_WithPartitionKey_with_only_partition_ke AssertSql("""ReadItem(["PK1c"], DerivedOnlySinglePartitionKeyEntity|PK1c)"""); } + public override async Task WithPartitionKey_and_predicate_with_id_and_conflicting_partition_key() + { + await base.WithPartitionKey_and_predicate_with_id_and_conflicting_partition_key(); + + AssertSql( + """ +SELECT VALUE c +FROM root c +WHERE (c["$type"] IN ("SinglePartitionKeyEntity", "DerivedSinglePartitionKeyEntity") AND ((c["Id"] = "b29bced8-e1e5-420e-82d7-1c7a51703d34") AND (c["PartitionKey"] = "PK2"))) +"""); + } + + public override async Task WithPartitionKey_and_predicate_with_id_and_conflicting_partition_key_leaf() + { + await base.WithPartitionKey_and_predicate_with_id_and_conflicting_partition_key_leaf(); + + AssertSql( + """ +SELECT VALUE c +FROM root c +WHERE ((c["$type"] = "DerivedSinglePartitionKeyEntity") AND ((c["Id"] = "188d3253-81be-4a87-b58f-a2bd07e6b98c") AND (c["PartitionKey"] = "PK2"))) +"""); + } + public override async Task Multiple_incompatible_predicate_comparisons_cause_no_ReadItem_leaf() { await base.Multiple_incompatible_predicate_comparisons_cause_no_ReadItem_leaf(); diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryInheritanceTestBase.cs b/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryInheritanceTestBase.cs index 39a0c545c79..134128da1eb 100644 --- a/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryInheritanceTestBase.cs +++ b/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryInheritanceTestBase.cs @@ -261,6 +261,16 @@ public virtual Task ReadItem_with_WithPartitionKey_with_only_partition_key_leaf( ss => ss.Set().WithPartitionKey("PK1c").Where(e => e.PartitionKey == "PK1c"), ss => ss.Set().Where(e => e.PartitionKey == "PK1c")); + [Fact] + public virtual Task WithPartitionKey_and_predicate_with_id_and_conflicting_partition_key_leaf() + => AssertQuery( + async: true, + ss => ss.Set().WithPartitionKey("PK1") + .Where(e => e.Id == Guid.Parse("188D3253-81BE-4A87-B58F-A2BD07E6B98C") && e.PartitionKey == "PK2"), + ss => ss.Set().Where(e => e.PartitionKey == "PK1") + .Where(e => e.Id == Guid.Parse("188D3253-81BE-4A87-B58F-A2BD07E6B98C") && e.PartitionKey == "PK2"), + assertEmpty: true); + [Fact] public virtual Task Multiple_incompatible_predicate_comparisons_cause_no_ReadItem_leaf() { diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryNoDiscriminatorInIdTest.cs b/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryNoDiscriminatorInIdTest.cs index d8351308d83..7d4f6466f61 100644 --- a/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryNoDiscriminatorInIdTest.cs +++ b/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryNoDiscriminatorInIdTest.cs @@ -369,6 +369,22 @@ public override async Task ReadItem_with_WithPartitionKey_with_only_partition_ke AssertSql("""ReadItem(["PK1a"], PK1a)"""); } + public override async Task WithPartitionKey_and_predicate_with_id_two_locals_same_value() + { + await base.WithPartitionKey_and_predicate_with_id_two_locals_same_value(); + + // Not ReadItem because pkForWith and pkForWhere are different parameter expressions (different names), + // so they can't be compared at translate time even though they have the same runtime value. + AssertSql( + """ +@pkForWhere='PK1' + +SELECT VALUE c +FROM root c +WHERE (c["$type"] IN ("SinglePartitionKeyEntity", "DerivedSinglePartitionKeyEntity") AND ((c["id"] = "b29bced8-e1e5-420e-82d7-1c7a51703d34") AND (c["PartitionKey"] = @pkForWhere))) +"""); + } + public override async Task Multiple_incompatible_predicate_comparisons_cause_no_ReadItem() { await base.Multiple_incompatible_predicate_comparisons_cause_no_ReadItem(); @@ -767,6 +783,30 @@ public override async Task ReadItem_with_WithPartitionKey_with_only_partition_ke AssertSql("""ReadItem(["PK1c"], PK1c)"""); } + public override async Task WithPartitionKey_and_predicate_with_id_and_conflicting_partition_key() + { + await base.WithPartitionKey_and_predicate_with_id_and_conflicting_partition_key(); + + AssertSql( + """ +SELECT VALUE c +FROM root c +WHERE (c["$type"] IN ("SinglePartitionKeyEntity", "DerivedSinglePartitionKeyEntity") AND ((c["id"] = "b29bced8-e1e5-420e-82d7-1c7a51703d34") AND (c["PartitionKey"] = "PK2"))) +"""); + } + + public override async Task WithPartitionKey_and_predicate_with_id_and_conflicting_partition_key_leaf() + { + await base.WithPartitionKey_and_predicate_with_id_and_conflicting_partition_key_leaf(); + + AssertSql( + """ +SELECT VALUE c +FROM root c +WHERE ((c["$type"] = "DerivedSinglePartitionKeyEntity") AND ((c["id"] = "188d3253-81be-4a87-b58f-a2bd07e6b98c") AND (c["PartitionKey"] = "PK2"))) +"""); + } + public override async Task Multiple_incompatible_predicate_comparisons_cause_no_ReadItem_leaf() { await base.Multiple_incompatible_predicate_comparisons_cause_no_ReadItem_leaf(); diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryRootDiscriminatorInIdTest.cs b/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryRootDiscriminatorInIdTest.cs index d2000eb7ce8..10d74c7119c 100644 --- a/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryRootDiscriminatorInIdTest.cs +++ b/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryRootDiscriminatorInIdTest.cs @@ -362,6 +362,22 @@ public override async Task ReadItem_with_WithPartitionKey_with_only_partition_ke AssertSql("""ReadItem(["PK1a"], OnlySinglePartitionKeyEntity|PK1a)"""); } + public override async Task WithPartitionKey_and_predicate_with_id_two_locals_same_value() + { + await base.WithPartitionKey_and_predicate_with_id_two_locals_same_value(); + + // Not ReadItem because pkForWith and pkForWhere are different parameter expressions (different names), + // so they can't be compared at translate time even though they have the same runtime value. + AssertSql( + """ +@pkForWhere='PK1' + +SELECT VALUE c +FROM root c +WHERE (c["$type"] IN ("SinglePartitionKeyEntity", "DerivedSinglePartitionKeyEntity") AND ((c["Id"] = "b29bced8-e1e5-420e-82d7-1c7a51703d34") AND (c["PartitionKey"] = @pkForWhere))) +"""); + } + public override async Task Multiple_incompatible_predicate_comparisons_cause_no_ReadItem() { await base.Multiple_incompatible_predicate_comparisons_cause_no_ReadItem(); @@ -762,6 +778,30 @@ public override async Task ReadItem_with_WithPartitionKey_with_only_partition_ke AssertSql("""ReadItem(["PK1c"], OnlySinglePartitionKeyEntity|PK1c)"""); } + public override async Task WithPartitionKey_and_predicate_with_id_and_conflicting_partition_key() + { + await base.WithPartitionKey_and_predicate_with_id_and_conflicting_partition_key(); + + AssertSql( + """ +SELECT VALUE c +FROM root c +WHERE (c["$type"] IN ("SinglePartitionKeyEntity", "DerivedSinglePartitionKeyEntity") AND ((c["Id"] = "b29bced8-e1e5-420e-82d7-1c7a51703d34") AND (c["PartitionKey"] = "PK2"))) +"""); + } + + public override async Task WithPartitionKey_and_predicate_with_id_and_conflicting_partition_key_leaf() + { + await base.WithPartitionKey_and_predicate_with_id_and_conflicting_partition_key_leaf(); + + AssertSql( + """ +SELECT VALUE c +FROM root c +WHERE ((c["$type"] = "DerivedSinglePartitionKeyEntity") AND ((c["Id"] = "188d3253-81be-4a87-b58f-a2bd07e6b98c") AND (c["PartitionKey"] = "PK2"))) +"""); + } + public override async Task Multiple_incompatible_predicate_comparisons_cause_no_ReadItem_leaf() { await base.Multiple_incompatible_predicate_comparisons_cause_no_ReadItem_leaf(); diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryTest.cs b/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryTest.cs index 6be7c6327a1..c8e6c63e809 100644 --- a/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryTest.cs +++ b/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryTest.cs @@ -361,6 +361,35 @@ public override async Task ReadItem_with_WithPartitionKey_with_only_partition_ke AssertSql("""ReadItem(["PK1a"], PK1a)"""); } + public override async Task WithPartitionKey_and_predicate_with_id_and_conflicting_partition_key() + { + await base.WithPartitionKey_and_predicate_with_id_and_conflicting_partition_key(); + + // Not ReadItem because partition key values were provided both via WithPartitionKey and in the predicate (see #38238). + AssertSql( + """ +SELECT VALUE c +FROM root c +WHERE ((c["id"] = "b29bced8-e1e5-420e-82d7-1c7a51703d34") AND (c["PartitionKey"] = "PK2")) +"""); + } + + public override async Task WithPartitionKey_and_predicate_with_id_two_locals_same_value() + { + await base.WithPartitionKey_and_predicate_with_id_two_locals_same_value(); + + // Not ReadItem because pkForWith and pkForWhere are different parameter expressions (different names), + // so they can't be compared at translate time even though they have the same runtime value. + AssertSql( + """ +@pkForWhere='PK1' + +SELECT VALUE c +FROM root c +WHERE ((c["id"] = "b29bced8-e1e5-420e-82d7-1c7a51703d34") AND (c["PartitionKey"] = @pkForWhere)) +"""); + } + public override async Task Multiple_incompatible_predicate_comparisons_cause_no_ReadItem() { await base.Multiple_incompatible_predicate_comparisons_cause_no_ReadItem(); diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryTestBase.cs b/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryTestBase.cs index c598dcf1a64..9cb0b7a6195 100644 --- a/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryTestBase.cs +++ b/test/EFCore.Cosmos.FunctionalTests/Query/ReadItemPartitionKeyQueryTestBase.cs @@ -292,6 +292,30 @@ public virtual Task ReadItem_with_WithPartitionKey_with_only_partition_key() ss => ss.Set().WithPartitionKey("PK1a").Where(e => e.PartitionKey == "PK1a"), ss => ss.Set().Where(e => e.PartitionKey == "PK1a")); + [Fact] + public virtual Task WithPartitionKey_and_predicate_with_id_and_conflicting_partition_key() + => AssertQuery( + async: true, + ss => ss.Set().WithPartitionKey("PK1") + .Where(e => e.Id == Guid.Parse("B29BCED8-E1E5-420E-82D7-1C7A51703D34") && e.PartitionKey == "PK2"), + ss => ss.Set().Where(e => e.PartitionKey == "PK1") + .Where(e => e.Id == Guid.Parse("B29BCED8-E1E5-420E-82D7-1C7A51703D34") && e.PartitionKey == "PK2"), + assertEmpty: true); + + [Fact] + public virtual Task WithPartitionKey_and_predicate_with_id_two_locals_same_value() + { + var pkForWith = "PK1"; + var pkForWhere = "PK1"; + + return AssertQuery( + async: true, + ss => ss.Set().WithPartitionKey(pkForWith) + .Where(e => e.Id == Guid.Parse("B29BCED8-E1E5-420E-82D7-1C7A51703D34") && e.PartitionKey == pkForWhere), + ss => ss.Set().Where(e => e.PartitionKey == pkForWith) + .Where(e => e.Id == Guid.Parse("B29BCED8-E1E5-420E-82D7-1C7A51703D34") && e.PartitionKey == pkForWhere)); + } + [Fact] public virtual Task Multiple_incompatible_predicate_comparisons_cause_no_ReadItem() {