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

Fixes dotnet#26652
roji · Nov 17, 2021 · 455b94a · 455b94a
1 parent 6070f8d
commit 455b94a
Show file tree

Hide file tree

Showing 10 changed files with 126 additions and 79 deletions.
diff --git a/src/EFCore.Sqlite.Core/Query/Internal/SqliteQuerySqlGenerator.cs b/src/EFCore.Sqlite.Core/Query/Internal/SqliteQuerySqlGenerator.cs
@@ -27,6 +27,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
@@ -1596,6 +1596,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)