Fix to #23990 - Relational: IS (NOT) NULL requires parenthesis when n…

…ot in left side of an equality

We were generating incorrect sql for some cases involving nullability check on nullable bool. E.g. true = someBool IS NULL. Instead we need true = (someBool IS NULL)
Note: this is not really an issue for sql server because of search conditions, but sqlite would yield incorrect data for those queries.

Fixes #23990

src/EFCore.Relational/Query/QuerySqlGenerator.cs

@@ -556,7 +556,12 @@ protected override Expression VisitSqlBinary(SqlBinaryExpression sqlBinaryExpres
  }
 
  private static bool RequiresBrackets(SqlExpression expression)
- => expression is SqlBinaryExpression || expression is LikeExpression;
+ => expression is SqlBinaryExpression
+ || expression is LikeExpression
+ || (expression is SqlUnaryExpression unary
+ && unary.Operand.Type == typeof(bool)
+ && (unary.OperatorType == ExpressionType.Equal
+ || unary.OperatorType == ExpressionType.NotEqual));
 
  /// <inheritdoc />
  protected override Expression VisitSqlConstant(SqlConstantExpression sqlConstantExpression)

test/EFCore.Relational.Specification.Tests/Query/NullSemanticsQueryTestBase.cs

@@ -1546,6 +1546,101 @@ public virtual Task Multiple_non_equality_comparisons_with_null_in_the_middle(bo
  ss => ss.Set<NullSemanticsEntity1>().Where(e => e.NullableIntA != 1 && e.NullableIntA != null && e.NullableIntA != 2));
  }
 
+ [ConditionalTheory]
+ [MemberData(nameof(IsAsyncData))]
+ public virtual async Task Bool_equal_nullable_bool_HasValue(bool async)
+ {
+ await AssertQuery(
+ async,
+ ss => ss.Set<NullSemanticsEntity1>().Where(e => true == e.NullableBoolA.HasValue));
+
+ var prm = false;
+ await AssertQuery(
+ async,
+ ss => ss.Set<NullSemanticsEntity1>().Where(e => prm == e.NullableBoolA.HasValue));
+
+ await AssertQuery(
+ async,
+ ss => ss.Set<NullSemanticsEntity1>().Where(e => e.BoolB == e.NullableBoolA.HasValue));
+ }
+
+ [ConditionalTheory]
+ [MemberData(nameof(IsAsyncData))]
+ public virtual async Task Bool_equal_nullable_bool_compared_to_null(bool async)
+ {
+ await AssertQuery(
+ async,
+ ss => ss.Set<NullSemanticsEntity1>().Where(e => true == (e.NullableBoolA == null)));
+
+ var prm = false;
+ await AssertQuery(
+ async,
+ ss => ss.Set<NullSemanticsEntity1>().Where(e => prm == (e.NullableBoolA != null)));
+ }
+
+ [ConditionalTheory]
+ [MemberData(nameof(IsAsyncData))]
+ public virtual async Task Bool_not_equal_nullable_bool_HasValue(bool async)
+ {
+ await AssertQuery(
+ async,
+ ss => ss.Set<NullSemanticsEntity1>().Where(e => true != e.NullableBoolA.HasValue));
+
+ var prm = false;
+ await AssertQuery(
+ async,
+ ss => ss.Set<NullSemanticsEntity1>().Where(e => prm != e.NullableBoolA.HasValue));
+
+ await AssertQuery(
+ async,
+ ss => ss.Set<NullSemanticsEntity1>().Where(e => e.BoolB != e.NullableBoolA.HasValue));
+ }
+
+ [ConditionalTheory]
+ [MemberData(nameof(IsAsyncData))]
+ public virtual async Task Bool_not_equal_nullable_bool_compared_to_null(bool async)
+ {
+ await AssertQuery(
+ async,
+ ss => ss.Set<NullSemanticsEntity1>().Where(e => true != (e.NullableBoolA == null)));
+
+ var prm = false;
+ await AssertQuery(
+ async,
+ ss => ss.Set<NullSemanticsEntity1>().Where(e => prm != (e.NullableBoolA != null)));
+ }
+
+ [ConditionalTheory]
+ [MemberData(nameof(IsAsyncData))]
+ public virtual async Task Bool_logical_operation_with_nullable_bool_HasValue(bool async)
+ {
+ await AssertQuery(
+ async,
+ ss => ss.Set<NullSemanticsEntity1>().Where(e => true || e.NullableBoolA.HasValue));
+
+ var prm = false;
+ await AssertQuery(
+ async,
+ ss => ss.Set<NullSemanticsEntity1>().Where(e => prm && e.NullableBoolA.HasValue));
+
+ await AssertQuery(
+ async,
+ ss => ss.Set<NullSemanticsEntity1>().Where(e => e.BoolB | e.NullableBoolA.HasValue));
+ }
+
+ [ConditionalTheory]
+ [MemberData(nameof(IsAsyncData))]
+ public virtual async Task Comparison_compared_to_null_check_on_bool(bool async)
+ {
+ await AssertQuery(
+ async,
+ ss => ss.Set<NullSemanticsEntity1>().Where(e => (e.IntA == e.IntB) != e.NullableBoolA.HasValue));
+
+ await AssertQuery(
+ async,
+ ss => ss.Set<NullSemanticsEntity1>().Where(e => (e.IntA != e.IntB) == (e.NullableBoolA != null)));
+ }
+
  [ConditionalTheory]
  [MemberData(nameof(IsAsyncData))]
  public virtual async Task Multiple_non_equality_comparisons_including_null_comparison_work_for_relational_null_semantics(bool async)

