From 300202b0bce0386566bacf0f18a6c4bbab7eb8df Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Thu, 20 Feb 2025 22:36:02 +0100 Subject: [PATCH] Process Coalesce-simplified Convert node properly in funcletizer Fixes #35656 (cherry picked from commit c54f51d61b9ed1eca91fcc061edcf1a5f606a753) --- .../Internal/ExpressionTreeFuncletizer.cs | 11 ++++++++-- .../Query/GearsOfWarQueryTestBase.cs | 17 ++++++++++++++++ .../Query/GearsOfWarQuerySqlServerTest.cs | 14 +++++++++++++ .../Query/TPCGearsOfWarQuerySqlServerTest.cs | 20 +++++++++++++++++++ .../Query/TPTGearsOfWarQuerySqlServerTest.cs | 17 ++++++++++++++++ .../TemporalGearsOfWarQuerySqlServerTest.cs | 14 +++++++++++++ .../Query/GearsOfWarQuerySqliteTest.cs | 14 +++++++++++++ 7 files changed, 105 insertions(+), 2 deletions(-) diff --git a/src/EFCore/Query/Internal/ExpressionTreeFuncletizer.cs b/src/EFCore/Query/Internal/ExpressionTreeFuncletizer.cs index 3d70a71491f..d39cfdbd962 100644 --- a/src/EFCore/Query/Internal/ExpressionTreeFuncletizer.cs +++ b/src/EFCore/Query/Internal/ExpressionTreeFuncletizer.cs @@ -109,6 +109,9 @@ public class ExpressionTreeFuncletizer : ExpressionVisitor private static readonly bool UseOldBehavior35111 = AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue35111", out var enabled35111) && enabled35111; + private static readonly bool UseOldBehavior35656 = + AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue35656", out var enabled35656) && enabled35656; + private static readonly MethodInfo ReadOnlyCollectionIndexerGetter = typeof(ReadOnlyCollection).GetProperties() .Single(p => p.GetIndexParameters() is { Length: 1 } indexParameters && indexParameters[0].ParameterType == typeof(int)).GetMethod!; @@ -2132,8 +2135,12 @@ static Expression RemoveConvert(Expression expression) } } - private static Expression ConvertIfNeeded(Expression expression, Type type) - => expression.Type == type ? expression : Convert(expression, type); + private Expression ConvertIfNeeded(Expression expression, Type type) + => expression.Type == type + ? expression + : UseOldBehavior35656 + ? Convert(expression, type) + : Visit(Convert(expression, type)); private bool IsGenerallyEvaluatable(Expression expression) => _evaluatableExpressionFilter.IsEvaluatableExpression(expression, _model) diff --git a/test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs index 93ffe9f0cfb..b65b15d2d0c 100644 --- a/test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs +++ b/test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs @@ -6603,6 +6603,23 @@ public virtual Task Enum_array_contains(bool async) .Where(w => w.SynergyWith != null && types.Contains(w.SynergyWith.AmmunitionType))); } + [ConditionalTheory] // #35656 + [MemberData(nameof(IsAsyncData))] + public virtual Task Coalesce_with_non_root_evaluatable_Convert(bool async) + { + MilitaryRank? rank = MilitaryRank.Private; + + // The coalesce is simplified away in the funcletizer (since rank is non-null), but a Convert node is added + // to convert from MilitaryRank? (the type of rank) to the type of the coalesce expression (non-nullable + // MilitaryRank). + // This resulting Convert node isn't evaluatable as root (enum convert), and so the NotEvaluatableAsRootHandler + // is invoked. + return AssertQuery( + async, + // ReSharper disable once ConstantNullCoalescingCondition + ss => ss.Set().Where(g => (rank ?? g.Rank) == g.Rank)); + } + [ConditionalTheory] [MemberData(nameof(IsAsyncData))] public virtual async Task Client_eval_followed_by_aggregate_operation(bool async) diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/GearsOfWarQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/GearsOfWarQuerySqlServerTest.cs index cae4816b1ae..9429aab66b8 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/GearsOfWarQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/GearsOfWarQuerySqlServerTest.cs @@ -8162,6 +8162,20 @@ FROM OPENJSON(@__types_0_without_nulls) AS [t] """); } + public override async Task Coalesce_with_non_root_evaluatable_Convert(bool async) + { + await base.Coalesce_with_non_root_evaluatable_Convert(async); + + AssertSql( + """ +@__rank_0='1' (Nullable = true) + +SELECT [g].[Nickname], [g].[SquadId], [g].[AssignedCityName], [g].[CityOfBirthName], [g].[Discriminator], [g].[FullName], [g].[HasSoulPatch], [g].[LeaderNickname], [g].[LeaderSquadId], [g].[Rank] +FROM [Gears] AS [g] +WHERE @__rank_0 = [g].[Rank] +"""); + } + [ConditionalTheory] [MemberData(nameof(IsAsyncData))] public async Task DataLength_function_for_string_parameter(bool async) diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/TPCGearsOfWarQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/TPCGearsOfWarQuerySqlServerTest.cs index ee461d32d9c..c54d2675c03 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/TPCGearsOfWarQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/TPCGearsOfWarQuerySqlServerTest.cs @@ -10883,6 +10883,26 @@ FROM OPENJSON(@__types_0_without_nulls) AS [t] """); } + public override async Task Coalesce_with_non_root_evaluatable_Convert(bool async) + { + await base.Coalesce_with_non_root_evaluatable_Convert(async); + + AssertSql( + """ +@__rank_0='1' (Nullable = true) + +SELECT [u].[Nickname], [u].[SquadId], [u].[AssignedCityName], [u].[CityOfBirthName], [u].[FullName], [u].[HasSoulPatch], [u].[LeaderNickname], [u].[LeaderSquadId], [u].[Rank], [u].[Discriminator] +FROM ( + SELECT [g].[Nickname], [g].[SquadId], [g].[AssignedCityName], [g].[CityOfBirthName], [g].[FullName], [g].[HasSoulPatch], [g].[LeaderNickname], [g].[LeaderSquadId], [g].[Rank], N'Gear' AS [Discriminator] + FROM [Gears] AS [g] + UNION ALL + SELECT [o].[Nickname], [o].[SquadId], [o].[AssignedCityName], [o].[CityOfBirthName], [o].[FullName], [o].[HasSoulPatch], [o].[LeaderNickname], [o].[LeaderSquadId], [o].[Rank], N'Officer' AS [Discriminator] + FROM [Officers] AS [o] +) AS [u] +WHERE @__rank_0 = [u].[Rank] +"""); + } + [ConditionalTheory] [MemberData(nameof(IsAsyncData))] public async Task DataLength_function_for_string_parameter(bool async) diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/TPTGearsOfWarQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/TPTGearsOfWarQuerySqlServerTest.cs index 076e6c19d97..d5fa8b8562d 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/TPTGearsOfWarQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/TPTGearsOfWarQuerySqlServerTest.cs @@ -9220,6 +9220,23 @@ FROM OPENJSON(@__types_0_without_nulls) AS [t] """); } + public override async Task Coalesce_with_non_root_evaluatable_Convert(bool async) + { + await base.Coalesce_with_non_root_evaluatable_Convert(async); + + AssertSql( + """ +@__rank_0='1' (Nullable = true) + +SELECT [g].[Nickname], [g].[SquadId], [g].[AssignedCityName], [g].[CityOfBirthName], [g].[FullName], [g].[HasSoulPatch], [g].[LeaderNickname], [g].[LeaderSquadId], [g].[Rank], CASE + WHEN [o].[Nickname] IS NOT NULL THEN N'Officer' +END AS [Discriminator] +FROM [Gears] AS [g] +LEFT JOIN [Officers] AS [o] ON [g].[Nickname] = [o].[Nickname] AND [g].[SquadId] = [o].[SquadId] +WHERE @__rank_0 = [g].[Rank] +"""); + } + [ConditionalTheory] [MemberData(nameof(IsAsyncData))] public async Task DataLength_function_for_string_parameter(bool async) diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/TemporalGearsOfWarQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/TemporalGearsOfWarQuerySqlServerTest.cs index ca649d80e12..5d82e72fff3 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/TemporalGearsOfWarQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/TemporalGearsOfWarQuerySqlServerTest.cs @@ -6987,6 +6987,20 @@ SELECT TOP(1) ~CAST(([g].[Rank] & 2) ^ 2 AS bit) AS [BitwiseTrue], ~CAST(([g].[R """); } + public override async Task Coalesce_with_non_root_evaluatable_Convert(bool async) + { + await base.Coalesce_with_non_root_evaluatable_Convert(async); + + AssertSql( + """ +@__rank_0='1' (Nullable = true) + +SELECT [g].[Nickname], [g].[SquadId], [g].[AssignedCityName], [g].[CityOfBirthName], [g].[Discriminator], [g].[FullName], [g].[HasSoulPatch], [g].[LeaderNickname], [g].[LeaderSquadId], [g].[PeriodEnd], [g].[PeriodStart], [g].[Rank] +FROM [Gears] FOR SYSTEM_TIME AS OF '2010-01-01T00:00:00.0000000' AS [g] +WHERE @__rank_0 = [g].[Rank] +"""); + } + public override async Task Comparison_with_value_converted_subclass(bool async) { await base.Comparison_with_value_converted_subclass(async); diff --git a/test/EFCore.Sqlite.FunctionalTests/Query/GearsOfWarQuerySqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/Query/GearsOfWarQuerySqliteTest.cs index 6e69f0b1ce3..77293ffbe86 100644 --- a/test/EFCore.Sqlite.FunctionalTests/Query/GearsOfWarQuerySqliteTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/Query/GearsOfWarQuerySqliteTest.cs @@ -6383,6 +6383,20 @@ LIMIT @__p_0 """); } + public override async Task Coalesce_with_non_root_evaluatable_Convert(bool async) + { + await base.Coalesce_with_non_root_evaluatable_Convert(async); + + AssertSql( + """ +@__rank_0='1' (Nullable = true) + +SELECT "g"."Nickname", "g"."SquadId", "g"."AssignedCityName", "g"."CityOfBirthName", "g"."Discriminator", "g"."FullName", "g"."HasSoulPatch", "g"."LeaderNickname", "g"."LeaderSquadId", "g"."Rank" +FROM "Gears" AS "g" +WHERE @__rank_0 = "g"."Rank" +"""); + } + public override async Task Correlated_collections_with_Take(bool async) { await base.Correlated_collections_with_Take(async);