Fix to #12830 - Query: invalid sql for query projecting a sql functio…

…n (e.g ROUND) and ordering by it twice

Problem was that when performing Expression comparison using ExpressionEqualityComparer, the constituent sequences were compared using default comparer (via SequenceEquals method), which doesn't know how to properly handle some Expressions (e.g. ConstantExpression).
Fix is to add SequenceEquals method to out ExpressionEqualityComparer, that will perform the sequence comparison using its own logic.

src/EFCore.Relational/Query/Expressions/InExpression.cs

@@ -6,6 +6,7 @@
 using System.Linq;
 using System.Linq.Expressions;
 using JetBrains.Annotations;
+using Microsoft.EntityFrameworkCore.Query.Internal;
 using Microsoft.EntityFrameworkCore.Query.Sql;
 using Microsoft.EntityFrameworkCore.Utilities;
 
@@ -170,7 +171,7 @@ private bool Equals(InExpression other)
             => Operand.Equals(other.Operand)
                && (Values == null
                    ? other.Values == null
-                   : Values.SequenceEqual(other.Values))
+                    : ExpressionEqualityComparer.Instance.SequenceEquals(Values, other.Values))
                && (SubQuery == null
                    ? other.SubQuery == null
                    : SubQuery.Equals(other.SubQuery));

src/EFCore.Relational/Query/Expressions/SqlFunctionExpression.cs

@@ -8,6 +8,7 @@
 using System.Linq;
 using System.Linq.Expressions;
 using JetBrains.Annotations;
+using Microsoft.EntityFrameworkCore.Query.Internal;
 using Microsoft.EntityFrameworkCore.Query.Sql;
 using Microsoft.EntityFrameworkCore.Utilities;
 
@@ -257,7 +258,7 @@ private bool Equals(SqlFunctionExpression other)
                && string.Equals(FunctionName, other.FunctionName)
                && string.Equals(Schema, other.Schema)
                && IsNiladic == other.IsNiladic
-               && _arguments.SequenceEqual(other._arguments)
+               && ExpressionEqualityComparer.Instance.SequenceEquals(_arguments, other._arguments)
                && (Instance == null && other.Instance == null
                    || Instance?.Equals(other.Instance) == true);
 

src/EFCore.Specification.Tests/Query/SimpleQueryTestBase.Functions.cs

@@ -1268,5 +1268,32 @@ public virtual Task Static_equals_int_compared_to_long(bool isAsync)
                 isAsync,
                 os => os.Where(o => Equals(o.OrderID, arg)));
         }
+
+        [Theory]
+        [MemberData(nameof(IsAsyncData))]
+        public virtual Task Projecting_Math_Truncate_and_ordering_by_it_twice(bool isAsync)
+        {
+            return AssertQuery<Order>(
+                isAsync,
+                os => os.Where(o => o.OrderID < 10250).Select(o => new { A = Math.Truncate((double)o.OrderID) }).OrderBy(r => r.A).OrderBy(r => r.A));
+        }
+
+        [Theory]
+        [MemberData(nameof(IsAsyncData))]
+        public virtual Task Projecting_Math_Truncate_and_ordering_by_it_twice2(bool isAsync)
+        {
+            return AssertQuery<Order>(
+                isAsync,
+                os => os.Where(o => o.OrderID < 10250).Select(o => new { A = Math.Truncate((double)o.OrderID) }).OrderBy(r => r.A).OrderByDescending(r => r.A));
+        }
+
+        [Theory]
+        [MemberData(nameof(IsAsyncData))]
+        public virtual Task Projecting_Math_Truncate_and_ordering_by_it_twice3(bool isAsync)
+        {
+            return AssertQuery<Order>(
+                isAsync,
+                os => os.Where(o => o.OrderID < 10250).Select(o => new { A = Math.Truncate((double)o.OrderID) }).OrderByDescending(r => r.A).ThenBy(r => r.A));
+        }
     }
 }

