Put parentheses around IS NULL for non-bool operands in SQLite

Fixes dotnet#26652
roji · Nov 17, 2021 · 12dc1a2 · 12dc1a2
1 parent cf4b213
commit 12dc1a2
Show file tree

Hide file tree

Showing 9 changed files with 87 additions and 40 deletions.
diff --git a/src/EFCore.Sqlite.Core/Query/Internal/SqliteQuerySqlGenerator.cs b/src/EFCore.Sqlite.Core/Query/Internal/SqliteQuerySqlGenerator.cs
@@ -26,6 +26,35 @@ public SqliteQuerySqlGenerator(QuerySqlGeneratorDependencies dependencies)
         {
         }
 
+        /// <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 VisitSqlUnary(SqlUnaryExpression sqlUnaryExpression)
+        {
+            switch (sqlUnaryExpression.OperatorType)
+            {
+                // EF uses unary Equal and NotEqual to represent is-null checking.
+                // These need to be surrounded in parentheses in various cases (e.g. where TRUE = x IS NOT NULL),
+                // see
+                case ExpressionType.Equal:
+                    Sql.Append("(");
+                    Visit(sqlUnaryExpression.Operand);
+                    Sql.Append(" IS NULL)");
+                    return sqlUnaryExpression;
+
+                case ExpressionType.NotEqual:
+                    Sql.Append("(");
+                    Visit(sqlUnaryExpression.Operand);
+                    Sql.Append(" IS NOT NULL)");
+                    return sqlUnaryExpression;
+            }
+
+            return base.VisitSqlUnary(sqlUnaryExpression);
+        }
+
         /// <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

diff --git a/test/EFCore.Relational.Specification.Tests/Query/NullSemanticsQueryTestBase.cs b/test/EFCore.Relational.Specification.Tests/Query/NullSemanticsQueryTestBase.cs
@@ -1606,6 +1606,24 @@ await AssertQuery(
                 ss => ss.Set<NullSemanticsEntity1>().Where(e => e.BoolB != e.NullableBoolA.HasValue));
         }
 
+        [ConditionalTheory]
+        [MemberData(nameof(IsAsyncData))]
+        public virtual async Task Bool_not_equal_nullable_int_HasValue(bool async)
+        {
+            await AssertQuery(
+                async,
+                ss => ss.Set<NullSemanticsEntity1>().Where(e => true != e.NullableIntA.HasValue));
+
+            var prm = false;
+            await AssertQuery(
+                async,
+                ss => ss.Set<NullSemanticsEntity1>().Where(e => prm != e.NullableIntA.HasValue));
+
+            await AssertQuery(
+                async,
+                ss => ss.Set<NullSemanticsEntity1>().Where(e => e.BoolB != e.NullableIntA.HasValue));
+        }
+
         [ConditionalTheory]
         [MemberData(nameof(IsAsyncData))]
         public virtual async Task Bool_not_equal_nullable_bool_compared_to_null(bool async)

diff --git a/test/EFCore.Sqlite.FunctionalTests/Query/GearsOfWarQuerySqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/Query/GearsOfWarQuerySqliteTest.cs
@@ -217,7 +217,7 @@ public override async Task Negate_on_like_expression(bool async)
             AssertSql(
                 @"SELECT ""s"".""Id"", ""s"".""Banner"", ""s"".""Banner5"", ""s"".""InternalNumber"", ""s"".""Name""
 FROM ""Squads"" AS ""s""
-WHERE ""s"".""Name"" IS NOT NULL AND NOT (""s"".""Name"" LIKE 'us%')");
+WHERE (""s"".""Name"" IS NOT NULL) AND NOT (""s"".""Name"" LIKE 'us%')");
         }
 
         public override async Task Select_datetimeoffset_comparison_in_projection(bool async)

diff --git a/test/EFCore.Sqlite.FunctionalTests/Query/NorthwindFunctionsQuerySqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/Query/NorthwindFunctionsQuerySqliteTest.cs
@@ -184,7 +184,7 @@ public override async Task String_StartsWith_Literal(bool async)
             AssertSql(
                 @"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"".""ContactName"" IS NOT NULL AND (""c"".""ContactName"" LIKE 'M%')");
+WHERE (""c"".""ContactName"" IS NOT NULL) AND (""c"".""ContactName"" LIKE 'M%')");
         }
 
         public override async Task String_StartsWith_Identity(bool async)
@@ -194,7 +194,7 @@ public override async Task String_StartsWith_Identity(bool async)
             AssertSql(
                 @"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"".""ContactName"" = '') OR (""c"".""ContactName"" IS NOT NULL AND (((""c"".""ContactName"" LIKE ""c"".""ContactName"" || '%') AND (substr(""c"".""ContactName"", 1, length(""c"".""ContactName"")) = ""c"".""ContactName"")) OR (""c"".""ContactName"" = '')))");
