Better management of negated expressions (#31028)

Closes #31027

src/EFCore.Cosmos/Query/Internal/CosmosContainsTranslator.cs

@@ -40,15 +40,15 @@ public CosmosContainsTranslator(ISqlExpressionFactory sqlExpressionFactory)
             && method.GetGenericMethodDefinition().Equals(EnumerableMethods.Contains)
             && ValidateValues(arguments[0]))
         {
-            return _sqlExpressionFactory.In(arguments[1], arguments[0], false);
+            return _sqlExpressionFactory.In(arguments[1], arguments[0]);
         }
 
         if (arguments.Count == 1
             && method.IsContainsMethod()
             && instance != null
             && ValidateValues(instance))
         {
-            return _sqlExpressionFactory.In(arguments[0], instance, false);
+            return _sqlExpressionFactory.In(arguments[0], instance);
         }
 
         return null;

src/EFCore.Cosmos/Query/Internal/CosmosShapedQueryCompilingExpressionVisitor.InExpressionValuesExpandingExpressionVisitor.cs

@@ -70,8 +70,7 @@ public override Expression Visit(Expression expression)
                 var updatedInExpression = inValues.Count > 0
                     ? _sqlExpressionFactory.In(
                         (SqlExpression)Visit(inExpression.Item),
-                        _sqlExpressionFactory.Constant(inValues, typeMapping),
-                        inExpression.IsNegated)
+                        _sqlExpressionFactory.Constant(inValues, typeMapping))
                     : null;
 
                 var nullCheckExpression = hasNullValue

src/EFCore.Cosmos/Query/Internal/CosmosSqlTranslatingExpressionVisitor.cs

@@ -704,8 +704,7 @@ protected override Expression VisitTypeBinary(TypeBinaryExpression typeBinaryExp
                         _sqlExpressionFactory.Constant(concreteEntityTypes[0].GetDiscriminatorValue()))
                     : _sqlExpressionFactory.In(
                         discriminatorColumn,
-                        _sqlExpressionFactory.Constant(concreteEntityTypes.Select(et => et.GetDiscriminatorValue()).ToList()),
-                        negated: false);
+                        _sqlExpressionFactory.Constant(concreteEntityTypes.Select(et => et.GetDiscriminatorValue()).ToList()));
             }
         }
 

src/EFCore.Cosmos/Query/Internal/ISqlExpressionFactory.cs