src/EFCore/Query/Internal/ExpressionEqualityComparer.cs

@@ -311,6 +311,27 @@ private int GetHashCode<T>(IList<T> expressions)
         /// </summary>
         public virtual bool Equals(Expression x, Expression y) => new ExpressionComparer().Compare(x, y);
 
+        /// <summary>
+        ///     This API supports the Entity Framework Core infrastructure and is not intended to be used
+        ///     directly from your code. This API may change or be removed in future releases.
+        /// </summary>
+        public virtual bool SequenceEquals(IEnumerable<Expression> x, IEnumerable<Expression> y)
+        {
+            if (x == null || y == null)
+            {
+                return false;
+            }
+
+            if (x.Count() != y.Count())
+            {
+                return false;
+            }
+
+            var comparer = new ExpressionComparer();
+
+            return x.Zip(y, (l, r) => comparer.Compare(l, r)).All(r => r);
+        }
+
         private sealed class ExpressionComparer
         {
             private ScopedDictionary<ParameterExpression, ParameterExpression> _parameterScope;
@@ -586,15 +607,18 @@ private bool CompareExtension(Expression a, Expression b)
                         nullConditionalExpressionB.AccessOperation);
                 }
 
-                return a is NullSafeEqualExpression nullConditionalEqualExpressionA
-                    && b is NullSafeEqualExpression nullConditionalEqualExpressionB
-                    ? Compare(
+                if (a is NullSafeEqualExpression nullConditionalEqualExpressionA
+                    && b is NullSafeEqualExpression nullConditionalEqualExpressionB)
+                {
+                    return Compare(
                                nullConditionalEqualExpressionA.OuterKeyNullCheck,
                                nullConditionalEqualExpressionB.OuterKeyNullCheck)
                            && Compare(
                                nullConditionalEqualExpressionA.EqualExpression,
-                               nullConditionalEqualExpressionB.EqualExpression)
-                    : a.Equals(b);
+                               nullConditionalEqualExpressionB.EqualExpression);
+                }
+
+                return a.Equals(b);
             }
 
             private bool CompareInvocation(InvocationExpression a, InvocationExpression b)

test/EFCore.SqlServer.FunctionalTests/Query/SimpleQuerySqlServerTest.Functions.cs

@@ -1305,5 +1305,38 @@ public override async Task Static_equals_int_compared_to_long(bool isAsync)
 FROM [Orders] AS [o]
 WHERE 0 = 1");
         }
+
+        public override async Task Projecting_Math_Truncate_and_ordering_by_it_twice(bool isAsync)
+        {
+            await base.Projecting_Math_Truncate_and_ordering_by_it_twice(isAsync);
+
+            AssertSql(
+                @"SELECT ROUND(CAST([o].[OrderID] AS float), 0, 1) AS [A]
+FROM [Orders] AS [o]
+WHERE [o].[OrderID] < 10250
+ORDER BY [A]");
+        }
+
+        public override async Task Projecting_Math_Truncate_and_ordering_by_it_twice2(bool isAsync)
+        {
+            await base.Projecting_Math_Truncate_and_ordering_by_it_twice2(isAsync);
+
+            AssertSql(
+                @"SELECT ROUND(CAST([o].[OrderID] AS float), 0, 1) AS [A]
+FROM [Orders] AS [o]
+WHERE [o].[OrderID] < 10250
+ORDER BY [A] DESC");
+        }
+
+        public override async Task Projecting_Math_Truncate_and_ordering_by_it_twice3(bool isAsync)
+        {
+            await base.Projecting_Math_Truncate_and_ordering_by_it_twice3(isAsync);
+
+            AssertSql(
+                @"SELECT ROUND(CAST([o].[OrderID] AS float), 0, 1) AS [A]
+FROM [Orders] AS [o]
+WHERE [o].[OrderID] < 10250
+ORDER BY [A] DESC");
+        }
     }
 }