+WHERE (""c"".""ContactName"" = '') OR ((""c"".""ContactName"" IS NOT NULL) AND (((""c"".""ContactName"" LIKE ""c"".""ContactName"" || '%') AND (substr(""c"".""ContactName"", 1, length(""c"".""ContactName"")) = ""c"".""ContactName"")) OR (""c"".""ContactName"" = '')))");
         }
 
         public override async Task String_StartsWith_Column(bool async)
@@ -204,7 +204,7 @@ public override async Task String_StartsWith_Column(bool async)
             AssertSql(
                 @"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"".""ContactName"" = '') OR (""c"".""ContactName"" IS NOT NULL AND (((""c"".""ContactName"" LIKE ""c"".""ContactName"" || '%') AND (substr(""c"".""ContactName"", 1, length(""c"".""ContactName"")) = ""c"".""ContactName"")) OR (""c"".""ContactName"" = '')))");
+WHERE (""c"".""ContactName"" = '') OR ((""c"".""ContactName"" IS NOT NULL) AND (((""c"".""ContactName"" LIKE ""c"".""ContactName"" || '%') AND (substr(""c"".""ContactName"", 1, length(""c"".""ContactName"")) = ""c"".""ContactName"")) OR (""c"".""ContactName"" = '')))");
         }
 
         public override async Task String_StartsWith_MethodCall(bool async)
@@ -214,7 +214,7 @@ public override async Task String_StartsWith_MethodCall(bool async)
             AssertSql(
                 @"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"".""ContactName"" IS NOT NULL AND (""c"".""ContactName"" LIKE 'M%')");
+WHERE (""c"".""ContactName"" IS NOT NULL) AND (""c"".""ContactName"" LIKE 'M%')");
         }
 
         public override async Task String_EndsWith_Literal(bool async)
@@ -224,7 +224,7 @@ public override async Task String_EndsWith_Literal(bool async)
             AssertSql(
                 @"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"".""ContactName"" IS NOT NULL AND (""c"".""ContactName"" LIKE '%b')");
+WHERE (""c"".""ContactName"" IS NOT NULL) AND (""c"".""ContactName"" LIKE '%b')");
         }
 
         public override async Task String_EndsWith_Identity(bool async)
@@ -234,7 +234,7 @@ public override async Task String_EndsWith_Identity(bool async)
             AssertSql(
                 @"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"".""ContactName"" = '') OR (""c"".""ContactName"" IS NOT NULL AND ((substr(""c"".""ContactName"", -length(""c"".""ContactName"")) = ""c"".""ContactName"") OR (""c"".""ContactName"" = '')))");
+WHERE (""c"".""ContactName"" = '') OR ((""c"".""ContactName"" IS NOT NULL) AND ((substr(""c"".""ContactName"", -length(""c"".""ContactName"")) = ""c"".""ContactName"") OR (""c"".""ContactName"" = '')))");
         }
 
         public override async Task String_EndsWith_Column(bool async)
@@ -244,7 +244,7 @@ public override async Task String_EndsWith_Column(bool async)
             AssertSql(
                 @"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"".""ContactName"" = '') OR (""c"".""ContactName"" IS NOT NULL AND ((substr(""c"".""ContactName"", -length(""c"".""ContactName"")) = ""c"".""ContactName"") OR (""c"".""ContactName"" = '')))");
+WHERE (""c"".""ContactName"" = '') OR ((""c"".""ContactName"" IS NOT NULL) AND ((substr(""c"".""ContactName"", -length(""c"".""ContactName"")) = ""c"".""ContactName"") OR (""c"".""ContactName"" = '')))");
         }
 
         public override async Task String_EndsWith_MethodCall(bool async)
@@ -254,7 +254,7 @@ public override async Task String_EndsWith_MethodCall(bool async)
             AssertSql(
                 @"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"".""ContactName"" IS NOT NULL AND (""c"".""ContactName"" LIKE '%m')");
+WHERE (""c"".""ContactName"" IS NOT NULL) AND (""c"".""ContactName"" LIKE '%m')");
         }
 
         public override async Task String_Contains_Literal(bool async)
@@ -322,7 +322,7 @@ public override async Task IsNullOrWhiteSpace_in_predicate(bool async)
             AssertSql(
                 @"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"".""Region"" IS NULL OR (trim(""c"".""Region"") = '')");
+WHERE (""c"".""Region"" IS NULL) OR (trim(""c"".""Region"") = '')");
         }
 
         public override async Task Indexof_with_emptystring(bool async)
