diff --git a/src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs b/src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs index 3f826412a13..a23361e96d3 100644 --- a/src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs +++ b/src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs @@ -1521,7 +1521,7 @@ private bool TryApplyPredicate(ShapedQueryExpression source, LambdaExpression pr if (TranslateLambdaExpression(source, predicate) is { } translation) { - if (translation is not SqlConstantExpression { Value: true }) + if (translation is not SqlConstantExpression { Value: true } && translation is not SqlUnaryExpression { OperatorType: ExpressionType.Not, Operand: SqlConstantExpression { Value: false } }) { select.ApplyPredicate(translation); } diff --git a/src/EFCore.Cosmos/Query/Internal/CosmosSqlTranslatingExpressionVisitor.cs b/src/EFCore.Cosmos/Query/Internal/CosmosSqlTranslatingExpressionVisitor.cs index f963e03ee06..41880efc125 100644 --- a/src/EFCore.Cosmos/Query/Internal/CosmosSqlTranslatingExpressionVisitor.cs +++ b/src/EFCore.Cosmos/Query/Internal/CosmosSqlTranslatingExpressionVisitor.cs @@ -4,6 +4,8 @@ using System.Collections; using System.Diagnostics.CodeAnalysis; using Microsoft.EntityFrameworkCore.Cosmos.Internal; +using Microsoft.EntityFrameworkCore.Cosmos.Metadata.Internal; +using Microsoft.EntityFrameworkCore.Cosmos.Query.Internal.Expressions; using Microsoft.EntityFrameworkCore.Internal; using static Microsoft.EntityFrameworkCore.Infrastructure.ExpressionExtensions; @@ -26,8 +28,8 @@ public class CosmosSqlTranslatingExpressionVisitor( { private const string RuntimeParameterPrefix = "entity_equality_"; - private static readonly MethodInfo ParameterValueExtractorMethod = - typeof(CosmosSqlTranslatingExpressionVisitor).GetTypeInfo().GetDeclaredMethod(nameof(ParameterValueExtractor))!; + private static readonly MethodInfo ParameterPropertyValueExtractorMethod = + typeof(CosmosSqlTranslatingExpressionVisitor).GetTypeInfo().GetDeclaredMethod(nameof(ParameterPropertyValueExtractor))!; private static readonly MethodInfo ParameterListValueExtractorMethod = typeof(CosmosSqlTranslatingExpressionVisitor).GetTypeInfo().GetDeclaredMethod(nameof(ParameterListValueExtractor))!; @@ -1065,69 +1067,52 @@ private bool TryRewriteEntityEquality( bool equalsMethod, [NotNullWhen(true)] out Expression? result) { - var leftEntityReference = left as EntityReferenceExpression; - var rightEntityReference = right as EntityReferenceExpression; - - if (leftEntityReference == null - && rightEntityReference == null) + var entityReference = left as EntityReferenceExpression ?? right as EntityReferenceExpression; + if (entityReference == null) { result = null; return false; } - if (IsNullSqlConstantExpression(left) - || IsNullSqlConstantExpression(right)) + var entityType = entityReference.EntityType; + var compareReference = entityReference == left ? right : left; + + // Null equality + if (IsNullSqlConstantExpression(compareReference)) { - var nonNullEntityReference = (IsNullSqlConstantExpression(left) ? rightEntityReference : leftEntityReference)!; - var entityType1 = nonNullEntityReference.EntityType; - var primaryKeyProperties1 = entityType1.FindPrimaryKey()?.Properties; - if (primaryKeyProperties1 == null) + if (entityType.IsDocumentRoot() && entityReference.Subquery == null) { - throw new InvalidOperationException( - CoreStrings.EntityEqualityOnKeylessEntityNotSupported( - nodeType == ExpressionType.Equal - ? equalsMethod ? nameof(object.Equals) : "==" - : equalsMethod - ? "!" + nameof(object.Equals) - : "!=", - entityType1.DisplayName())); + // Document root can never be be null + result = Visit(Expression.Constant(nodeType != ExpressionType.Equal)); + return true; } - result = Visit( - primaryKeyProperties1.Select(p => - Expression.MakeBinary( - nodeType, CreatePropertyAccessExpression(nonNullEntityReference, p), - Expression.Constant(null, p.ClrType.MakeNullable()))) - .Aggregate((l, r) => nodeType == ExpressionType.Equal ? Expression.OrElse(l, r) : Expression.AndAlso(l, r))); - + // Treat type as object for null comparison + var access = new SqlObjectAccessExpression(entityReference.Object); + result = sqlExpressionFactory.MakeBinary(nodeType, access, sqlExpressionFactory.Constant(null, typeof(object))!, typeMappingSource.FindMapping(typeof(bool)))!; return true; } - var leftEntityType = leftEntityReference?.EntityType; - var rightEntityType = rightEntityReference?.EntityType; - var entityType = leftEntityType ?? rightEntityType; - - Check.DebugAssert(entityType != null, "At least either side should be entityReference so entityType should be non-null."); - - if (leftEntityType != null - && rightEntityType != null - && leftEntityType.GetRootType() != rightEntityType.GetRootType()) + if (entityType.FindPrimaryKey()?.Properties is not { } primaryKeyProperties) { - result = sqlExpressionFactory.Constant(false); - return true; + throw new InvalidOperationException( + CoreStrings.EntityEqualityOnKeylessEntityNotSupported( + nodeType == ExpressionType.Equal + ? equalsMethod ? nameof(object.Equals) : "==" + : equalsMethod + ? "!" + nameof(object.Equals) + : "!=", + entityType.DisplayName())); } - var primaryKeyProperties = entityType.FindPrimaryKey()?.Properties; - if (primaryKeyProperties == null) + if (compareReference is EntityReferenceExpression compareEntityReference) { - throw new InvalidOperationException( - CoreStrings.EntityEqualityOnKeylessEntityNotSupported( - nodeType == ExpressionType.Equal - ? equalsMethod ? nameof(object.Equals) : "==" - : equalsMethod - ? "!" + nameof(object.Equals) - : "!=", - entityType.DisplayName())); + // Comparing of 2 different entity types is always false. + if (entityType.GetRootType() != compareEntityReference.EntityType.GetRootType()) + { + result = Visit(Expression.Constant(false)); + return true; + } } result = Visit( @@ -1154,7 +1139,7 @@ private Expression CreatePropertyAccessExpression(Expression target, IProperty p case SqlParameterExpression sqlParameterExpression: var lambda = Expression.Lambda( Expression.Call( - ParameterValueExtractorMethod.MakeGenericMethod(property.ClrType.MakeNullable()), + ParameterPropertyValueExtractorMethod.MakeGenericMethod(property.ClrType.MakeNullable()), QueryCompilationContext.QueryContextParameter, Expression.Constant(sqlParameterExpression.Name, typeof(string)), Expression.Constant(property, typeof(IProperty))), @@ -1174,7 +1159,7 @@ when memberInitExpression.Bindings.SingleOrDefault(mb => mb.Member.Name == prope } } - private static T? ParameterValueExtractor(QueryContext context, string baseParameterName, IProperty property) + private static T? ParameterPropertyValueExtractor(QueryContext context, string baseParameterName, IProperty property) { var baseParameter = context.Parameters[baseParameterName]; return baseParameter == null ? (T?)(object?)null : (T?)property.GetGetter().GetClrValue(baseParameter); @@ -1262,6 +1247,7 @@ private EntityReferenceExpression(EntityReferenceExpression typeReference, IType EntityType = (IEntityType)structuralType; } + public Expression Object => (Expression?)Parameter ?? Subquery ?? throw new UnreachableException(); public new StructuralTypeShaperExpression? Parameter { get; } public ShapedQueryExpression? Subquery { get; } public IEntityType EntityType { get; } diff --git a/src/EFCore.Cosmos/Query/Internal/Expressions/SqlObjectAccessExpression.cs b/src/EFCore.Cosmos/Query/Internal/Expressions/SqlObjectAccessExpression.cs new file mode 100644 index 00000000000..4fe2f8c54ac --- /dev/null +++ b/src/EFCore.Cosmos/Query/Internal/Expressions/SqlObjectAccessExpression.cs @@ -0,0 +1,90 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.EntityFrameworkCore.Cosmos.Storage.Internal; + +namespace Microsoft.EntityFrameworkCore.Cosmos.Query.Internal.Expressions; + +/// +/// Represents an structural type object access on a CosmosJSON object +/// +/// +/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to +/// the same compatibility standards as public APIs. It may be changed or removed without notice in +/// 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. +/// +[DebuggerDisplay("{Microsoft.EntityFrameworkCore.Query.ExpressionPrinter.Print(this), nq}")] +public class SqlObjectAccessExpression(Expression @object) + : SqlExpression(typeof(object), CosmosTypeMapping.Default), IAccessExpression +{ + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// 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 virtual Expression Object { get; } = @object; + + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// 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 virtual string? PropertyName => null; + + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// 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. + /// + protected override Expression VisitChildren(ExpressionVisitor visitor) + => Update(visitor.Visit(Object)); + + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// 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 virtual SqlObjectAccessExpression Update(Expression @object) + => ReferenceEquals(@object, Object) + ? this + : new SqlObjectAccessExpression(@object); + + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// 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. + /// + protected override void Print(ExpressionPrinter expressionPrinter) + => expressionPrinter.Visit(Object); + + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// 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 override bool Equals(object? obj) + => obj != null + && (ReferenceEquals(this, obj) + || obj is SqlObjectAccessExpression expression + && Equals(expression)); + + private bool Equals(SqlObjectAccessExpression expression) + => base.Equals(expression) + && Object.Equals(expression.Object); + + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// 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 override int GetHashCode() + => HashCode.Combine(base.GetHashCode(), Object); +} diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/Associations/OwnedNavigations/OwnedNavigationsStructuralEqualityCosmosTest.cs b/test/EFCore.Cosmos.FunctionalTests/Query/Associations/OwnedNavigations/OwnedNavigationsStructuralEqualityCosmosTest.cs index b19b56da69b..bed53ec3287 100644 --- a/test/EFCore.Cosmos.FunctionalTests/Query/Associations/OwnedNavigations/OwnedNavigationsStructuralEqualityCosmosTest.cs +++ b/test/EFCore.Cosmos.FunctionalTests/Query/Associations/OwnedNavigations/OwnedNavigationsStructuralEqualityCosmosTest.cs @@ -48,14 +48,32 @@ WHERE false """); } - public override Task Associate_with_inline_null() - => Assert.ThrowsAsync(() => base.Associate_with_inline_null()); + public override async Task Associate_with_inline_null() + { + await base.Associate_with_inline_null(); + + AssertSql( + """ +SELECT VALUE c +FROM root c +WHERE (c["OptionalAssociate"] = null) +"""); + } public override Task Associate_with_parameter_null() => Assert.ThrowsAsync(() => base.Associate_with_parameter_null()); - public override Task Nested_associate_with_inline_null() - => Assert.ThrowsAsync(() => base.Nested_associate_with_inline_null()); + public override async Task Nested_associate_with_inline_null() + { + await base.Nested_associate_with_inline_null(); + + AssertSql( + """ +SELECT VALUE c +FROM root c +WHERE (c["RequiredAssociate"]["OptionalNestedAssociate"] = null) +"""); + } public override async Task Nested_associate_with_inline() { diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindMiscellaneousQueryCosmosTest.cs b/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindMiscellaneousQueryCosmosTest.cs index 9f732656d56..07f8a382dd7 100644 --- a/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindMiscellaneousQueryCosmosTest.cs +++ b/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindMiscellaneousQueryCosmosTest.cs @@ -167,7 +167,7 @@ public override Task Entity_equality_null(bool async) """ SELECT VALUE c["id"] FROM root c -WHERE (c["id"] = null) +WHERE false """); }); @@ -181,7 +181,6 @@ public override Task Entity_equality_not_null(bool async) """ SELECT VALUE c["id"] FROM root c -WHERE (c["id"] != null) """); }); @@ -2883,7 +2882,7 @@ public override Task Comparing_entity_to_null_using_Equals(bool async) """ SELECT VALUE c["id"] FROM root c -WHERE (STARTSWITH(c["id"], "A") AND NOT((c["id"] = null))) +WHERE STARTSWITH(c["id"], "A") ORDER BY c["id"] """); }); @@ -2929,7 +2928,7 @@ public override Task Comparing_collection_navigation_to_null(bool async) """ SELECT VALUE c["id"] FROM root c -WHERE (c["id"] = null) +WHERE false """); }); @@ -4004,7 +4003,7 @@ public override Task Entity_equality_through_include(bool async) """ SELECT VALUE c["id"] FROM root c -WHERE (c["id"] = null) +WHERE false """); }); @@ -4113,7 +4112,7 @@ public override Task Entity_equality_not_null_composite_key(bool async) """ SELECT VALUE c FROM root c -WHERE ((c["$type"] = "OrderDetail") AND ((c["OrderID"] != null) AND (c["ProductID"] != null))) +WHERE (c["$type"] = "OrderDetail") """); }); @@ -4183,7 +4182,12 @@ public override Task Null_parameter_name_works(bool async) { await base.Null_parameter_name_works(a); - AssertSql("ReadItem(None, null)"); + AssertSql( + """ +SELECT VALUE c +FROM root c +WHERE false +"""); }); public override Task Where_Property_shadow_closure(bool async) @@ -4276,7 +4280,7 @@ public override Task Entity_equality_null_composite_key(bool async) """ SELECT VALUE c FROM root c -WHERE ((c["$type"] = "OrderDetail") AND ((c["OrderID"] = null) OR (c["ProductID"] = null))) +WHERE ((c["$type"] = "OrderDetail") AND false) """); });