Fix to #23877 - Odata $filter and $orderby not working with Select to…

… new DTO for one to one relationship

Problem was that during query optimization, TryOptimizeMemberAccessOverConditional could change the type of the expression (from non nullable to nullable), which then would throw when trying to re-construct the expression tree upstream.
Fix is to compensate for type changes across the visitor and only reconstructing original type at the top level (projection, lambda, method call arguments, etc).

Fixes #23877

src/EFCore.InMemory/Query/Internal/InMemoryExpressionTranslatingExpressionVisitor.cs

@@ -336,6 +336,18 @@ protected override Expression VisitMember(MemberExpression memberExpression)
             Check.NotNull(memberExpression, nameof(memberExpression));
 
             var innerExpression = Visit(memberExpression.Expression);
+
+            // when visiting unary we remove converts from nullable to non-nullable
+            // however if this happens for memberExpression.Expression we are unable to bind
+            if (innerExpression != null
+                && memberExpression.Expression != null
+                && innerExpression.Type != memberExpression.Expression.Type
+                && innerExpression.Type.IsNullableType()
+                && innerExpression.Type.UnwrapNullableType() == memberExpression.Expression.Type)
+            {
+                innerExpression = Expression.Convert(innerExpression, memberExpression.Expression.Type);
+            }
+
             if (memberExpression.Expression != null
                 && innerExpression == QueryCompilationContext.NotTranslatedExpression)
             {

src/EFCore/Query/Internal/QueryOptimizingExpressionVisitor.cs

@@ -42,7 +42,13 @@ public class QueryOptimizingExpressionVisitor : ExpressionVisitor
         /// </summary>
         protected override Expression VisitMember(MemberExpression memberExpression)
         {
-            var visitedExpression = base.VisitMember(memberExpression);
+            var expression = memberExpression.Expression != null
+                ? MatchExpressionType(
+                    Visit(memberExpression.Expression),
+                    memberExpression.Expression.Type)
+                : null;
+
+            var visitedExpression = memberExpression.Update(expression);
 
             return TryOptimizeMemberAccessOverConditional(visitedExpression) ?? visitedExpression;
         }
@@ -128,7 +134,23 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
                 return Expression.Call(null, anyMethod, new[] { methodCallExpression.Arguments[0], anyLambda });
             }
 
-            var visited = (MethodCallExpression)base.VisitMethodCall(methodCallExpression);
+            var @object = default(Expression);
+            if (methodCallExpression.Object != null)
+            {
+                @object = MatchExpressionType(
+                    Visit(methodCallExpression.Object),
+                    methodCallExpression.Object.Type);
+            }
+
+            var arguments = new Expression[methodCallExpression.Arguments.Count];
+            for (var i = 0; i < arguments.Length; i++)
+            {
+                arguments[i] = MatchExpressionType(
+                    Visit(methodCallExpression.Arguments[i]),
+                    methodCallExpression.Arguments[i].Type);
+            }
+
+            var visited = methodCallExpression.Update(@object!, arguments);
 
             // In VB.NET, comparison operators between strings (equality, greater-than, less-than) yield
             // calls to a VB-specific CompareString method. Normalize that to string.Compare.
@@ -157,6 +179,11 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
             return visited;
         }
 
+        private Expression MatchExpressionType(Expression expression, Type typeToMatch)
+            => expression.Type != typeToMatch
+                ? Expression.Convert(expression, typeToMatch)
+                : expression;
+
         /// <summary>
         ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
         ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
@@ -198,7 +225,160 @@ protected override Expression VisitUnary(UnaryExpression unaryExpression)
                         result);
             }
 