@@ -624,15 +624,15 @@ public override async Task IsNullOrEmpty_in_predicate(bool async)
             AssertSql(
                 @"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"".""Region"" IS NULL OR (""c"".""Region"" = '')");
+WHERE (""c"".""Region"" IS NULL) OR (""c"".""Region"" = '')");
         }
 
         public override async Task IsNullOrEmpty_in_projection(bool async)
         {
             await base.IsNullOrEmpty_in_projection(async);
 
             AssertSql(
-                @"SELECT ""c"".""CustomerID"" AS ""Id"", ""c"".""Region"" IS NULL OR (""c"".""Region"" = '') AS ""Value""
+                @"SELECT ""c"".""CustomerID"" AS ""Id"", (""c"".""Region"" IS NULL) OR (""c"".""Region"" = '') AS ""Value""
 FROM ""Customers"" AS ""c""");
         }
 
@@ -643,7 +643,7 @@ public override async Task IsNullOrEmpty_negated_in_predicate(bool async)
             AssertSql(
                 @"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"".""Region"" IS NOT NULL AND (""c"".""Region"" <> '')");
+WHERE (""c"".""Region"" IS NOT NULL) AND (""c"".""Region"" <> '')");
         }
 
         public override Task Datetime_subtraction_TotalDays(bool async)

diff --git a/test/EFCore.Sqlite.FunctionalTests/Query/NorthwindMiscellaneousQuerySqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/Query/NorthwindMiscellaneousQuerySqliteTest.cs
@@ -1,4 +1,4 @@
-// Licensed to the .NET Foundation under one or more agreements.
+// Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System;
@@ -32,7 +32,7 @@ public override async Task Query_expression_with_to_string_and_contains(bool asy
             AssertSql(
                 @"SELECT ""o"".""CustomerID""
 FROM ""Orders"" AS ""o""
-WHERE ""o"".""OrderDate"" IS NOT NULL AND (('10' = '') OR (instr(CAST(""o"".""EmployeeID"" AS TEXT), '10') > 0))");
+WHERE (""o"".""OrderDate"" IS NOT NULL) AND (('10' = '') OR (instr(CAST(""o"".""EmployeeID"" AS TEXT), '10') > 0))");
         }
 
         public override async Task Take_Skip(bool async)
@@ -87,7 +87,7 @@ public override async Task Select_expression_date_add_year(bool async)
             AssertSql(
                 @"SELECT rtrim(rtrim(strftime('%Y-%m-%d %H:%M:%f', ""o"".""OrderDate"", CAST(1 AS TEXT) || ' years'), '0'), '.') AS ""OrderDate""
 FROM ""Orders"" AS ""o""
-WHERE ""o"".""OrderDate"" IS NOT NULL");
+WHERE (""o"".""OrderDate"" IS NOT NULL)");
         }
 
         [ConditionalTheory(Skip = "issue #25851")]
@@ -108,7 +108,7 @@ public override async Task Select_expression_datetime_add_hour(bool async)
             AssertSql(
                 @"SELECT rtrim(rtrim(strftime('%Y-%m-%d %H:%M:%f', ""o"".""OrderDate"", CAST(1.0 AS TEXT) || ' hours'), '0'), '.') AS ""OrderDate""
 FROM ""Orders"" AS ""o""
-WHERE ""o"".""OrderDate"" IS NOT NULL");
+WHERE (""o"".""OrderDate"" IS NOT NULL)");
         }
 
         public override async Task Select_expression_datetime_add_minute(bool async)
@@ -118,7 +118,7 @@ public override async Task Select_expression_datetime_add_minute(bool async)
             AssertSql(
                 @"SELECT rtrim(rtrim(strftime('%Y-%m-%d %H:%M:%f', ""o"".""OrderDate"", CAST(1.0 AS TEXT) || ' minutes'), '0'), '.') AS ""OrderDate""
 FROM ""Orders"" AS ""o""
-WHERE ""o"".""OrderDate"" IS NOT NULL");
+WHERE (""o"".""OrderDate"" IS NOT NULL)");
         }
 
         public override async Task Select_expression_datetime_add_second(bool async)
@@ -128,7 +128,7 @@ public override async Task Select_expression_datetime_add_second(bool async)
             AssertSql(
                 @"SELECT rtrim(rtrim(strftime('%Y-%m-%d %H:%M:%f', ""o"".""OrderDate"", CAST(1.0 AS TEXT) || ' seconds'), '0'), '.') AS ""OrderDate""
 FROM ""Orders"" AS ""o""
-WHERE ""o"".""OrderDate"" IS NOT NULL");
+WHERE (""o"".""OrderDate"" IS NOT NULL)");
         }
 
         [ConditionalTheory(Skip = "issue #25851")]
@@ -149,7 +149,7 @@ public override async Task Select_expression_date_add_milliseconds_above_the_ran
             AssertSql(
                 @"SELECT rtrim(rtrim(strftime('%Y-%m-%d %H:%M:%f', ""o"".""OrderDate"", CAST((1000000000000.0 / 1000.0) AS TEXT) || ' seconds'), '0'), '.') AS ""OrderDate""
 FROM ""Orders"" AS ""o""
-WHERE ""o"".""OrderDate"" IS NOT NULL");
+WHERE (""o"".""OrderDate"" IS NOT NULL)");
         }
 
         public override async Task Select_expression_date_add_milliseconds_below_the_range(bool async)
@@ -159,7 +159,7 @@ public override async Task Select_expression_date_add_milliseconds_below_the_ran
             AssertSql(
                 @"SELECT rtrim(rtrim(strftime('%Y-%m-%d %H:%M:%f', ""o"".""OrderDate"", CAST((-1000000000000.0 / 1000.0) AS TEXT) || ' seconds'), '0'), '.') AS ""OrderDate""
 FROM ""Orders"" AS ""o""
-WHERE ""o"".""OrderDate"" IS NOT NULL");
+WHERE (""o"".""OrderDate"" IS NOT NULL)");
         }
 
         public override async Task Select_expression_date_add_milliseconds_large_number_divided(bool async)
@@ -171,7 +171,7 @@ public override async Task Select_expression_date_add_milliseconds_large_number_
 
 SELECT rtrim(rtrim(strftime('%Y-%m-%d %H:%M:%f', ""o"".""OrderDate"", COALESCE(CAST(CAST((CAST(((CAST(strftime('%f', ""o"".""OrderDate"") AS REAL) * 1000.0) % 1000.0) AS INTEGER) / @__millisecondsPerDay_0) AS REAL) AS TEXT), '') || ' days', COALESCE(CAST((CAST((CAST(((CAST(strftime('%f', ""o"".""OrderDate"") AS REAL) * 1000.0) % 1000.0) AS INTEGER) % @__millisecondsPerDay_0) AS REAL) / 1000.0) AS TEXT), '') || ' seconds'), '0'), '.') AS ""OrderDate""
 FROM ""Orders"" AS ""o""
-WHERE ""o"".""OrderDate"" IS NOT NULL");
+WHERE (""o"".""OrderDate"" IS NOT NULL)");
         }
 
         public override async Task Add_minutes_on_constant_value(bool async)

diff --git a/test/EFCore.Sqlite.FunctionalTests/Query/NorthwindQueryFiltersQuerySqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/Query/NorthwindQueryFiltersQuerySqliteTest.cs
@@ -1,4 +1,4 @@
-// Licensed to the .NET Foundation under one or more agreements.
+// Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Threading.Tasks;
@@ -27,7 +27,7 @@ public override async Task Count_query(bool async)
 
 SELECT COUNT(*)
 FROM ""Customers"" AS ""c""
-WHERE (@__ef_filter__TenantPrefix_0 = '') OR (""c"".""CompanyName"" IS NOT NULL AND (((""c"".""CompanyName"" LIKE @__ef_filter__TenantPrefix_0 || '%') AND (substr(""c"".""CompanyName"", 1, length(@__ef_filter__TenantPrefix_0)) = @__ef_filter__TenantPrefix_0)) OR (@__ef_filter__TenantPrefix_0 = '')))");
+WHERE (@__ef_filter__TenantPrefix_0 = '') OR ((""c"".""CompanyName"" IS NOT NULL) AND (((""c"".""CompanyName"" LIKE @__ef_filter__TenantPrefix_0 || '%') AND (substr(""c"".""CompanyName"", 1, length(@__ef_filter__TenantPrefix_0)) = @__ef_filter__TenantPrefix_0)) OR (@__ef_filter__TenantPrefix_0 = '')))");
         }
 
         private void AssertSql(params string[] expected)

diff --git a/test/EFCore.Sqlite.FunctionalTests/Query/NorthwindWhereQuerySqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/Query/NorthwindWhereQuerySqliteTest.cs
@@ -191,7 +191,7 @@ public override async Task Where_string_indexof(bool async)
             AssertSql(
                 @"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 ((instr(""c"".""City"", 'Sea') - 1) <> -1) OR ""c"".""City"" IS NULL");
+WHERE ((instr(""c"".""City"", 'Sea') - 1) <> -1) OR (""c"".""City"" IS NULL)");
         }
 
         public override async Task Where_string_replace(bool async)