Add missing Converts when simplifying in funcletizer (#35122) (#35202)

Fixes #35095

(cherry picked from commit 3cae7a8)

Author: roji

src/EFCore/Query/Internal/ExpressionTreeFuncletizer.cs

@@ -100,6 +100,9 @@ public class ExpressionTreeFuncletizer : ExpressionVisitor
 
     private static readonly IReadOnlySet<string> EmptyStringSet = new HashSet<string>();
 
+    private static readonly bool UseOldBehavior35095 =
+        AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue35095", out var enabled35095) && enabled35095;
+
     private static readonly MethodInfo ReadOnlyCollectionIndexerGetter = typeof(ReadOnlyCollection<Expression>).GetProperties()
         .Single(p => p.GetIndexParameters() is { Length: 1 } indexParameters && indexParameters[0].ParameterType == typeof(int)).GetMethod!;
 
@@ -379,17 +382,23 @@ protected override Expression VisitBinary(BinaryExpression binary)
                 case ExpressionType.Coalesce:
                     leftValue = Evaluate(left);
 
+                    Expression returnValue;
                     switch (leftValue)
                     {
                         case null:
-                            return Visit(binary.Right, out _state);
+                            returnValue = Visit(binary.Right, out _state);
+                            break;
                         case bool b:
                             _state = leftState with { StateType = StateType.EvaluatableWithoutCapturedVariable };
-                            return Constant(b);
+                            returnValue = Constant(b);
+                            break;
                         default:
-                            return left;
+                            returnValue = left;
+                            break;
                     }
 
+                    return UseOldBehavior35095 ? returnValue : ConvertIfNeeded(returnValue, binary.Type);
+
                 case ExpressionType.OrElse or ExpressionType.AndAlso when Evaluate(left) is bool leftBoolValue:
                 {
                     left = Constant(leftBoolValue);
@@ -511,9 +520,10 @@ protected override Expression VisitConditional(ConditionalExpression conditional
         // If the test evaluates, simplify the conditional away by bubbling up the leg that remains
         if (testState.IsEvaluatable && Evaluate(test) is bool testBoolValue)
         {
-            return testBoolValue
+            var returnValue = testBoolValue
                 ? Visit(conditional.IfTrue, out _state)
                 : Visit(conditional.IfFalse, out _state);
+            return UseOldBehavior35095 ? returnValue : ConvertIfNeeded(returnValue, conditional.Type);
         }
 
         var ifTrue = Visit(conditional.IfTrue, out var ifTrueState);
@@ -2101,6 +2111,9 @@ static Expression RemoveConvert(Expression expression)
         }
     }
 
+    private static Expression ConvertIfNeeded(Expression expression, Type type)
+        => expression.Type == type ? expression : Convert(expression, type);
+
     private bool IsGenerallyEvaluatable(Expression expression)
         => _evaluatableExpressionFilter.IsEvaluatableExpression(expression, _model)
             && (_parameterize

test/EFCore.Cosmos.FunctionalTests/Query/NorthwindWhereQueryCosmosTest.cs

@@ -3303,6 +3303,18 @@ FROM root c
 SELECT VALUE c["OrderID"]
 FROM root c
 WHERE ((c["$type"] = "Order") AND (c["OrderID"] = 10252))
+""");
+            });
+
+    public override Task Simplifiable_coalesce_over_nullable(bool async)
+        => Fixture.NoSyncTest(
+            async, async a =>
+            {
+                await base.Simplifiable_coalesce_over_nullable(a);
+
+                AssertSql(
+                    """
+ReadItem(None, Order|10248)
 """);
             });
 

test/EFCore.Specification.Tests/Query/NorthwindWhereQueryTestBase.cs

@@ -2536,7 +2536,18 @@ await AssertQuery(
             elementAsserter: (e, a) => AssertEqual(e.Id, a.Id));
     }
 
-    #region Evaluation order of predicates
+    [ConditionalTheory] // #35095
+    [MemberData(nameof(IsAsyncData))]
+    public virtual Task Simplifiable_coalesce_over_nullable(bool async)
+    {
+        int? orderId = 10248;
+
+        return AssertQuery(
+            async,
+            ss => ss.Set<Order>().Where(o => o.OrderID == (orderId ?? 0)));
+    }
+
+    #region Evaluation order of operators
 
     [ConditionalTheory]
     [MemberData(nameof(IsAsyncData))]
@@ -2559,5 +2570,5 @@ public virtual Task Take_and_Distinct_evaluation_order(bool async)
             async,
             ss => ss.Set<Customer>().Select(c => c.ContactTitle).OrderBy(t => t).Take(3).Distinct());
 
-    #endregion Evaluation order of predicates
+    #endregion Evaluation order of operators
 }

test/EFCore.SqlServer.FunctionalTests/Query/NorthwindWhereQuerySqlServerTest.cs

@@ -3425,7 +3425,21 @@ FROM [Orders] AS [o]
 """);
     }
 
-    #region Evaluation order of predicates
+    public override async Task Simplifiable_coalesce_over_nullable(bool async)
+    {
+        await base.Simplifiable_coalesce_over_nullable(async);
+
+        AssertSql(
+            """
+@__p_0='10248'
+
+SELECT [o].[OrderID], [o].[CustomerID], [o].[EmployeeID], [o].[OrderDate]
+FROM [Orders] AS [o]
+WHERE [o].[OrderID] = @__p_0
+""");
+    }
+
+    #region Evaluation order of operators
 
     public override async Task Take_and_Where_evaluation_order(bool async)
     {
@@ -3483,7 +3497,7 @@ ORDER BY [c].[ContactTitle]
 """);
     }
 
-    #endregion Evaluation order of predicates
+    #endregion Evaluation order of operators
 
     private void AssertSql(params string[] expected)
         => Fixture.TestSqlLoggerFactory.AssertBaseline(expected);