From 3a23a8675e761a1707e21cf6408a1f795e37d360 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 16 Oct 2025 07:19:04 +0000 Subject: [PATCH 1/4] Initial plan From b9a972256c542b3a9136f875d7db9b4f1798141f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 16 Oct 2025 07:38:14 +0000 Subject: [PATCH 2/4] Fix null check optimization for IQueryable/DbSet types Co-authored-by: cincuranet <4540597+cincuranet@users.noreply.github.com> --- .../NullCheckRemovingExpressionVisitor.cs | 45 ++++++++++++++++++- .../Query/NorthwindWhereQueryTestBase.cs | 12 +++++ 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/src/EFCore/Query/Internal/NullCheckRemovingExpressionVisitor.cs b/src/EFCore/Query/Internal/NullCheckRemovingExpressionVisitor.cs index 8482d63a307..822ada647bc 100644 --- a/src/EFCore/Query/Internal/NullCheckRemovingExpressionVisitor.cs +++ b/src/EFCore/Query/Internal/NullCheckRemovingExpressionVisitor.cs @@ -24,7 +24,9 @@ protected override Expression VisitBinary(BinaryExpression binaryExpression) { var visitedExpression = base.VisitBinary(binaryExpression); - return TryOptimizeConditionalEquality(visitedExpression) ?? visitedExpression; + return TryOptimizeQueryableNullCheck(visitedExpression) + ?? TryOptimizeConditionalEquality(visitedExpression) + ?? visitedExpression; } /// @@ -75,6 +77,34 @@ protected override Expression VisitConditional(ConditionalExpression conditional return base.VisitConditional(conditionalExpression); } + private static Expression? TryOptimizeQueryableNullCheck(Expression expression) + { + // Optimize IQueryable/DbSet null checks + // IQueryable != null => true + // IQueryable == null => false + if (expression is BinaryExpression + { + NodeType: ExpressionType.Equal or ExpressionType.NotEqual + } binaryExpression) + { + var isLeftNull = IsNullConstant(binaryExpression.Left); + var isRightNull = IsNullConstant(binaryExpression.Right); + + if (isLeftNull != isRightNull) + { + var nonNullExpression = isLeftNull ? binaryExpression.Right : binaryExpression.Left; + + if (IsQueryableType(nonNullExpression.Type)) + { + var result = binaryExpression.NodeType == ExpressionType.NotEqual; + return Expression.Constant(result, typeof(bool)); + } + } + } + + return null; + } + private static Expression? TryOptimizeConditionalEquality(Expression expression) { // Simplify (a ? b : null) == null => !a || b == null @@ -161,4 +191,17 @@ protected override Expression VisitUnary(UnaryExpression unaryExpression) private static bool IsNullConstant(Expression expression) => expression is ConstantExpression { Value: null }; + + private static bool IsQueryableType(Type type) + { + if (type.IsGenericType) + { + var genericTypeDefinition = type.GetGenericTypeDefinition(); + return genericTypeDefinition == typeof(IQueryable<>) + || genericTypeDefinition == typeof(IOrderedQueryable<>) + || genericTypeDefinition == typeof(DbSet<>); + } + + return type == typeof(IQueryable); + } } diff --git a/test/EFCore.Specification.Tests/Query/NorthwindWhereQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/NorthwindWhereQueryTestBase.cs index e4beae2c27e..120dbe03414 100644 --- a/test/EFCore.Specification.Tests/Query/NorthwindWhereQueryTestBase.cs +++ b/test/EFCore.Specification.Tests/Query/NorthwindWhereQueryTestBase.cs @@ -1418,6 +1418,18 @@ public virtual Task Where_Queryable_AsEnumerable_Contains_negated(bool async) elementSorter: e => e.CustomerID, elementAsserter: (e, a) => AssertCollection(e.Subquery, a.Subquery)); + [ConditionalTheory, MemberData(nameof(IsAsyncData))] + public virtual Task Where_Queryable_null_check_with_Contains(bool async) + { + return AssertQuery( + async, + ss => + { + var ids = ss.Set().Select(c => c.CustomerID); + return ss.Set().Where(c => ids != null && ids.Contains(c.CustomerID)); + }); + } + [ConditionalTheory, MemberData(nameof(IsAsyncData))] public virtual Task Where_Queryable_ToList_Count_member(bool async) => AssertQuery( From 3de19b9f7757766b3efd7eca13ae73ccbc5c4afe Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 16 Oct 2025 07:45:34 +0000 Subject: [PATCH 3/4] Add additional test for IQueryable == null scenario Co-authored-by: cincuranet <4540597+cincuranet@users.noreply.github.com> --- .../Query/NorthwindWhereQueryTestBase.cs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/EFCore.Specification.Tests/Query/NorthwindWhereQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/NorthwindWhereQueryTestBase.cs index 120dbe03414..31ff784560c 100644 --- a/test/EFCore.Specification.Tests/Query/NorthwindWhereQueryTestBase.cs +++ b/test/EFCore.Specification.Tests/Query/NorthwindWhereQueryTestBase.cs @@ -1430,6 +1430,19 @@ public virtual Task Where_Queryable_null_check_with_Contains(bool async) }); } + [ConditionalTheory, MemberData(nameof(IsAsyncData))] + public virtual Task Where_Queryable_null_check_equal(bool async) + { + return AssertQuery( + async, + ss => + { + var ids = ss.Set().Select(c => c.CustomerID); + return ss.Set().Where(c => ids == null || !ids.Contains(c.CustomerID)); + }, + assertEmpty: true); + } + [ConditionalTheory, MemberData(nameof(IsAsyncData))] public virtual Task Where_Queryable_ToList_Count_member(bool async) => AssertQuery( From e36862c085326391bea6331b97983dff9c15a932 Mon Sep 17 00:00:00 2001 From: Jiri Cincura Date: Thu, 16 Oct 2025 13:03:16 +0200 Subject: [PATCH 4/4] Fix on top of Copilot's work. --- .../NullCheckRemovingExpressionVisitor.cs | 73 ++++++++----------- .../Query/NorthwindWhereQueryCosmosTest.cs | 16 ++++ .../Query/NorthwindWhereQueryTestBase.cs | 46 ++++++------ .../Query/NorthwindWhereQuerySqlServerTest.cs | 30 ++++++++ 4 files changed, 97 insertions(+), 68 deletions(-) diff --git a/src/EFCore/Query/Internal/NullCheckRemovingExpressionVisitor.cs b/src/EFCore/Query/Internal/NullCheckRemovingExpressionVisitor.cs index 822ada647bc..c6300a97b1e 100644 --- a/src/EFCore/Query/Internal/NullCheckRemovingExpressionVisitor.cs +++ b/src/EFCore/Query/Internal/NullCheckRemovingExpressionVisitor.cs @@ -24,8 +24,8 @@ protected override Expression VisitBinary(BinaryExpression binaryExpression) { var visitedExpression = base.VisitBinary(binaryExpression); - return TryOptimizeQueryableNullCheck(visitedExpression) - ?? TryOptimizeConditionalEquality(visitedExpression) + return TryOptimizeConditionalEquality(visitedExpression) + ?? TryOptimizeQueryableNullCheck(visitedExpression) ?? visitedExpression; } @@ -77,34 +77,6 @@ protected override Expression VisitConditional(ConditionalExpression conditional return base.VisitConditional(conditionalExpression); } - private static Expression? TryOptimizeQueryableNullCheck(Expression expression) - { - // Optimize IQueryable/DbSet null checks - // IQueryable != null => true - // IQueryable == null => false - if (expression is BinaryExpression - { - NodeType: ExpressionType.Equal or ExpressionType.NotEqual - } binaryExpression) - { - var isLeftNull = IsNullConstant(binaryExpression.Left); - var isRightNull = IsNullConstant(binaryExpression.Right); - - if (isLeftNull != isRightNull) - { - var nonNullExpression = isLeftNull ? binaryExpression.Right : binaryExpression.Left; - - if (IsQueryableType(nonNullExpression.Type)) - { - var result = binaryExpression.NodeType == ExpressionType.NotEqual; - return Expression.Constant(result, typeof(bool)); - } - } - } - - return null; - } - private static Expression? TryOptimizeConditionalEquality(Expression expression) { // Simplify (a ? b : null) == null => !a || b == null @@ -145,6 +117,34 @@ protected override Expression VisitConditional(ConditionalExpression conditional return null; } + private static Expression? TryOptimizeQueryableNullCheck(Expression expression) + { + // Optimize IQueryable/DbSet null checks: + // * IQueryable != null => true + // * IQueryable == null => false + if (expression is BinaryExpression + { + NodeType: ExpressionType.Equal or ExpressionType.NotEqual + } binaryExpression) + { + var isLeftNull = IsNullConstant(binaryExpression.Left); + var isRightNull = IsNullConstant(binaryExpression.Right); + + if (isLeftNull != isRightNull) + { + var nonNullExpression = isLeftNull ? binaryExpression.Right : binaryExpression.Left; + + if (nonNullExpression.Type.IsAssignableTo(typeof(IQueryable))) + { + var result = binaryExpression.NodeType == ExpressionType.NotEqual; + return Expression.Constant(result); + } + } + } + + return null; + } + private sealed class NullSafeAccessVerifyingExpressionVisitor : ExpressionVisitor { private readonly ISet _nullSafeAccesses = new HashSet(ExpressionEqualityComparer.Instance); @@ -191,17 +191,4 @@ protected override Expression VisitUnary(UnaryExpression unaryExpression) private static bool IsNullConstant(Expression expression) => expression is ConstantExpression { Value: null }; - - private static bool IsQueryableType(Type type) - { - if (type.IsGenericType) - { - var genericTypeDefinition = type.GetGenericTypeDefinition(); - return genericTypeDefinition == typeof(IQueryable<>) - || genericTypeDefinition == typeof(IOrderedQueryable<>) - || genericTypeDefinition == typeof(DbSet<>); - } - - return type == typeof(IQueryable); - } } diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindWhereQueryCosmosTest.cs b/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindWhereQueryCosmosTest.cs index f91ad1dee97..2a93b5d8fba 100644 --- a/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindWhereQueryCosmosTest.cs +++ b/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindWhereQueryCosmosTest.cs @@ -1699,6 +1699,22 @@ public override async Task Where_Queryable_AsEnumerable_Contains_negated(bool as AssertSql(); } + public override async Task Where_Queryable_not_null_check_with_Contains(bool async) + { + // Cosmos client evaluation. Issue #17246. + await AssertTranslationFailed(() => base.Where_Queryable_not_null_check_with_Contains(async)); + + AssertSql(); + } + + public override async Task Where_Queryable_null_check_with_Contains(bool async) + { + // Cosmos client evaluation. Issue #17246. + await AssertTranslationFailed(() => base.Where_Queryable_null_check_with_Contains(async)); + + AssertSql(); + } + public override Task Where_list_object_contains_over_value_type(bool async) => Fixture.NoSyncTest( async, async a => diff --git a/test/EFCore.Specification.Tests/Query/NorthwindWhereQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/NorthwindWhereQueryTestBase.cs index 31ff784560c..6abf5422980 100644 --- a/test/EFCore.Specification.Tests/Query/NorthwindWhereQueryTestBase.cs +++ b/test/EFCore.Specification.Tests/Query/NorthwindWhereQueryTestBase.cs @@ -1418,31 +1418,6 @@ public virtual Task Where_Queryable_AsEnumerable_Contains_negated(bool async) elementSorter: e => e.CustomerID, elementAsserter: (e, a) => AssertCollection(e.Subquery, a.Subquery)); - [ConditionalTheory, MemberData(nameof(IsAsyncData))] - public virtual Task Where_Queryable_null_check_with_Contains(bool async) - { - return AssertQuery( - async, - ss => - { - var ids = ss.Set().Select(c => c.CustomerID); - return ss.Set().Where(c => ids != null && ids.Contains(c.CustomerID)); - }); - } - - [ConditionalTheory, MemberData(nameof(IsAsyncData))] - public virtual Task Where_Queryable_null_check_equal(bool async) - { - return AssertQuery( - async, - ss => - { - var ids = ss.Set().Select(c => c.CustomerID); - return ss.Set().Where(c => ids == null || !ids.Contains(c.CustomerID)); - }, - assertEmpty: true); - } - [ConditionalTheory, MemberData(nameof(IsAsyncData))] public virtual Task Where_Queryable_ToList_Count_member(bool async) => AssertQuery( @@ -1463,6 +1438,27 @@ public virtual Task Where_Queryable_ToArray_Length_member(bool async) assertOrder: true, elementAsserter: (e, a) => AssertCollection(e, a)); + [ConditionalTheory, MemberData(nameof(IsAsyncData))] + public virtual Task Where_Queryable_not_null_check_with_Contains(bool async) + => AssertQuery( + async, + ss => + { + var ids = ss.Set().Select(c => c.CustomerID); + return ss.Set().Where(c => ids != null && ids.Contains(c.CustomerID)); + }); + + [ConditionalTheory, MemberData(nameof(IsAsyncData))] + public virtual Task Where_Queryable_null_check_with_Contains(bool async) + => AssertQuery( + async, + ss => + { + var ids = ss.Set().Select(c => c.CustomerID); + return ss.Set().Where(c => ids == null || !ids.Contains(c.CustomerID)); + }, + assertEmpty: true); + [ConditionalTheory, MemberData(nameof(IsAsyncData))] public virtual Task Where_collection_navigation_ToList_Count(bool async) => AssertQuery( diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindWhereQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindWhereQuerySqlServerTest.cs index 63a234e7e7c..2cc60852b77 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindWhereQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindWhereQuerySqlServerTest.cs @@ -1890,6 +1890,36 @@ ORDER BY [c].[CustomerID] """); } + public override async Task Where_Queryable_not_null_check_with_Contains(bool async) + { + await base.Where_Queryable_not_null_check_with_Contains(async); + + AssertSql( + """ +SELECT [c].[CustomerID], [c].[Address], [c].[City], [c].[CompanyName], [c].[ContactName], [c].[ContactTitle], [c].[Country], [c].[Fax], [c].[Phone], [c].[PostalCode], [c].[Region] +FROM [Customers] AS [c] +WHERE [c].[CustomerID] IN ( + SELECT [c0].[CustomerID] + FROM [Customers] AS [c0] +) +"""); + } + + public override async Task Where_Queryable_null_check_with_Contains(bool async) + { + await base.Where_Queryable_null_check_with_Contains(async); + + AssertSql( + """ +SELECT [c].[CustomerID], [c].[Address], [c].[City], [c].[CompanyName], [c].[ContactName], [c].[ContactTitle], [c].[Country], [c].[Fax], [c].[Phone], [c].[PostalCode], [c].[Region] +FROM [Customers] AS [c] +WHERE [c].[CustomerID] NOT IN ( + SELECT [c0].[CustomerID] + FROM [Customers] AS [c0] +) +"""); + } + public override async Task Where_collection_navigation_ToList_Count(bool async) { await base.Where_collection_navigation_ToList_Count(async);