Query: MakeUnary/Binary should return null if invalid operator type

Resolves #20622

src/EFCore.Relational/Query/SqlExpressionFactory.cs

@@ -224,10 +224,14 @@ private SqlExpression ApplyTypeMappingOnSqlBinary(
         public virtual SqlBinaryExpression MakeBinary(
             ExpressionType operatorType, SqlExpression left, SqlExpression right, RelationalTypeMapping typeMapping)
         {
-            Check.NotNull(operatorType, nameof(operatorType));
             Check.NotNull(left, nameof(left));
             Check.NotNull(right, nameof(right));
 
+            if (!SqlBinaryExpression.IsValidOperator(operatorType))
+            {
+                return null;
+            }
+
             var returnType = left.Type;
             switch (operatorType)
             {
@@ -417,6 +421,11 @@ public virtual SqlUnaryExpression MakeUnary(
             Check.NotNull(operand, nameof(operand));
             Check.NotNull(type, nameof(type));
 
+            if (!SqlUnaryExpression.IsValidOperator(operatorType))
+            {
+                return null;
+            }
+
             return (SqlUnaryExpression)ApplyTypeMapping(new SqlUnaryExpression(operatorType, operand, type, null), typeMapping);
         }
 

src/EFCore.Relational/Query/SqlExpressions/SqlBinaryExpression.cs

@@ -46,10 +46,8 @@ public class SqlBinaryExpression : SqlExpression
             //ExpressionType.LeftShift,
         };
 
-        private static ExpressionType VerifyOperator(ExpressionType operatorType)
-            => _allowedOperators.Contains(operatorType)
-                ? operatorType
-                : throw new InvalidOperationException(CoreStrings.UnsupportedBinaryOperator);
+        internal static bool IsValidOperator(ExpressionType operatorType)
+            => _allowedOperators.Contains(operatorType);
 
         /// <summary>
         ///     Creates a new instance of the <see cref="SqlBinaryExpression" /> class.
@@ -70,8 +68,12 @@ public SqlBinaryExpression(
             Check.NotNull(left, nameof(left));
             Check.NotNull(right, nameof(right));
 
-            OperatorType = VerifyOperator(operatorType);
+            if (!IsValidOperator(operatorType))
+            {
+                throw new InvalidOperationException(CoreStrings.UnsupportedBinaryOperator);
+            }
 
+            OperatorType = operatorType;
             Left = left;
             Right = right;
         }

src/EFCore.Relational/Query/SqlExpressions/SqlUnaryExpression.cs

@@ -31,10 +31,8 @@ public class SqlUnaryExpression : SqlExpression
             ExpressionType.Negate
         };
 
-        private static ExpressionType VerifyOperator(ExpressionType operatorType)
-            => _allowedOperators.Contains(operatorType)
-                ? operatorType
-                : throw new InvalidOperationException(CoreStrings.UnsupportedUnary);
+        internal static bool IsValidOperator(ExpressionType operatorType)
+            => _allowedOperators.Contains(operatorType);
 
         /// <summary>
         ///     Creates a new instance of the <see cref="SqlUnaryExpression" /> class.
@@ -53,7 +51,12 @@ public SqlUnaryExpression(
             Check.NotNull(operand, nameof(operand));
             Check.NotNull(type, nameof(type));
 
-            OperatorType = VerifyOperator(operatorType);
+            if (!IsValidOperator(operatorType))
+            {
+                throw new InvalidOperationException(CoreStrings.UnsupportedUnary);
+            }
+
+            OperatorType = operatorType;
             Operand = operand;
         }
 

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

@@ -9,6 +9,7 @@
 using Microsoft.EntityFrameworkCore.TestModels.Northwind;
 using Microsoft.EntityFrameworkCore.TestUtilities;
 using Xunit;
+using Microsoft.EntityFrameworkCore.Diagnostics;
 
 // ReSharper disable AccessToModifiedClosure
 // ReSharper disable InconsistentNaming
@@ -212,17 +213,17 @@ public virtual void Query_with_array_parameter()
             using (var context = CreateContext())
             {
                 Assert.Equal(
-                    "Unsupported Binary operator type specified.",
+                    CoreStrings.TranslationFailed("DbSet<Customer>()    .Where(c => c.CustomerID == __args[0])"),
                     Assert.Throws<InvalidOperationException>(
-                        () => query(context, new[] { "ALFKI" }).First().CustomerID).Message);
+                        () => query(context, new[] { "ALFKI" }).First().CustomerID).Message.Replace("\r", "").Replace("\n", ""));
             }
 
             using (var context = CreateContext())
             {
                 Assert.Equal(
-                    "Unsupported Binary operator type specified.",
+                    CoreStrings.TranslationFailed("DbSet<Customer>()    .Where(c => c.CustomerID == __args[0])"),
                     Assert.Throws<InvalidOperationException>(
-                        () => query(context, new[] { "ANATR" }).First().CustomerID).Message);
+                        () => query(context, new[] { "ANATR" }).First().CustomerID).Message.Replace("\r", "").Replace("\n", ""));
             }
         }
 
