Process Coalesce-simplified Convert node properly in funcletizer (dotnet#35657)

Fixes dotnet#35656

(cherry picked from commit c54f51d)

Author: roji

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<Expression>).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)

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<Gear>().Where(g => (rank ?? g.Rank) == g.Rank));
+    }
+
     [ConditionalTheory]
     [MemberData(nameof(IsAsyncData))]
     public virtual async Task Client_eval_followed_by_aggregate_operation(bool async)

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='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 = [g].[Rank]
+""");
+    }
+
     [ConditionalTheory]
     [MemberData(nameof(IsAsyncData))]
     public async Task DataLength_function_for_string_parameter(bool async)

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='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 = [u].[Rank]
+""");
+    }
+
     [ConditionalTheory]
     [MemberData(nameof(IsAsyncData))]
     public async Task DataLength_function_for_string_parameter(bool async)

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='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 = [g].[Rank]
+""");
+    }
+
     [ConditionalTheory]
     [MemberData(nameof(IsAsyncData))]
     public async Task DataLength_function_for_string_parameter(bool async)

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='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 = [g].[Rank]
+""");
+    }
+
     public override async Task Comparison_with_value_converted_subclass(bool async)
     {
         await base.Comparison_with_value_converted_subclass(async);

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='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 = "g"."Rank"
+""");
+    }
+
     public override async Task Correlated_collections_with_Take(bool async)
     {
         await base.Correlated_collections_with_Take(async);