Fixing some small bugs in sql generation

- removing redundant term in Contains translation with constant argument,
- fixing logic around IN sql gen,
- clean up logic around de morgan optimization,
- adding new optimization op(a, b) == true -> op(a, b),
- removing a number of tests that are no longer relevant due to query pipeline changes.

src/EFCore.Relational/Query/Pipeline/InExpressionValuesExpandingExpressionVisitor.cs

@@ -77,17 +77,21 @@ public override Expression Visit(Expression expression)
  : null;
 
  var nullCheckExpression = hasNullValue
- ? _sqlExpressionFactory.IsNull(inExpression.Item)
+ ? inExpression.Negated
+ ? _sqlExpressionFactory.IsNotNull(inExpression.Item)
+ : _sqlExpressionFactory.IsNull(inExpression.Item)
  : null;
 
  if (updatedInExpression != null && nullCheckExpression != null)
  {
- return _sqlExpressionFactory.OrElse(updatedInExpression, nullCheckExpression);
+ return inExpression.Negated
+ ? _sqlExpressionFactory.AndAlso(updatedInExpression, nullCheckExpression)
+ : _sqlExpressionFactory.OrElse(updatedInExpression, nullCheckExpression);
  }
 
  if (updatedInExpression == null && nullCheckExpression == null)
  {
- return _sqlExpressionFactory.Equal(_sqlExpressionFactory.Constant(true), _sqlExpressionFactory.Constant(false));
+ return _sqlExpressionFactory.Equal(_sqlExpressionFactory.Constant(true), _sqlExpressionFactory.Constant(inExpression.Negated));
  }
 
  return (SqlExpression)updatedInExpression ?? nullCheckExpression;

src/EFCore.Relational/Query/Pipeline/RelationalShapedQueryOptimizingExpressionVisitors.cs

