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

Fixes #35656

Author: roji

Diff for: src/EFCore/Query/Internal/ExpressionTreeFuncletizer.cs

@@ -2179,8 +2179,8 @@ 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 : Visit(Convert(expression, type));
 
     private bool IsGenerallyEvaluatable(Expression expression)
         => _evaluatableExpressionFilter.IsEvaluatableExpression(expression, _model)

Diff for: test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs

@@ -6013,6 +6013,23 @@ public virtual Task Constant_enum_with_same_underlying_value_as_previously_param
     //             .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)

Diff for: test/EFCore.SqlServer.FunctionalTests/Query/GearsOfWarQuerySqlServerTest.cs

@@ -7381,6 +7381,20 @@ ORDER BY [g].[Nickname]
 // """);
 //     }
 
+    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)

Diff for: test/EFCore.SqlServer.FunctionalTests/Query/TPCGearsOfWarQuerySqlServerTest.cs

@@ -9913,6 +9913,26 @@ ORDER BY [u].[Nickname]
 // """);
 //     }
 
+    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)

Diff for: test/EFCore.SqlServer.FunctionalTests/Query/TPTGearsOfWarQuerySqlServerTest.cs

@@ -8376,6 +8376,23 @@ ORDER BY [g].[Nickname]
 // """);
 //     }
 
+    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)

Diff for: test/EFCore.SqlServer.FunctionalTests/Query/TemporalGearsOfWarQuerySqlServerTest.cs

@@ -6092,6 +6092,20 @@ ORDER BY [g].[Nickname]
 """);
     }
 
+    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);

Diff for: test/EFCore.Sqlite.FunctionalTests/Query/GearsOfWarQuerySqliteTest.cs

@@ -5454,6 +5454,20 @@ LIMIT @p
 """);
     }
 
+    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);