@@ -256,7 +256,7 @@ SqlConditionalExpression Condition(
     ///     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>
-    InExpression In(SqlExpression item, SqlExpression values, bool negated);
+    InExpression In(SqlExpression item, SqlExpression values);
 
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to

src/EFCore.Cosmos/Query/Internal/InExpression.cs

@@ -19,13 +19,11 @@ public class InExpression : SqlExpression
     /// </summary>
     public InExpression(
         SqlExpression item,
-        bool negated,
         SqlExpression values,
         CoreTypeMapping typeMapping)
         : base(typeof(bool), typeMapping)
     {
         Item = item;
-        IsNegated = negated;
         Values = values;
     }
 
@@ -37,14 +35,6 @@ public InExpression(
     /// </summary>
     public virtual SqlExpression Item { get; }
 
-    /// <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>
-    public virtual bool IsNegated { get; }
-
     /// <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
@@ -67,15 +57,6 @@ protected override Expression VisitChildren(ExpressionVisitor visitor)
         return Update(newItem, values);
     }
 
-    /// <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>
-    public virtual InExpression Negate()
-        => new(Item, !IsNegated, Values, TypeMapping!);
-
     /// <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
@@ -84,7 +65,7 @@ public virtual InExpression Negate()
     /// </summary>
     public virtual InExpression Update(SqlExpression item, SqlExpression values)
         => item != Item || values != Values
-            ? new InExpression(item, IsNegated, values, TypeMapping!)
+            ? new InExpression(item, values, TypeMapping!)
             : this;
 
     /// <summary>
@@ -96,7 +77,7 @@ public virtual InExpression Update(SqlExpression item, SqlExpression values)
     protected override void Print(ExpressionPrinter expressionPrinter)
     {
         expressionPrinter.Visit(Item);
-        expressionPrinter.Append(IsNegated ? " NOT IN " : " IN ");
+        expressionPrinter.Append(" IN ");
         expressionPrinter.Append("(");
         expressionPrinter.Visit(Values);
         expressionPrinter.Append(")");
@@ -117,7 +98,6 @@ public override bool Equals(object? obj)
     private bool Equals(InExpression inExpression)
         => base.Equals(inExpression)
             && Item.Equals(inExpression.Item)
-            && IsNegated.Equals(inExpression.IsNegated)
             && Values.Equals(inExpression.Values);
 
     /// <summary>
@@ -127,5 +107,5 @@ private bool Equals(InExpression inExpression)
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
     public override int GetHashCode()
-        => HashCode.Combine(base.GetHashCode(), Item, IsNegated, Values);
+        => HashCode.Combine(base.GetHashCode(), Item, Values);
 }

src/EFCore.Cosmos/Query/Internal/QuerySqlGenerator.cs

@@ -387,6 +387,13 @@ protected override Expression VisitSqlUnary(SqlUnaryExpression sqlUnaryExpressio
         if (sqlUnaryExpression.OperatorType == ExpressionType.Not
             && sqlUnaryExpression.Operand.Type == typeof(bool))
         {
+            if (sqlUnaryExpression.Operand is InExpression inExpression)
+            {
+                GenerateIn(inExpression, negated: true);
+
+                return sqlUnaryExpression;
+            }
+
             op = "NOT";
         }
 
@@ -506,18 +513,29 @@ protected override Expression VisitSqlParameter(SqlParameterExpression sqlParame
     ///     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 VisitIn(InExpression inExpression)
+    protected sealed override Expression VisitIn(InExpression inExpression)
+    {
+        GenerateIn(inExpression, negated: false);
+
+        return inExpression;
+    }
+
+    /// <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 virtual void GenerateIn(InExpression inExpression, bool negated)
     {
         Visit(inExpression.Item);
-        _sqlBuilder.Append(inExpression.IsNegated ? " NOT IN " : " IN ");
+        _sqlBuilder.Append(negated ? " NOT IN " : " IN ");
         _sqlBuilder.Append('(');
         var valuesConstant = (SqlConstantExpression)inExpression.Values;
         var valuesList = ((IEnumerable<object>)valuesConstant.Value)
             .Select(v => new SqlConstantExpression(Expression.Constant(v), valuesConstant.TypeMapping)).ToList();
         GenerateList(valuesList, e => Visit(e));
         _sqlBuilder.Append(')');
-
-        return inExpression;
     }
 
     /// <summary>

src/EFCore.Cosmos/Query/Internal/SqlExpressionFactory.cs

@@ -473,14 +473,14 @@ public virtual SqlConditionalExpression Condition(SqlExpression test, SqlExpress
     ///     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>
-    public virtual InExpression In(SqlExpression item, SqlExpression values, bool negated)
+    public virtual InExpression In(SqlExpression item, SqlExpression values)
     {
         var typeMapping = item.TypeMapping ?? _typeMappingSource.FindMapping(item.Type, _model);
 
         item = ApplyTypeMapping(item, typeMapping);
         values = ApplyTypeMapping(values, typeMapping);
 
-        return new InExpression(item, negated, values, _boolTypeMapping);
+        return new InExpression(item, values, _boolTypeMapping);
     }
 
     /// <summary>
@@ -539,8 +539,7 @@ private void AddDiscriminator(SelectExpression selectExpression, IEntityType ent
 
             selectExpression.ApplyPredicate(
                 In(
-                    (SqlExpression)discriminatorColumn, Constant(concreteEntityTypes.Select(et => et.GetDiscriminatorValue()).ToList()),
-                    negated: false));
+                    (SqlExpression)discriminatorColumn, Constant(concreteEntityTypes.Select(et => et.GetDiscriminatorValue()).ToList())));
         }
     }
 }

src/EFCore.Relational/Query/ISqlExpressionFactory.cs