@@ -476,17 +477,17 @@ public virtual async Task Query_with_array_parameter_async()
             using (var context = CreateContext())
             {
                 Assert.Equal(
-                    "Unsupported Binary operator type specified.",
+                    CoreStrings.TranslationFailed("DbSet<Customer>()    .Where(c => c.CustomerID == __args[0])"),
                     (await Assert.ThrowsAsync<InvalidOperationException>(
-                        () => query(context, new[] { "ALFKI" }).ToListAsync())).Message);
+                        () => query(context, new[] { "ALFKI" }).ToListAsync())).Message.Replace("\r", "").Replace("\n",""));
             }
 
             using (var context = CreateContext())
             {
                 Assert.Equal(
-                    "Unsupported Binary operator type specified.",
+                    CoreStrings.TranslationFailed("DbSet<Customer>()    .Where(c => c.CustomerID == __args[0])"),
                     (await Assert.ThrowsAsync<InvalidOperationException>(
-                        () => query(context, new[] { "ANATR" }).ToListAsync())).Message);
+                        () => query(context, new[] { "ANATR" }).ToListAsync())).Message.Replace("\r", "").Replace("\n", ""));
             }
         }
 
@@ -641,6 +642,19 @@ public virtual async Task Compiled_query_with_max_parameters()
             Assert.Equal(14, await asyncSingleResultQueryWithCancellationToken(context, "ALFKI", "ANATR", "ANTON", "AROUT", "BERGS", "BLAUS", "BLONP", "BOLID", "BONAP", "BSBEV", "CACTU", "CENTC", "CHOPS", "CONSH", default));
         }
 
+        [ConditionalFact]
+        public virtual void MakeBinary_does_not_throw_for_unsupported_operator()
+        {
+            var query = EF.CompileQuery((NorthwindContext context, object[] parameters)
+                => context.Customers.Where(c => c.CustomerID == (string)parameters[0]));
+
+            using var context = CreateContext();
+
+            var result = query(context, new[] { "ALFKI" }).ToList();
+
+            Assert.Single(result);
+        }
+
         protected NorthwindContext CreateContext() => Fixture.CreateContext();
     }
 }

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

@@ -94,7 +94,7 @@ from e in ss.Set<Employee>().OrderBy(e => e.EmployeeID).Take(5)
                 entryCount: 15);
         }
 
-        [ConditionalTheory]
+        [ConditionalTheory (Skip = "Issue#19247")]
         [MemberData(nameof(IsAsyncData))]
         public virtual async Task Projection_when_arithmetic_mixed_subqueries(bool async)
         {

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

@@ -1,8 +1,11 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
+using System;
 using System.Threading.Tasks;
+using Microsoft.EntityFrameworkCore.Diagnostics;
 using Microsoft.EntityFrameworkCore.TestUtilities;
+using Xunit;
 using Xunit.Abstractions;
 
 namespace Microsoft.EntityFrameworkCore.Query
@@ -322,6 +325,14 @@ FROM [Customers] AS [c]
 WHERE ((((((((((((([c].[CustomerID] = @__s1) OR ([c].[CustomerID] = @__s2)) OR ([c].[CustomerID] = @__s3)) OR ([c].[CustomerID] = @__s4)) OR ([c].[CustomerID] = @__s5)) OR ([c].[CustomerID] = @__s6)) OR ([c].[CustomerID] = @__s7)) OR ([c].[CustomerID] = @__s8)) OR ([c].[CustomerID] = @__s9)) OR ([c].[CustomerID] = @__s10)) OR ([c].[CustomerID] = @__s11)) OR ([c].[CustomerID] = @__s12)) OR ([c].[CustomerID] = @__s13)) OR ([c].[CustomerID] = @__s14)");
         }
 
+        public override void MakeBinary_does_not_throw_for_unsupported_operator()
+        {
+            Assert.Equal(
+                CoreStrings.TranslationFailed("DbSet<Customer>()    .Where(c => c.CustomerID == (string)(__parameters[0]))"),
+                Assert.Throws<InvalidOperationException>(
+                    () => base.MakeBinary_does_not_throw_for_unsupported_operator()).Message.Replace("\r", "").Replace("\n", ""));
+        }
+
         private void AssertSql(params string[] expected)
             => Fixture.TestSqlLoggerFactory.AssertBaseline(expected);
     }

test/EFCore.Sqlite.FunctionalTests/Query/CompiledQueryInMemoryTest.cs

@@ -1,7 +1,10 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
+using System;
+using Microsoft.EntityFrameworkCore.Diagnostics;
 using Microsoft.EntityFrameworkCore.TestUtilities;
+using Xunit;
 
 namespace Microsoft.EntityFrameworkCore.Query
 {
@@ -11,5 +14,13 @@ public NorthwindCompiledQuerySqliteTest(NorthwindQuerySqliteFixture<NoopModelCus
             : base(fixture)
         {
         }
+
+        public override void MakeBinary_does_not_throw_for_unsupported_operator()
+        {
+            Assert.Equal(
+                CoreStrings.TranslationFailed("DbSet<Customer>()    .Where(c => c.CustomerID == (string)(__parameters[0]))"),
+                Assert.Throws<InvalidOperationException>(
+                    () => base.MakeBinary_does_not_throw_for_unsupported_operator()).Message.Replace("\r", "").Replace("\n", ""));
+        }
     }
 }