@@ -10,17 +10,16 @@ namespace Microsoft.EntityFrameworkCore.Relational.Query.Pipeline
 {
  public class RelationalShapedQueryOptimizer : ShapedQueryOptimizer
  {
- private readonly QueryCompilationContext _queryCompilationContext;
-
  public RelationalShapedQueryOptimizer(
  QueryCompilationContext queryCompilationContext,
  ISqlExpressionFactory sqlExpressionFactory)
  {
- _queryCompilationContext = queryCompilationContext;
+ UseRelationalNulls = RelationalOptionsExtension.Extract(queryCompilationContext.ContextOptions).UseRelationalNulls;
  SqlExpressionFactory = sqlExpressionFactory;
  }
 
- protected ISqlExpressionFactory SqlExpressionFactory { get; private set; }
+ protected ISqlExpressionFactory SqlExpressionFactory { get; }
+ protected bool UseRelationalNulls { get; }
 
  public override Expression Visit(Expression query)
  {
@@ -29,12 +28,12 @@ public override Expression Visit(Expression query)
  query = new CollectionJoinApplyingExpressionVisitor().Visit(query);
  query = new SelectExpressionTableAliasUniquifyingExpressionVisitor().Visit(query);
 
- if (!RelationalOptionsExtension.Extract(_queryCompilationContext.ContextOptions).UseRelationalNulls)
+ if (!UseRelationalNulls)
  {
  query = new NullSemanticsRewritingVisitor(SqlExpressionFactory).Visit(query);
  }
 
- query = new SqlExpressionOptimizingVisitor(SqlExpressionFactory).Visit(query);
+ query = new SqlExpressionOptimizingVisitor(SqlExpressionFactory, UseRelationalNulls).Visit(query);
  query = new NullComparisonTransformingExpressionVisitor().Visit(query);
 
  if (query is ShapedQueryExpression shapedQueryExpression)

src/EFCore.Relational/Query/Pipeline/SqlExpressionOptimizingVisitor.cs

@@ -10,6 +10,7 @@ namespace Microsoft.EntityFrameworkCore.Query.Pipeline
  public class SqlExpressionOptimizingVisitor : ExpressionVisitor
  {
  private readonly ISqlExpressionFactory _sqlExpressionFactory;
+ private readonly bool _useRelationalNulls;
 
  private static bool TryNegate(ExpressionType expressionType, out ExpressionType result)
  {
@@ -29,9 +30,10 @@ private static bool TryNegate(ExpressionType expressionType, out ExpressionType
  return negated.HasValue;
  }
 
- public SqlExpressionOptimizingVisitor(ISqlExpressionFactory sqlExpressionFactory)
+ public SqlExpressionOptimizingVisitor(ISqlExpressionFactory sqlExpressionFactory, bool useRelationalNulls)
  {
  _sqlExpressionFactory = sqlExpressionFactory;
+ _useRelationalNulls = useRelationalNulls;
  }
 
  protected override Expression VisitExtension(Expression extensionExpression)
@@ -51,15 +53,9 @@ protected override Expression VisitExtension(Expression extensionExpression)
 
  private Expression VisitSqlUnaryExpression(SqlUnaryExpression sqlUnaryExpression)
  {
- // !(true) -> false
- // !(false) -> true
- if (sqlUnaryExpression.OperatorType == ExpressionType.Not
- && sqlUnaryExpression.Operand is SqlConstantExpression innerConstantBool
- && innerConstantBool.Value is bool value)
+ if (sqlUnaryExpression.OperatorType == ExpressionType.Not)
  {
- return value
- ? _sqlExpressionFactory.Constant(false, sqlUnaryExpression.TypeMapping)
- : _sqlExpressionFactory.Constant(true, sqlUnaryExpression.TypeMapping);
+ return VisitNot(sqlUnaryExpression);
  }
 
  // NULL IS NULL -> true
@@ -80,27 +76,6 @@ private Expression VisitSqlUnaryExpression(SqlUnaryExpression sqlUnaryExpression
 
  if (sqlUnaryExpression.Operand is SqlUnaryExpression innerUnary)
  {
- if (sqlUnaryExpression.OperatorType == ExpressionType.Not)
- {
- // !(!a) -> a
- if (innerUnary.OperatorType == ExpressionType.Not)
- {
- return Visit(innerUnary.Operand);
- }
-
- if (innerUnary.OperatorType == ExpressionType.Equal)
- {
- //!(a IS NULL) -> a IS NOT NULL
- return Visit(_sqlExpressionFactory.IsNotNull(innerUnary.Operand));
- }
-
- //!(a IS NOT NULL) -> a IS NULL
- if (innerUnary.OperatorType == ExpressionType.NotEqual)
- {
- return Visit(_sqlExpressionFactory.IsNull(innerUnary.Operand));
- }
- }
-
  // (!a) IS NULL <==> a IS NULL
  if (sqlUnaryExpression.OperatorType == ExpressionType.Equal
  && innerUnary.OperatorType == ExpressionType.Not)
@@ -116,6 +91,47 @@ private Expression VisitSqlUnaryExpression(SqlUnaryExpression sqlUnaryExpression
  }
  }
 
+ var newOperand = (SqlExpression)Visit(sqlUnaryExpression.Operand);
+
+ return sqlUnaryExpression.Update(newOperand);
+ }
+
+ private Expression VisitNot(SqlUnaryExpression sqlUnaryExpression)
+ {
+ // !(true) -> false
+ // !(false) -> true
+ if (sqlUnaryExpression.Operand is SqlConstantExpression innerConstantBool
+ && innerConstantBool.Value is bool value)
+ {
+ return _sqlExpressionFactory.Constant(!value, sqlUnaryExpression.TypeMapping);
+ }
+
+ if (sqlUnaryExpression.Operand is InExpression inExpression)
+ {
+ return Visit(inExpression.Negate());
+ }
+
+ if (sqlUnaryExpression.Operand is SqlUnaryExpression innerUnary)
+ {
+ // !(!a) -> a
+ if (innerUnary.OperatorType == ExpressionType.Not)
+ {
+ return Visit(innerUnary.Operand);
+ }
+
+ if (innerUnary.OperatorType == ExpressionType.Equal)
+ {
+ //!(a IS NULL) -> a IS NOT NULL
+ return Visit(_sqlExpressionFactory.IsNotNull(innerUnary.Operand));
+ }
+
+ //!(a IS NOT NULL) -> a IS NULL
+ if (innerUnary.OperatorType == ExpressionType.NotEqual)
+ {
+ return Visit(_sqlExpressionFactory.IsNull(innerUnary.Operand));
+ }
+ }
+
  if (sqlUnaryExpression.Operand is SqlBinaryExpression innerBinary)
  {
  // De Morgan's
@@ -130,11 +146,11 @@ private Expression VisitSqlUnaryExpression(SqlUnaryExpression sqlUnaryExpression
  : _sqlExpressionFactory.AndAlso(newLeft, newRight);
  }
 
- // note that those optimizations are only valid in 2-value logic
+ // those optimizations are only valid in 2-value logic
  // they are safe to do here because null semantics removes possibility of nulls in the tree
  // however if we decide to do "partial" null semantics (that doesn't distinguish between NULL and FALSE, e.g. for predicates)
  // we need to be extra careful here
- if (TryNegate(innerBinary.OperatorType, out var negated))
+ if (!_useRelationalNulls && TryNegate(innerBinary.OperatorType, out var negated))
  {
  return Visit(
  _sqlExpressionFactory.MakeBinary(
@@ -158,7 +174,6 @@ private Expression VisitSqlBinaryExpression(SqlBinaryExpression sqlBinaryExpress
  if (sqlBinaryExpression.OperatorType == ExpressionType.AndAlso
  || sqlBinaryExpression.OperatorType == ExpressionType.OrElse)
  {
-
  var newLeftConstant = newLeft as SqlConstantExpression;
  var newRightConstant = newRight as SqlConstantExpression;
 
@@ -194,6 +209,31 @@ private Expression VisitSqlBinaryExpression(SqlBinaryExpression sqlBinaryExpress
  return sqlBinaryExpression.Update(newLeft, newRight);
  }
 
+ // those optimizations are only valid in 2-value logic
+ // they are safe to do here because null semantics removes possibility of nulls in the tree
+ // however if we decide to do "partial" null semantics (that doesn't distinguish between NULL and FALSE, e.g. for predicates)
+ // we need to be extra careful here
+ if (!_useRelationalNulls
+ && (sqlBinaryExpression.OperatorType == ExpressionType.Equal || sqlBinaryExpression.OperatorType == ExpressionType.NotEqual))
+ {
+ // op(a, b) == true -> op(a, b)
+ // op(a, b) != false -> op(a, b)
+ // op(a, b) == false -> !op(a, b)
+ // op(a, b) != true -> !op(a, b)
+ var constant = sqlBinaryExpression.Left as SqlConstantExpression ?? sqlBinaryExpression.Right as SqlConstantExpression;
+ var binary = sqlBinaryExpression.Left as SqlBinaryExpression ?? sqlBinaryExpression.Right as SqlBinaryExpression;
+ if (constant != null && binary != null && TryNegate(binary.OperatorType, out var negated))
+ {
+ return (bool)constant.Value == (sqlBinaryExpression.OperatorType == ExpressionType.Equal)
+ ? binary
+ : _sqlExpressionFactory.MakeBinary(
+ negated,
+ sqlBinaryExpression.Left,
+ sqlBinaryExpression.Right,
+ sqlBinaryExpression.TypeMapping);
+ }
+ }
+
  return sqlBinaryExpression.Update(newLeft, newRight);
  }
  }

src/EFCore.SqlServer/Query/Pipeline/SqlServerShapedQueryOptimizingExpressionVisitors.cs

@@ -20,6 +20,7 @@ public override Expression Visit(Expression query)
  {
  query = base.Visit(query);
  query = new SearchConditionConvertingExpressionVisitor(SqlExpressionFactory).Visit(query);
+ query = new SqlExpressionOptimizingVisitor(SqlExpressionFactory, UseRelationalNulls).Visit(query);
 
  return query;
  }

src/EFCore.SqlServer/Query/Pipeline/SqlServerStringMethodTranslator.cs

@@ -216,10 +216,28 @@ public SqlExpression Translate(SqlExpression instance, MethodInfo method, IList<
  {
  var pattern = arguments[0];
  var stringTypeMapping = ExpressionExtensions.InferTypeMapping(instance, pattern);
-
  instance = _sqlExpressionFactory.ApplyTypeMapping(instance, stringTypeMapping);
  pattern = _sqlExpressionFactory.ApplyTypeMapping(pattern, stringTypeMapping);
 
+ if (pattern is SqlConstantExpression constantPattern)
+ {
+ if ((string)constantPattern.Value == string.Empty)
+ {
+ return _sqlExpressionFactory.Constant(true);
+ }
+
+ return _sqlExpressionFactory.GreaterThan(
+ _sqlExpressionFactory.Function(
+ "CHARINDEX",
+ new[]
+ {
+ pattern,
+ instance
+ },
+ typeof(int)),
+ _sqlExpressionFactory.Constant(0));
+ }
+
  return _sqlExpressionFactory.OrElse(
  _sqlExpressionFactory.Equal(
  pattern,

src/EFCore/Query/Pipeline/EntityEqualityRewritingExpressionVisitor.cs

@@ -315,7 +315,7 @@ protected virtual Expression VisitOrderingMethodCall(MethodCallExpression method
  }
 
  var genericMethodDefinition = methodCallExpression.Method.GetGenericMethodDefinition();
- var isFirstOrdering =
+ var firstOrdering =
  genericMethodDefinition == LinqMethodHelpers.QueryableOrderByMethodInfo
  || genericMethodDefinition == LinqMethodHelpers.QueryableOrderByDescendingMethodInfo;
  var isAscending =
@@ -336,28 +336,28 @@ protected virtual Expression VisitOrderingMethodCall(MethodCallExpression method
  body.CreateEFPropertyExpression(keyProperty, makeNullable: false)),
  param);
 
- var orderingMethodInfo = GetOrderingMethodInfo(isFirstOrdering, isAscending);
+ var orderingMethodInfo = GetOrderingMethodInfo(firstOrdering, isAscending);
 
  expression = Expression.Call(
  orderingMethodInfo.MakeGenericMethod(entityType.ClrType, keyProperty.ClrType),
  expression,
  rewrittenKeySelector
  );
 
- isFirstOrdering = false;
+ firstOrdering = false;
  }
 
  return expression;
 
- static MethodInfo GetOrderingMethodInfo(bool isFirstOrdering, bool isAscending)
+ static MethodInfo GetOrderingMethodInfo(bool firstOrdering, bool ascending)
  {
- if (isFirstOrdering)
+ if (firstOrdering)
  {
- return isAscending
+ return ascending
  ? LinqMethodHelpers.QueryableOrderByMethodInfo
  : LinqMethodHelpers.QueryableOrderByDescendingMethodInfo;
  }
- return isAscending
+ return ascending
  ? LinqMethodHelpers.QueryableThenByMethodInfo
  : LinqMethodHelpers.QueryableThenByDescendingMethodInfo;
  }