StringCompareExpression throws 100% of the times

[thatotherone]

This is related to #1767
Discovered in EntityFramework.Relational 7.0.0-beta8, but is present in current master as far as I can tell.

When string.Compare() is used in the Where clause of relational DbSet it ends up throwing:

error   : [Microsoft.AspNet.Diagnostics.ExceptionHandlerMiddleware] An unhandled exception has occurred: Extension node must override the property Expression.Type.
System.InvalidOperationException: Extension node must override the property Expression.Type.
   at System.Linq.Expressions.Expression.get_Type()
   at System.Linq.Expressions.Expression.AndAlso(Expression left, Expression right, MethodInfo method)
   at Microsoft.Data.Entity.Query.RelationalQueryModelVisitor.VisitWhereClause(WhereClause whereClause, QueryModel queryModel, Int32 index)
   at Remotion.Linq.QueryModelVisitorBase.VisitBodyClauses(ObservableCollection`1 bodyClauses, QueryModel queryModel)
   at Remotion.Linq.QueryModelVisitorBase.VisitQueryModel(QueryModel queryModel)
   at Microsoft.Data.Entity.Query.EntityQueryModelVisitor.VisitQueryModel(QueryModel queryModel)
   at Microsoft.Data.Entity.Query.RelationalQueryModelVisitor.VisitQueryModel(QueryModel queryModel)
   at Microsoft.Data.Entity.Query.Internal.SqlServerQueryModelVisitor.VisitQueryModel(QueryModel queryModel)
   at Microsoft.Data.Entity.Query.EntityQueryModelVisitor.CreateQueryExecutor[TResult](QueryModel queryModel)
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at Microsoft.Data.Entity.Query.QueryCompiler.<>c__DisplayClass16_0`1.<CompileQuery>b__0()
   at Microsoft.Data.Entity.Query.Internal.CompiledQueryCache.GetOrAddQuery[TResult](Object cacheKey, Func`1 compiler)
   at Microsoft.Data.Entity.Query.QueryCompiler.Execute[TResult](Expression query)

It appears that StringCompareExpression doesn't override its return type. Fix is simple:

diff --git a/src/EntityFramework.Relational/Query/Expressions/StringCompareExpression.cs b/src/EntityFramework.Relational/Query/Expressions/StringCompareExpression.cs
index b48689e..0a86498 100644
--- a/src/EntityFramework.Relational/Query/Expressions/StringCompareExpression.cs
+++ b/src/EntityFramework.Relational/Query/Expressions/StringCompareExpression.cs
@@ -1,6 +1,7 @@
 // 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.Linq.Expressions;
 using JetBrains.Annotations;
 using Microsoft.Data.Entity.Query.Sql;
@@ -23,6 +24,8 @@ namespace Microsoft.Data.Entity.Query.Expressions

         public override ExpressionType NodeType => ExpressionType.Extension;

+        public override Type Type => typeof(bool);
+
         public virtual ExpressionType Operator => _op;

         public virtual Expression Left => _left;

[maumar]

@thatotherone can you provide a sample query that produces the problem? We have a bunch of tests for string.Compare that uses it in Where clause and they did not caught it. I would like to add some new cases to the test suite.

[thatotherone]

Sorry, my initial triage wasn't completely accurate. Problem is not just any string.Compare in any Where, but only when string.Compare appears in any Where after the first one (Expression.AndAlso on the stack trace). Here's a test update that is red initially and becomes green with a fix:

diff --git a/src/EntityFramework.Relational/Query/Expressions/StringCompareExpression.cs b/src/EntityFramework.Relational/Query/Expressions/StringCompareExpression.cs
index b48689e..0a86498 100644
--- a/src/EntityFramework.Relational/Query/Expressions/StringCompareExpression.cs
+++ b/src/EntityFramework.Relational/Query/Expressions/StringCompareExpression.cs
@@ -1,6 +1,7 @@
 // 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.Linq.Expressions;
 using JetBrains.Annotations;
 using Microsoft.Data.Entity.Query.Sql;
@@ -23,6 +24,8 @@ namespace Microsoft.Data.Entity.Query.Expressions

         public override ExpressionType NodeType => ExpressionType.Extension;

+        public override Type Type => typeof(bool);
+
         public virtual ExpressionType Operator => _op;

         public virtual Expression Left => _left;
diff --git a/test/EntityFramework.Core.FunctionalTests/QueryTestBase.cs b/test/EntityFramework.Core.FunctionalTests/QueryTestBase.cs
index a5d21d1..85c6b15 100644
--- a/test/EntityFramework.Core.FunctionalTests/QueryTestBase.cs
+++ b/test/EntityFramework.Core.FunctionalTests/QueryTestBase.cs
@@ -3491,6 +3491,18 @@ namespace Microsoft.Data.Entity.FunctionalTests
                 entryCount: 91);
         }

+        [Fact]
+        public virtual void String_Compare_multi_predicate()
+        {
+            AssertQuery<Customer>(
+                cs => cs.Where(c => string.Compare(c.CustomerID, "ALFKI") > -1).Where(c => string.Compare(c.CustomerID, "CACTU") == -1),
+                entryCount: 11);
+
+            AssertQuery<Customer>(
+                cs => cs.Where(c => string.Compare(c.ContactTitle, "Owner") == 0).Where(c => string.Compare(c.Country, "USA") != 0),
+                entryCount: 15);
+        }
+
         protected static string LocalMethod1()
         {
             return "M";
diff --git a/test/EntityFramework.SqlServer.FunctionalTests/QuerySqlServerTest.cs b/test/EntityFramework.SqlServer.FunctionalTests/QuerySqlServerTest.cs
index 5df9140..f855f62 100644
--- a/test/EntityFramework.SqlServer.FunctionalTests/QuerySqlServerTest.cs
+++ b/test/EntityFramework.SqlServer.FunctionalTests/QuerySqlServerTest.cs
@@ -3177,6 +3177,21 @@ WHERE [c].[CustomerID] < REPLACE('ALFKI', UPPER('ALF'), [c].[CustomerID])",
                 Sql);
         }

+        public override void String_Compare_multi_predicate()
+        {
+            base.String_Compare_multi_predicate();
+
+            Assert.Equal(
+                @"SELECT [c].[CustomerID], [c].[Address], [c].[City], [c].[CompanyName], [c].[ContactName], [c].[ContactTitle], [c].[Country], [c].[Fax], [c].[Phone], [c].[PostalCode], [c].[Region]
+FROM [Customers] AS [c]
+WHERE [c].[CustomerID] >= 'ALFKI' AND [c].[CustomerID] < 'CACTU'
+
+SELECT [c].[CustomerID], [c].[Address], [c].[City], [c].[CompanyName], [c].[ContactName], [c].[ContactTitle], [c].[Country], [c].[Fax], [c].[Phone], [c].[PostalCode], [c].[Region]
+FROM [Customers] AS [c]
+WHERE [c].[ContactTitle] = 'Owner' AND [c].[Country] <> 'USA'",
+                Sql);
+        }
+
         public override void Where_math_abs1()
         {
             base.Where_math_abs1();

[thatotherone]

@maumar also, these fail with different code paths, but with the same root cause

cs.Where(c => true && string.Compare(c.CustomerID, "ALFKI") > -1);
cs.Where(c => false || string.Compare(c.CustomerID, "ALFKI") > -1);

[maumar]

@thatotherone I just got around to look at this and the fix and the tests you proposed look good. If you like to contribute to EF, go ahead and submit a pull request and I will quickly process it. If you rather not or are unable to sign CLA please let me know and I will make the change instead. Thanks!

[thatotherone]

@maumar Please, go ahead and check this in. Thanks

[maumar]

fixed in bf9301d

[Villason]

Shouldn't this bug be reopened? I am having it with with RC1 code: #5369

[smitpatel]

@Villason - This bug has been fixed in RC2. Can you upgrade to RC2 and see if you are still seeing it.