test/EFCore.Specification.Tests/TestUtilities/ExpectedQueryRewritingVisitor.cs

@@ -302,7 +302,12 @@ private Expression AddNullProtectionForNonNullableMemberAccess(Expression expres
  instance.Type,
  memberExpression.Type.UnwrapNullableType());
 
- return Expression.Call(methodInfo, instance, maybeLambda);
+ var maybeMethodCall = Expression.Call(methodInfo, instance, maybeLambda);
+
+ return memberExpression.Member.DeclaringType.IsNullableType()
+ && memberExpression.Member.Name == "HasValue"
+ ? Expression.Coalesce(maybeMethodCall, Expression.Constant(false))
+ : maybeMethodCall;
  }
 
  return Visit(expression);

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

@@ -443,7 +443,7 @@ CROSS JOIN [FunkyCustomers] AS [f0]
 WHERE (CASE
  WHEN (([f0].[LastName] = N'') AND [f0].[LastName] IS NOT NULL) OR ([f].[FirstName] IS NOT NULL AND ([f0].[LastName] IS NOT NULL AND (RIGHT([f].[FirstName], LEN([f0].[LastName])) = [f0].[LastName]))) THEN CAST(1 AS bit)
  ELSE CAST(0 AS bit)
-END <> [f].[NullableBool]) OR [f].[NullableBool] IS NULL");
+END <> [f].[NullableBool]) OR ([f].[NullableBool] IS NULL)");
  }
 
  public override async Task String_FirstOrDefault_and_LastOrDefault(bool async)

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

@@ -4518,7 +4518,7 @@ INNER JOIN (
  FROM [Factions] AS [f]
  WHERE [f].[Name] = N'Swarm'
 ) AS [t] ON [l].[Name] = [t].[CommanderName]
-WHERE ([t].[Eradicated] <> CAST(1 AS bit)) OR [t].[Eradicated] IS NULL");
+WHERE ([t].[Eradicated] <> CAST(1 AS bit)) OR ([t].[Eradicated] IS NULL)");
  }
 
  public override async Task Null_semantics_on_nullable_bool_from_left_join_subquery_is_fully_applied(bool async)
@@ -4533,7 +4533,7 @@ LEFT JOIN (
  FROM [Factions] AS [f]
  WHERE [f].[Name] = N'Swarm'
 ) AS [t] ON [l].[Name] = [t].[CommanderName]
-WHERE ([t].[Eradicated] <> CAST(1 AS bit)) OR [t].[Eradicated] IS NULL");
+WHERE ([t].[Eradicated] <> CAST(1 AS bit)) OR ([t].[Eradicated] IS NULL)");
  }
 
  public override async Task Include_on_derived_type_with_order_by_and_paging(bool async)
@@ -6237,7 +6237,7 @@ public override async Task Nullable_bool_comparison_is_translated_to_server(bool
 
  AssertSql(
  @"SELECT CASE
- WHEN ([f].[Eradicated] = CAST(1 AS bit)) AND [f].[Eradicated] IS NOT NULL THEN CAST(1 AS bit)
+ WHEN ([f].[Eradicated] = CAST(1 AS bit)) AND ([f].[Eradicated] IS NOT NULL) THEN CAST(1 AS bit)
  ELSE CAST(0 AS bit)
 END AS [IsEradicated]
 FROM [Factions] AS [f]");
@@ -6697,10 +6697,10 @@ FROM [LocustLeaders] AS [l]
 WHERE (CASE
  WHEN [f].[Name] = N'Locust' THEN CAST(1 AS bit)
  ELSE NULL
-END <> CAST(1 AS bit)) OR CASE
+END <> CAST(1 AS bit)) OR (CASE
  WHEN [f].[Name] = N'Locust' THEN CAST(1 AS bit)
  ELSE NULL
-END IS NULL");
+END IS NULL)");
  }
 
  public override async Task Byte_array_contains_literal(bool async)