diff --git a/src/EFCore.Cosmos/Query/Internal/CosmosQueryTranslationPostprocessor.cs b/src/EFCore.Cosmos/Query/Internal/CosmosQueryTranslationPostprocessor.cs index cf11f40219e..156f4898731 100644 --- a/src/EFCore.Cosmos/Query/Internal/CosmosQueryTranslationPostprocessor.cs +++ b/src/EFCore.Cosmos/Query/Internal/CosmosQueryTranslationPostprocessor.cs @@ -27,7 +27,7 @@ public override Expression Process(Expression query) if (query is ShapedQueryExpression { QueryExpression: SelectExpression selectExpression }) { - selectExpression.ApplyProjection(); + selectExpression.ApplyProjection(clientProjection: true); } var afterValueConverterCompensation = new CosmosValueConverterCompensatingExpressionVisitor(sqlExpressionFactory).Visit(query); diff --git a/src/EFCore.Cosmos/Query/Internal/Expressions/SelectExpression.cs b/src/EFCore.Cosmos/Query/Internal/Expressions/SelectExpression.cs index 5f710a622d6..32cb248aa62 100644 --- a/src/EFCore.Cosmos/Query/Internal/Expressions/SelectExpression.cs +++ b/src/EFCore.Cosmos/Query/Internal/Expressions/SelectExpression.cs @@ -246,23 +246,154 @@ ParameterExpression parameterExpression when parameterValues.TryGetValue(paramet /// 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. /// - public void ApplyProjection() + public void ApplyProjection(bool clientProjection = false) { - if (Projection.Any()) + if (!Projection.Any()) { - return; + var result = new Dictionary(); + foreach (var (projectionMember, expression) in _projectionMapping) + { + result[projectionMember] = Constant( + AddToProjection( + expression, + projectionMember.Last?.Name)); + } + + _projectionMapping = result; } - var result = new Dictionary(); - foreach (var (projectionMember, expression) in _projectionMapping) + // A single projection is emitted as a Cosmos VALUE projection (SELECT VALUE c["a"]). When the projected value + // is a scalar nested inside an embedded object (e.g. an owned navigation or complex property), accessing it can + // produce undefined in Cosmos, and a VALUE projection silently filters those documents out. To keep this + // consistent with the multi-projection case (which projects a JSON object and surfaces undefined values - either + // throwing for non-nullable types or yielding null), demote such a projection to an object projection so the + // document is retained, unless the predicate already guarantees the projected path cannot be undefined. + // This is intentionally limited to a scalar whose Object is an ObjectAccessExpression (the scalar lives inside a + // nested embedded object). A scalar accessed directly off the root (Object is an ObjectReferenceExpression, e.g. + // x.Name) cannot be undefined-by-nesting and is left as a VALUE projection. It is also only applied to the + // top-level client projection: subqueries and collection projections rely on VALUE semantics for their shaping. + if (clientProjection + && _projection is [{ IsValueProjection: true, Expression: ScalarAccessExpression { Object: ObjectAccessExpression } scalarAccess } valueProjection] + && !TryMakeNestedScalarProjectionDefined(scalarAccess)) + { + _projection[0] = new ProjectionExpression(valueProjection.Expression, valueProjection.Alias, isValueProjection: false); + } + } + + // Tries to prove, from the predicate, that the nested scalar's access path cannot be undefined for any row that + // passes the filter, so the projection can remain an optimal VALUE projection (which itself filters out undefined). + // When proven, the now-redundant definedness guards (IS_DEFINED(path) or path != null on the scalar or one of its + // ancestor objects) are dropped from the predicate, while value comparisons that merely imply definedness are kept. + // A scalar nested in an embedded object can only be undefined if it, or one of its ancestor objects, is undefined, + // so a guard on any of them is sufficient. Returns false when the predicate doesn't guarantee definedness, in which + // case the caller demotes the projection to an object projection. + private bool TryMakeNestedScalarProjectionDefined(ScalarAccessExpression scalarAccess) + { + if (Predicate is null) + { + return false; + } + + var guaranteed = false; + var removedAny = false; + var retainedConjuncts = new List(); + foreach (var conjunct in GetConjuncts(Predicate)) + { + if (IsRemovableDefinednessGuard(conjunct)) + { + // The guard is made redundant by the VALUE projection (which filters out undefined), so drop it. + guaranteed = true; + removedAny = true; + continue; + } + + guaranteed |= ImpliesDefined(conjunct); + retainedConjuncts.Add(conjunct); + } + + if (!guaranteed) + { + return false; + } + + if (removedAny) + { + Predicate = retainedConjuncts.Count == 0 + ? null + : retainedConjuncts.Aggregate( + (left, right) => (SqlExpression)new SqlBinaryExpression( + ExpressionType.AndAlso, left, right, typeof(bool), right.TypeMapping)); + } + + return true; + + // e.g. IS_DEFINED(c["Associate"]["NestedAssociate"]["Id"]) or c["Associate"]["NestedAssociate"] != null + bool IsRemovableDefinednessGuard(SqlExpression conjunct) + => conjunct switch + { + SqlFunctionExpression { Name: "IS_DEFINED", Arguments: [var argument] } + => OnPath(argument), + SqlBinaryExpression { OperatorType: ExpressionType.NotEqual, Left: var left, Right: var right } + when IsNullConstant(left) || IsNullConstant(right) + => OnPath(IsNullConstant(left) ? right : left), + _ => false + }; + + // A value comparison forces its operands to be defined, since comparing undefined yields undefined (filtered + // out): e.g. c["Associate"]["NestedAssociate"]["Id"] = 1. These are real filters and are kept in the predicate. + bool ImpliesDefined(SqlExpression conjunct) + => conjunct is SqlBinaryExpression + { + OperatorType: ExpressionType.Equal or ExpressionType.NotEqual + or ExpressionType.GreaterThan or ExpressionType.GreaterThanOrEqual + or ExpressionType.LessThan or ExpressionType.LessThanOrEqual, + Left: var left, Right: var right + } + && (OnPath(left) || OnPath(right)); + + bool OnPath(Expression expression) { - result[projectionMember] = Constant( - AddToProjection( - expression, - projectionMember.Last?.Name)); + // A check against a whole structural type (e.g. x.Associate.NestedAssociate != null) is represented as a + // StructuralTypeProjectionExpression over the embedded object's access expression. + var access = expression is StructuralTypeProjectionExpression structuralProjection + ? structuralProjection.Object + : expression; + + for (Expression? current = scalarAccess; current is ScalarAccessExpression or ObjectAccessExpression;) + { + if (access.Equals(current)) + { + return true; + } + + current = current is ScalarAccessExpression scalar ? scalar.Object : ((ObjectAccessExpression)current).Object; + } + + return false; } - _projectionMapping = result; + static bool IsNullConstant(Expression expression) + => expression is SqlConstantExpression { Value: null }; + } + + private static IEnumerable GetConjuncts(SqlExpression predicate) + { + if (predicate is SqlBinaryExpression { OperatorType: ExpressionType.AndAlso, Left: SqlExpression left, Right: SqlExpression right }) + { + foreach (var conjunct in GetConjuncts(left)) + { + yield return conjunct; + } + + foreach (var conjunct in GetConjuncts(right)) + { + yield return conjunct; + } + } + else + { + yield return predicate; + } } /// diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/Associations/ComplexProperties/ComplexPropertiesProjectionCosmosTest.cs b/test/EFCore.Cosmos.FunctionalTests/Query/Associations/ComplexProperties/ComplexPropertiesProjectionCosmosTest.cs index 89d91a690ae..142b0041ac6 100644 --- a/test/EFCore.Cosmos.FunctionalTests/Query/Associations/ComplexProperties/ComplexPropertiesProjectionCosmosTest.cs +++ b/test/EFCore.Cosmos.FunctionalTests/Query/Associations/ComplexProperties/ComplexPropertiesProjectionCosmosTest.cs @@ -30,55 +30,84 @@ public override async Task Select_scalar_property_on_required_associate(QueryTra AssertSql( """ -SELECT VALUE c["RequiredAssociate"]["String"] +SELECT c["RequiredAssociate"]["String"] FROM root c """); } public override async Task Select_property_on_optional_associate(QueryTrackingBehavior queryTrackingBehavior) { - // When OptionalAssociate is null, the property access on it evaluates to undefined in Cosmos, causing the - // result to be filtered out entirely. - await AssertQuery( - ss => ss.Set().Select(x => x.OptionalAssociate!.String), - ss => ss.Set().Where(x => x.OptionalAssociate != null).Select(x => x.OptionalAssociate!.String), - queryTrackingBehavior: queryTrackingBehavior); + // A single property projection is emitted as an object projection (without VALUE) so that documents where the + // property access evaluates to undefined (e.g. OptionalAssociate is null) are retained rather than filtered out. + await base.Select_property_on_optional_associate(queryTrackingBehavior); AssertSql( """ -SELECT VALUE c["OptionalAssociate"]["String"] +SELECT c["OptionalAssociate"]["String"] FROM root c """); } public override async Task Select_value_type_property_on_null_associate_throws(QueryTrackingBehavior queryTrackingBehavior) { - // When OptionalAssociate is null, the property access on it evaluates to undefined in Cosmos, causing the - // result to be filtered out entirely. - await AssertQuery( - ss => ss.Set().Select(x => x.OptionalAssociate!.Int), - ss => ss.Set().Where(x => x.OptionalAssociate != null).Select(x => x.OptionalAssociate!.Int), - queryTrackingBehavior: queryTrackingBehavior); + // A single property projection is emitted as an object projection (without VALUE), so a value type property + // accessed on a null OptionalAssociate surfaces as undefined and throws in the shaper, just like the + // multi-property case. + await base.Select_value_type_property_on_null_associate_throws(queryTrackingBehavior); AssertSql( """ -SELECT VALUE c["OptionalAssociate"]["Int"] +SELECT c["OptionalAssociate"]["Int"] FROM root c """); } public override async Task Select_nullable_value_type_property_on_null_associate(QueryTrackingBehavior queryTrackingBehavior) { - // When OptionalAssociate is null, the property access on it evaluates to undefined in Cosmos, causing the - // result to be filtered out entirely. + // A single property projection is emitted as an object projection (without VALUE) so that documents where the + // property access evaluates to undefined (e.g. OptionalAssociate is null) are retained rather than filtered out. + await base.Select_nullable_value_type_property_on_null_associate(queryTrackingBehavior); + + AssertSql( + """ +SELECT c["OptionalAssociate"]["Int"] +FROM root c +"""); + } + + [Fact] + public virtual async Task Select_nested_scalar_guarded_by_navigation_predicate_uses_VALUE() + { + // The predicate only guards the navigation path's definedness, which the VALUE projection already enforces, + // so the redundant guards are dropped and the optimal VALUE projection is used. + await AssertQuery( + ss => ss.Set() + .Where(x => x.OptionalAssociate != null && x.OptionalAssociate.OptionalNestedAssociate != null) + .Select(x => x.OptionalAssociate!.OptionalNestedAssociate!.Int)); + + AssertSql( + """ +SELECT VALUE c["OptionalAssociate"]["OptionalNestedAssociate"]["Int"] +FROM root c +"""); + } + + [Fact] + public virtual async Task Select_nested_scalar_guarded_by_IsDefined_uses_VALUE() + { + // The IS_DEFINED guard only ensures the projected path is defined, which the VALUE projection already + // enforces, so the redundant guard is dropped and the optimal VALUE projection is used. await AssertQuery( - ss => ss.Set().Select(x => (int?)x.OptionalAssociate!.Int), - ss => ss.Set().Where(x => x.OptionalAssociate != null).Select(x => (int?)x.OptionalAssociate!.Int), - queryTrackingBehavior: queryTrackingBehavior); + ss => ss.Set() + .Where(x => EF.Functions.IsDefined(x.OptionalAssociate!.OptionalNestedAssociate!.Int)) + .Select(x => x.OptionalAssociate!.OptionalNestedAssociate!.Int), + ss => ss.Set() + .Where(x => x.OptionalAssociate != null && x.OptionalAssociate.OptionalNestedAssociate != null) + .Select(x => x.OptionalAssociate!.OptionalNestedAssociate!.Int)); AssertSql( """ -SELECT VALUE c["OptionalAssociate"]["Int"] +SELECT VALUE c["OptionalAssociate"]["OptionalNestedAssociate"]["Int"] FROM root c """); } @@ -187,7 +216,7 @@ public override async Task Select_untranslatable_method_on_associate_scalar_prop AssertSql( """ -SELECT VALUE c["RequiredAssociate"]["Int"] +SELECT c["RequiredAssociate"]["Int"] FROM root c """); } diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/Associations/OwnedNavigations/OwnedNavigationsProjectionCosmosTest.cs b/test/EFCore.Cosmos.FunctionalTests/Query/Associations/OwnedNavigations/OwnedNavigationsProjectionCosmosTest.cs index 408f651a6e8..a43de1e602a 100644 --- a/test/EFCore.Cosmos.FunctionalTests/Query/Associations/OwnedNavigations/OwnedNavigationsProjectionCosmosTest.cs +++ b/test/EFCore.Cosmos.FunctionalTests/Query/Associations/OwnedNavigations/OwnedNavigationsProjectionCosmosTest.cs @@ -31,55 +31,84 @@ public override async Task Select_scalar_property_on_required_associate(QueryTra AssertSql( """ -SELECT VALUE c["RequiredAssociate"]["String"] +SELECT c["RequiredAssociate"]["String"] FROM root c """); } public override async Task Select_property_on_optional_associate(QueryTrackingBehavior queryTrackingBehavior) { - // When OptionalAssociate is null, the property access on it evaluates to undefined in Cosmos, causing the - // result to be filtered out entirely. - await AssertQuery( - ss => ss.Set().Select(x => x.OptionalAssociate!.String), - ss => ss.Set().Where(x => x.OptionalAssociate != null).Select(x => x.OptionalAssociate!.String), - queryTrackingBehavior: queryTrackingBehavior); + // A single property projection is emitted as an object projection (without VALUE) so that documents where the + // property access evaluates to undefined (e.g. OptionalAssociate is null) are retained rather than filtered out. + await base.Select_property_on_optional_associate(queryTrackingBehavior); AssertSql( """ -SELECT VALUE c["OptionalAssociate"]["String"] +SELECT c["OptionalAssociate"]["String"] FROM root c """); } public override async Task Select_value_type_property_on_null_associate_throws(QueryTrackingBehavior queryTrackingBehavior) { - // When OptionalAssociate is null, the property access on it evaluates to undefined in Cosmos, causing the - // result to be filtered out entirely. - await AssertQuery( - ss => ss.Set().Select(x => x.OptionalAssociate!.Int), - ss => ss.Set().Where(x => x.OptionalAssociate != null).Select(x => x.OptionalAssociate!.Int), - queryTrackingBehavior: queryTrackingBehavior); + // A single property projection is emitted as an object projection (without VALUE), so a value type property + // accessed on a null OptionalAssociate surfaces as undefined and throws in the shaper, just like the + // multi-property case. + await base.Select_value_type_property_on_null_associate_throws(queryTrackingBehavior); AssertSql( """ -SELECT VALUE c["OptionalAssociate"]["Int"] +SELECT c["OptionalAssociate"]["Int"] FROM root c """); } public override async Task Select_nullable_value_type_property_on_null_associate(QueryTrackingBehavior queryTrackingBehavior) { - // When OptionalAssociate is null, the property access on it evaluates to undefined in Cosmos, causing the - // result to be filtered out entirely. + // A single property projection is emitted as an object projection (without VALUE) so that documents where the + // property access evaluates to undefined (e.g. OptionalAssociate is null) are retained rather than filtered out. + await base.Select_nullable_value_type_property_on_null_associate(queryTrackingBehavior); + + AssertSql( + """ +SELECT c["OptionalAssociate"]["Int"] +FROM root c +"""); + } + + [Fact] + public virtual async Task Select_nested_scalar_guarded_by_navigation_predicate_uses_VALUE() + { + // The predicate only guards the navigation path's definedness, which the VALUE projection already enforces, + // so the redundant guards are dropped and the optimal VALUE projection is used. + await AssertQuery( + ss => ss.Set() + .Where(x => x.OptionalAssociate != null && x.OptionalAssociate.OptionalNestedAssociate != null) + .Select(x => x.OptionalAssociate!.OptionalNestedAssociate!.Int)); + + AssertSql( + """ +SELECT VALUE c["OptionalAssociate"]["OptionalNestedAssociate"]["Int"] +FROM root c +"""); + } + + [Fact] + public virtual async Task Select_nested_scalar_guarded_by_IsDefined_uses_VALUE() + { + // The IS_DEFINED guard only ensures the projected path is defined, which the VALUE projection already + // enforces, so the redundant guard is dropped and the optimal VALUE projection is used. await AssertQuery( - ss => ss.Set().Select(x => (int?)x.OptionalAssociate!.Int), - ss => ss.Set().Where(x => x.OptionalAssociate != null).Select(x => (int?)x.OptionalAssociate!.Int), - queryTrackingBehavior: queryTrackingBehavior); + ss => ss.Set() + .Where(x => EF.Functions.IsDefined(x.OptionalAssociate!.OptionalNestedAssociate!.Int)) + .Select(x => x.OptionalAssociate!.OptionalNestedAssociate!.Int), + ss => ss.Set() + .Where(x => x.OptionalAssociate != null && x.OptionalAssociate.OptionalNestedAssociate != null) + .Select(x => x.OptionalAssociate!.OptionalNestedAssociate!.Int)); AssertSql( """ -SELECT VALUE c["OptionalAssociate"]["Int"] +SELECT VALUE c["OptionalAssociate"]["OptionalNestedAssociate"]["Int"] FROM root c """); } @@ -220,7 +249,7 @@ public override async Task Select_untranslatable_method_on_associate_scalar_prop AssertSql( """ -SELECT VALUE c["RequiredAssociate"]["Int"] +SELECT c["RequiredAssociate"]["Int"] FROM root c """); } diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/ComplexTypeQueryCosmosTest.cs b/test/EFCore.Cosmos.FunctionalTests/Query/ComplexTypeQueryCosmosTest.cs index 5552303dac9..74a150e866a 100644 --- a/test/EFCore.Cosmos.FunctionalTests/Query/ComplexTypeQueryCosmosTest.cs +++ b/test/EFCore.Cosmos.FunctionalTests/Query/ComplexTypeQueryCosmosTest.cs @@ -60,7 +60,7 @@ public override Task Select_single_property_on_nested_complex_type(bool async) AssertSql( """ -SELECT VALUE c["ShippingAddress"]["Country"]["FullName"] +SELECT c["ShippingAddress"]["Country"]["FullName"] FROM root c """); }); @@ -239,7 +239,7 @@ public override Task Select_single_property_on_nested_struct_complex_type(bool a AssertSql( """ -SELECT VALUE c["ShippingAddress"]["Country"]["FullName"] +SELECT c["ShippingAddress"]["Country"]["FullName"] FROM root c """); }); diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/ComplexTypeToJsonPropertyQueryCosmosTest.cs b/test/EFCore.Cosmos.FunctionalTests/Query/ComplexTypeToJsonPropertyQueryCosmosTest.cs index 258ba5cbae7..31377b087c4 100644 --- a/test/EFCore.Cosmos.FunctionalTests/Query/ComplexTypeToJsonPropertyQueryCosmosTest.cs +++ b/test/EFCore.Cosmos.FunctionalTests/Query/ComplexTypeToJsonPropertyQueryCosmosTest.cs @@ -60,7 +60,7 @@ public override Task Select_single_property_on_nested_complex_type(bool async) AssertSql( """ -SELECT VALUE c["ShippingAddressRenamed"]["CountryRenamed"]["FullNameRenamed"] +SELECT c["ShippingAddressRenamed"]["CountryRenamed"]["FullNameRenamed"] FROM root c """); }); @@ -239,7 +239,7 @@ public override Task Select_single_property_on_nested_struct_complex_type(bool a AssertSql( """ -SELECT VALUE c["ShippingAddressRenamed"]["CountryRenamed"]["FullNameRenamed"] +SELECT c["ShippingAddressRenamed"]["CountryRenamed"]["FullNameRenamed"] FROM root c """); }); diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/JsonQueryCosmosTest.cs b/test/EFCore.Cosmos.FunctionalTests/Query/JsonQueryCosmosTest.cs index 37a00de8df6..203a214f086 100644 --- a/test/EFCore.Cosmos.FunctionalTests/Query/JsonQueryCosmosTest.cs +++ b/test/EFCore.Cosmos.FunctionalTests/Query/JsonQueryCosmosTest.cs @@ -191,7 +191,7 @@ public override Task Basic_json_projection_scalar(bool async) AssertSql( """ -SELECT VALUE c["OwnedReferenceRoot"]["Name"] +SELECT c["OwnedReferenceRoot"]["Name"] FROM root c WHERE (c["Discriminator"] = "Basic") """); @@ -245,7 +245,7 @@ public override Task Custom_naming_projection_owned_scalar(bool async) AssertSql( """ -SELECT VALUE c["OwnedReferenceRoot"]["OwnedReferenceBranch"]["Fraction"] +SELECT c["OwnedReferenceRoot"]["OwnedReferenceBranch"]["Fraction"] FROM root c WHERE (c["Discriminator"] = "CustomNaming") """); @@ -386,7 +386,7 @@ public override Task Json_boolean_projection(bool async) AssertSql( """ -SELECT VALUE c["Reference"]["TestBoolean"] +SELECT c["Reference"]["TestBoolean"] FROM root c WHERE (c["Discriminator"] = "AllTypes") """); diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/OwnedQueryCosmosTest.cs b/test/EFCore.Cosmos.FunctionalTests/Query/OwnedQueryCosmosTest.cs index 814137a29ee..2ddefdf8003 100644 --- a/test/EFCore.Cosmos.FunctionalTests/Query/OwnedQueryCosmosTest.cs +++ b/test/EFCore.Cosmos.FunctionalTests/Query/OwnedQueryCosmosTest.cs @@ -648,7 +648,7 @@ public override Task Can_project_owned_indexer_properties(bool async) AssertSql( """ -SELECT VALUE c["PersonAddress"]["AddressLine"] +SELECT c["PersonAddress"]["AddressLine"] FROM root c WHERE c["Terminator"] IN ("OwnedPerson", "Branch", "LeafB", "LeafA") """); @@ -792,7 +792,7 @@ public override Task Projecting_indexer_property_ignores_include(bool async) AssertSql( """ -SELECT VALUE c["PersonAddress"]["ZipCode"] +SELECT c["PersonAddress"]["ZipCode"] AS Nation FROM root c WHERE c["Terminator"] IN ("OwnedPerson", "Branch", "LeafB", "LeafA") """); @@ -806,7 +806,7 @@ public override Task Projecting_indexer_property_ignores_include_converted(bool AssertSql( """ -SELECT VALUE c["PersonAddress"]["ZipCode"] +SELECT c["PersonAddress"]["ZipCode"] AS Nation FROM root c WHERE c["Terminator"] IN ("OwnedPerson", "Branch", "LeafB", "LeafA") """); @@ -976,7 +976,7 @@ public override Task Can_project_owned_indexer_properties_converted(bool async) AssertSql( """ -SELECT VALUE c["PersonAddress"]["AddressLine"] +SELECT c["PersonAddress"]["AddressLine"] FROM root c WHERE c["Terminator"] IN ("OwnedPerson", "Branch", "LeafB", "LeafA") """); @@ -1042,13 +1042,11 @@ public override async Task Non_nullable_property_through_optional_navigation(boo await CosmosTestHelpers.Instance.NoSyncTest( async, async a => { - await Assert.ThrowsAsync(() => AssertQuery( - async, - ss => ss.Set().Select(e => new { e.Throned.Value }))); + await base.Non_nullable_property_through_optional_navigation(a); AssertSql( """ -SELECT VALUE c["Throned"]["Value"] +SELECT c["Throned"]["Value"] FROM root c WHERE (c["Terminator"] = "Barton") """);