Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -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<IProperty, Expression?> _jsonIdPropertyValues = null!;
Expand Down Expand Up @@ -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 ?? [];
Expand Down Expand Up @@ -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;
}
}
}
Comment thread
AndriySvyryd marked this conversation as resolved.

foreach (var property in partitionKeyProperties)
{
if (liftPartitionKeys && _partitionKeyPropertyValues[property].ValueExpression is { } valueExpression)
Expand All @@ -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
{
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,16 @@ public virtual Task ReadItem_with_WithPartitionKey_with_only_partition_key_leaf(
ss => ss.Set<DerivedOnlySinglePartitionKeyEntity>().WithPartitionKey("PK1c").Where(e => e.PartitionKey == "PK1c"),
ss => ss.Set<DerivedOnlySinglePartitionKeyEntity>().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<DerivedSinglePartitionKeyEntity>().WithPartitionKey("PK1")
.Where(e => e.Id == Guid.Parse("188D3253-81BE-4A87-B58F-A2BD07E6B98C") && e.PartitionKey == "PK2"),
ss => ss.Set<DerivedSinglePartitionKeyEntity>().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()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Comment thread
AndriySvyryd marked this conversation as resolved.
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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,30 @@ public virtual Task ReadItem_with_WithPartitionKey_with_only_partition_key()
ss => ss.Set<OnlySinglePartitionKeyEntity>().WithPartitionKey("PK1a").Where(e => e.PartitionKey == "PK1a"),
ss => ss.Set<OnlySinglePartitionKeyEntity>().Where(e => e.PartitionKey == "PK1a"));

[Fact]
public virtual Task WithPartitionKey_and_predicate_with_id_and_conflicting_partition_key()
=> AssertQuery(
async: true,
ss => ss.Set<SinglePartitionKeyEntity>().WithPartitionKey("PK1")
.Where(e => e.Id == Guid.Parse("B29BCED8-E1E5-420E-82D7-1C7A51703D34") && e.PartitionKey == "PK2"),
ss => ss.Set<SinglePartitionKeyEntity>().Where(e => e.PartitionKey == "PK1")
.Where(e => e.Id == Guid.Parse("B29BCED8-E1E5-420E-82D7-1C7A51703D34") && e.PartitionKey == "PK2"),
assertEmpty: true);

Comment thread
AndriySvyryd marked this conversation as resolved.
[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<SinglePartitionKeyEntity>().WithPartitionKey(pkForWith)
.Where(e => e.Id == Guid.Parse("B29BCED8-E1E5-420E-82D7-1C7A51703D34") && e.PartitionKey == pkForWhere),
ss => ss.Set<SinglePartitionKeyEntity>().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()
{
Expand Down
Loading