-            return base.VisitUnary(unaryExpression);
+            return unaryExpression.Update(
+                Visit(unaryExpression.Operand));
+        }
+
+        /// <summary>
+        ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+        ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+        ///     any release. You should only use it directly in your code with extreme caution and knowing that
+        ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+        /// </summary>
+        protected override Expression VisitBinary(BinaryExpression binaryExpression)
+        {
+            var left = Visit(binaryExpression.Left);
+            var right = Visit(binaryExpression.Right);
+
+            if (binaryExpression.NodeType != ExpressionType.Coalesce
+                && left.Type != right.Type
+                && left.Type.UnwrapNullableType() == right.Type.UnwrapNullableType())
+            {
+                if (left.Type.IsNullableValueType())
+                {
+                    right = Expression.Convert(right, left.Type);
+                }
+                else
+                {
+                    left = Expression.Convert(left, right.Type);
+                }
+            }
+
+            return binaryExpression.Update(left, binaryExpression.Conversion, right);
+        }
+
+        /// <summary>
+        ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+        ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+        ///     any release. You should only use it directly in your code with extreme caution and knowing that
+        ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+        /// </summary>
+        protected override Expression VisitConditional(ConditionalExpression conditionalExpression)
+        {
+            var test = Visit(conditionalExpression.Test);
+            var ifTrue = Visit(conditionalExpression.IfTrue);
+            var ifFalse = Visit(conditionalExpression.IfFalse);
+
+            if (ifTrue.Type != ifFalse.Type
+                && ifTrue.Type.UnwrapNullableType() == ifFalse.Type.UnwrapNullableType())
+            {
+                if (ifTrue.Type.IsNullableValueType())
+                {
+                    ifFalse = Expression.Convert(ifFalse, ifTrue.Type);
+                }
+                else
+                {
+                    ifTrue = Expression.Convert(ifTrue, ifFalse.Type);
+                }
+
+                return Expression.Condition(test, ifTrue, ifFalse);
+            }
+
+            return conditionalExpression.Update(test, ifTrue, ifFalse);
+        }
+
+        /// <summary>
+        ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+        ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+        ///     any release. You should only use it directly in your code with extreme caution and knowing that
+        ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+        /// </summary>
+        protected override Expression VisitLambda<T>(Expression<T> lambdaExpression)
+        {
+            Check.NotNull(lambdaExpression, nameof(lambdaExpression));
+
+            var body = Visit(lambdaExpression.Body);
+
+            return body.Type != lambdaExpression.Body.Type
+                ? Expression.Lambda(Expression.Convert(body, lambdaExpression.Body.Type), lambdaExpression.Parameters)
+                : lambdaExpression.Update(body, lambdaExpression.Parameters)!;
+        }
+
+        /// <summary>
+        ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+        ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+        ///     any release. You should only use it directly in your code with extreme caution and knowing that
+        ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+        /// </summary>
+        protected override Expression VisitNew(NewExpression newExpression)
+        {
+            if (newExpression.Arguments.Count == 0)
+            {
+                return newExpression;
+            }
+
+            var arguments = new Expression[newExpression.Arguments.Count];
+            for (var i = 0; i < arguments.Length; i++)
+            {
+                arguments[i] = MatchExpressionType(
+                    Visit(newExpression.Arguments[i]),
+                    newExpression.Arguments[i].Type);
+            }
+
+            return newExpression.Update(arguments);
+        }
+
+        /// <summary>
+        ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+        ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+        ///     any release. You should only use it directly in your code with extreme caution and knowing that
+        ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+        /// </summary>
+        protected override ElementInit VisitElementInit(ElementInit elementInit)
+        {
+            var arguments = new Expression[elementInit.Arguments.Count];
+            for (var i = 0; i < arguments.Length; i++)
+            {
+                arguments[i] = MatchExpressionType(
+                    Visit(elementInit.Arguments[i]),
+                    elementInit.Arguments[i].Type);
+            }
+
+            return elementInit.Update(arguments);
+        }
+
+        /// <summary>
+        ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+        ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+        ///     any release. You should only use it directly in your code with extreme caution and knowing that
+        ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+        /// </summary>
+        protected override MemberAssignment VisitMemberAssignment(MemberAssignment memberAssignment)
+        {
+            var expression = MatchExpressionType(
+                Visit(memberAssignment.Expression),
+                memberAssignment.Expression.Type);
+
+            return memberAssignment.Update(expression);
+        }
+
+        /// <summary>
+        ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+        ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+        ///     any release. You should only use it directly in your code with extreme caution and knowing that
+        ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+        /// </summary>
+        protected override Expression VisitNewArray(NewArrayExpression newArrayExpression)
+        {
+            var expressions = new Expression[newArrayExpression.Expressions.Count];
+            for (var i = 0; i < expressions.Length; i++)
+            {
+                expressions[i] = MatchExpressionType(
+                    Visit(newArrayExpression.Expressions[i]),
+                    newArrayExpression.Expressions[i].Type);
+            }
+
+            return newArrayExpression.Update(expressions);
         }
 
         private bool TryExtractEqualityOperands(

test/EFCore.OData.FunctionalTests/Query/ODataQueryTestFixtureInitializer.cs

@@ -58,7 +58,7 @@ public static (string BaseAddress, IHttpClientFactory ClientFactory, IHost SelfH
                                 }
                             }
 
-                            endpoints.MaxTop(2).Expand().Select().OrderBy().Filter();
+                            endpoints.MaxTop(null).Expand().Select().OrderBy().Filter().Count();
                             endpoints.MapODataRoute("odata", "odata",
                                 edmModel,
                                 new DefaultODataPathHandler(),