@@ -399,27 +399,24 @@ SqlFunctionExpression NiladicFunction(
     ///     Creates a new <see cref="ExistsExpression" /> which represents an EXISTS operation in a SQL tree.
     /// </summary>
     /// <param name="subquery">A subquery to check existence of.</param>
-    /// <param name="negated">A value indicating if the existence check is negated.</param>
     /// <returns>An expression representing an EXISTS operation in a SQL tree.</returns>
-    ExistsExpression Exists(SelectExpression subquery, bool negated);
+    ExistsExpression Exists(SelectExpression subquery);
 
     /// <summary>
     ///     Creates a new <see cref="InExpression" /> which represents an IN operation in a SQL tree.
     /// </summary>
     /// <param name="item">An item to look into values.</param>
     /// <param name="values">A list of values in which item is searched.</param>
-    /// <param name="negated">A value indicating if the item should be present in the values or absent.</param>
     /// <returns>An expression representing an IN operation in a SQL tree.</returns>
-    InExpression In(SqlExpression item, SqlExpression values, bool negated);
+    InExpression In(SqlExpression item, SqlExpression values);
 
     /// <summary>
     ///     Creates a new <see cref="InExpression" /> which represents an IN operation in a SQL tree.
     /// </summary>
     /// <param name="item">An item to look into values.</param>
     /// <param name="subquery">A subquery in which item is searched.</param>
-    /// <param name="negated">A value indicating if the item should be present in the values or absent.</param>
     /// <returns>An expression representing an IN operation in a SQL tree.</returns>
-    InExpression In(SqlExpression item, SelectExpression subquery, bool negated);
+    InExpression In(SqlExpression item, SelectExpression subquery);
 
     /// <summary>
     ///     Creates a new <see cref="InExpression" /> which represents a LIKE in a SQL tree.

src/EFCore.Relational/Query/Internal/ContainsTranslator.cs

@@ -42,15 +42,15 @@ public ContainsTranslator(ISqlExpressionFactory sqlExpressionFactory)
             && method.GetGenericMethodDefinition().Equals(EnumerableMethods.Contains)
             && ValidateValues(arguments[0]))
         {
-            return _sqlExpressionFactory.In(RemoveObjectConvert(arguments[1]), arguments[0], negated: false);
+            return _sqlExpressionFactory.In(RemoveObjectConvert(arguments[1]), arguments[0]);
         }
 
         if (arguments.Count == 1
             && method.IsContainsMethod()
             && instance != null
             && ValidateValues(instance))
         {
-            return _sqlExpressionFactory.In(RemoveObjectConvert(arguments[0]), instance, negated: false);
+            return _sqlExpressionFactory.In(RemoveObjectConvert(arguments[0]), instance);
         }
 
         return null;

src/EFCore.Relational/Query/Internal/SqlExpressionSimplifyingExpressionVisitor.cs

@@ -307,10 +307,16 @@ or ExpressionType.LessThan
                             break;
                     }
 
-                    return _sqlExpressionFactory.In(
+                    var inExpression = _sqlExpressionFactory.In(
                         leftCandidateInfo.ColumnExpression,
-                        _sqlExpressionFactory.Constant(resultArray, leftCandidateInfo.TypeMapping),
-                        leftCandidateInfo.OperationType == ExpressionType.NotEqual);
+                        _sqlExpressionFactory.Constant(resultArray, leftCandidateInfo.TypeMapping));
+
+                    return leftCandidateInfo.OperationType switch
+                    {
+                        ExpressionType.Equal => inExpression,
+                        ExpressionType.NotEqual => _sqlExpressionFactory.Not(inExpression),
+                        _ => throw new InvalidOperationException("IMPOSSIBLE")
+                    };
                 }
 
                 if (leftConstantIsEnumerable && rightConstantIsEnumerable)
@@ -321,10 +327,16 @@ or ExpressionType.LessThan
                         (IEnumerable)leftCandidateInfo.ConstantValue,
                         (IEnumerable)rightCandidateInfo.ConstantValue);
 
-                    return _sqlExpressionFactory.In(
+                    var inExpression = _sqlExpressionFactory.In(
                         leftCandidateInfo.ColumnExpression,
-                        _sqlExpressionFactory.Constant(resultArray, leftCandidateInfo.TypeMapping),
-                        leftCandidateInfo.OperationType == ExpressionType.NotEqual);
+                        _sqlExpressionFactory.Constant(resultArray, leftCandidateInfo.TypeMapping));
+
+                    return leftCandidateInfo.OperationType switch
+                    {
+                        ExpressionType.Equal => inExpression,
+                        ExpressionType.NotEqual => _sqlExpressionFactory.Not(inExpression),
+                        _ => throw new InvalidOperationException("IMPOSSIBLE")
+                    };
                 }
             }
         }
@@ -424,10 +436,9 @@ private static bool TryGetInExpressionCandidateInfo(
         else if (sqlExpression is InExpression
                  {
                      Item: ColumnExpression column, Subquery: null, Values: SqlConstantExpression valuesConstant
-                 } inExpression)
+                 })
         {
-            candidateInfo = (column, valuesConstant.Value!, valuesConstant.TypeMapping!,
-                inExpression.IsNegated ? ExpressionType.NotEqual : ExpressionType.Equal);
+            candidateInfo = (column, valuesConstant.Value!, valuesConstant.TypeMapping!, ExpressionType.Equal);
 
             return true;
         }