Skip to content

Commit 8fdba79

Browse files
rojiroji
committed
Better management of negated expressions
Closes #31027
1 parent 4334ae0 commit 8fdba79

36 files changed

+253
-313
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,15 +40,15 @@ public CosmosContainsTranslator(ISqlExpressionFactory sqlExpressionFactory)
4040
&& method.GetGenericMethodDefinition().Equals(EnumerableMethods.Contains)
4141
&& ValidateValues(arguments[0]))
4242
{
43-
return _sqlExpressionFactory.In(arguments[1], arguments[0], false);
43+
return _sqlExpressionFactory.In(arguments[1], arguments[0]);
4444
}
4545

4646
if (arguments.Count == 1
4747
&& method.IsContainsMethod()
4848
&& instance != null
4949
&& ValidateValues(instance))
5050
{
51-
return _sqlExpressionFactory.In(arguments[0], instance, false);
51+
return _sqlExpressionFactory.In(arguments[0], instance);
5252
}
5353

5454
return null;

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,7 @@ public override Expression Visit(Expression expression)
7070
var updatedInExpression = inValues.Count > 0
7171
? _sqlExpressionFactory.In(
7272
(SqlExpression)Visit(inExpression.Item),
73-
_sqlExpressionFactory.Constant(inValues, typeMapping),
74-
inExpression.IsNegated)
73+
_sqlExpressionFactory.Constant(inValues, typeMapping))
7574
: null;
7675

7776
var nullCheckExpression = hasNullValue

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -704,8 +704,7 @@ protected override Expression VisitTypeBinary(TypeBinaryExpression typeBinaryExp
704704
_sqlExpressionFactory.Constant(concreteEntityTypes[0].GetDiscriminatorValue()))
705705
: _sqlExpressionFactory.In(
706706
discriminatorColumn,
707-
_sqlExpressionFactory.Constant(concreteEntityTypes.Select(et => et.GetDiscriminatorValue()).ToList()),
708-
negated: false);
707+
_sqlExpressionFactory.Constant(concreteEntityTypes.Select(et => et.GetDiscriminatorValue()).ToList()));
709708
}
710709
}
711710

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ SqlConditionalExpression Condition(
256256
/// any release. You should only use it directly in your code with extreme caution and knowing that
257257
/// doing so can result in application failures when updating to a new Entity Framework Core release.
258258
/// </summary>
259-
InExpression In(SqlExpression item, SqlExpression values, bool negated);
259+
InExpression In(SqlExpression item, SqlExpression values);
260260

261261
/// <summary>
262262
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to

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

Lines changed: 3 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,11 @@ public class InExpression : SqlExpression
1919
/// </summary>
2020
public InExpression(
2121
SqlExpression item,
22-
bool negated,
2322
SqlExpression values,
2423
CoreTypeMapping typeMapping)
2524
: base(typeof(bool), typeMapping)
2625
{
2726
Item = item;
28-
IsNegated = negated;
2927
Values = values;
3028
}
3129

@@ -37,14 +35,6 @@ public InExpression(
3735
/// </summary>
3836
public virtual SqlExpression Item { get; }
3937

40-
/// <summary>
41-
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
42-
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
43-
/// any release. You should only use it directly in your code with extreme caution and knowing that
44-
/// doing so can result in application failures when updating to a new Entity Framework Core release.
45-
/// </summary>
46-
public virtual bool IsNegated { get; }
47-
4838
/// <summary>
4939
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
5040
/// 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)
6757
return Update(newItem, values);
6858
}
6959

70-
/// <summary>
71-
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
72-
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
73-
/// any release. You should only use it directly in your code with extreme caution and knowing that
74-
/// doing so can result in application failures when updating to a new Entity Framework Core release.
75-
/// </summary>
76-
public virtual InExpression Negate()
77-
=> new(Item, !IsNegated, Values, TypeMapping!);
78-
7960
/// <summary>
8061
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
8162
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
@@ -84,7 +65,7 @@ public virtual InExpression Negate()
8465
/// </summary>
8566
public virtual InExpression Update(SqlExpression item, SqlExpression values)
8667
=> item != Item || values != Values
87-
? new InExpression(item, IsNegated, values, TypeMapping!)
68+
? new InExpression(item, values, TypeMapping!)
8869
: this;
8970

9071
/// <summary>
@@ -96,7 +77,7 @@ public virtual InExpression Update(SqlExpression item, SqlExpression values)
9677
protected override void Print(ExpressionPrinter expressionPrinter)
9778
{
9879
expressionPrinter.Visit(Item);
99-
expressionPrinter.Append(IsNegated ? " NOT IN " : " IN ");
80+
expressionPrinter.Append(" IN ");
10081
expressionPrinter.Append("(");
10182
expressionPrinter.Visit(Values);
10283
expressionPrinter.Append(")");
@@ -117,7 +98,6 @@ public override bool Equals(object? obj)
11798
private bool Equals(InExpression inExpression)
11899
=> base.Equals(inExpression)
119100
&& Item.Equals(inExpression.Item)
120-
&& IsNegated.Equals(inExpression.IsNegated)
121101
&& Values.Equals(inExpression.Values);
122102

123103
/// <summary>
@@ -127,5 +107,5 @@ private bool Equals(InExpression inExpression)
127107
/// doing so can result in application failures when updating to a new Entity Framework Core release.
128108
/// </summary>
129109
public override int GetHashCode()
130-
=> HashCode.Combine(base.GetHashCode(), Item, IsNegated, Values);
110+
=> HashCode.Combine(base.GetHashCode(), Item, Values);
131111
}

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

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -387,6 +387,13 @@ protected override Expression VisitSqlUnary(SqlUnaryExpression sqlUnaryExpressio
387387
if (sqlUnaryExpression.OperatorType == ExpressionType.Not
388388
&& sqlUnaryExpression.Operand.Type == typeof(bool))
389389
{
390+
if (sqlUnaryExpression.Operand is InExpression inExpression)
391+
{
392+
GenerateIn(inExpression, negated: true);
393+
394+
return sqlUnaryExpression;
395+
}
396+
390397
op = "NOT";
391398
}
392399

@@ -506,18 +513,29 @@ protected override Expression VisitSqlParameter(SqlParameterExpression sqlParame
506513
/// any release. You should only use it directly in your code with extreme caution and knowing that
507514
/// doing so can result in application failures when updating to a new Entity Framework Core release.
508515
/// </summary>
509-
protected override Expression VisitIn(InExpression inExpression)
516+
protected sealed override Expression VisitIn(InExpression inExpression)
517+
{
518+
GenerateIn(inExpression, negated: false);
519+
520+
return inExpression;
521+
}
522+
523+
/// <summary>
524+
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
525+
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
526+
/// any release. You should only use it directly in your code with extreme caution and knowing that
527+
/// doing so can result in application failures when updating to a new Entity Framework Core release.
528+
/// </summary>
529+
protected virtual void GenerateIn(InExpression inExpression, bool negated)
510530
{
511531
Visit(inExpression.Item);
512-
_sqlBuilder.Append(inExpression.IsNegated ? " NOT IN " : " IN ");
532+
_sqlBuilder.Append(negated ? " NOT IN " : " IN ");
513533
_sqlBuilder.Append('(');
514534
var valuesConstant = (SqlConstantExpression)inExpression.Values;
515535
var valuesList = ((IEnumerable<object>)valuesConstant.Value)
516536
.Select(v => new SqlConstantExpression(Expression.Constant(v), valuesConstant.TypeMapping)).ToList();
517537
GenerateList(valuesList, e => Visit(e));
518538
_sqlBuilder.Append(')');
519-
520-
return inExpression;
521539
}
522540

523541
/// <summary>

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -473,14 +473,14 @@ public virtual SqlConditionalExpression Condition(SqlExpression test, SqlExpress
473473
/// any release. You should only use it directly in your code with extreme caution and knowing that
474474
/// doing so can result in application failures when updating to a new Entity Framework Core release.
475475
/// </summary>
476-
public virtual InExpression In(SqlExpression item, SqlExpression values, bool negated)
476+
public virtual InExpression In(SqlExpression item, SqlExpression values)
477477
{
478478
var typeMapping = item.TypeMapping ?? _typeMappingSource.FindMapping(item.Type, _model);
479479

480480
item = ApplyTypeMapping(item, typeMapping);
481481
values = ApplyTypeMapping(values, typeMapping);
482482

483-
return new InExpression(item, negated, values, _boolTypeMapping);
483+
return new InExpression(item, values, _boolTypeMapping);
484484
}
485485

486486
/// <summary>
@@ -539,8 +539,7 @@ private void AddDiscriminator(SelectExpression selectExpression, IEntityType ent
539539

540540
selectExpression.ApplyPredicate(
541541
In(
542-
(SqlExpression)discriminatorColumn, Constant(concreteEntityTypes.Select(et => et.GetDiscriminatorValue()).ToList()),
543-
negated: false));
542+
(SqlExpression)discriminatorColumn, Constant(concreteEntityTypes.Select(et => et.GetDiscriminatorValue()).ToList())));
544543
}
545544
}
546545
}

‎src/EFCore.Relational/Query/ISqlExpressionFactory.cs‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -399,27 +399,24 @@ SqlFunctionExpression NiladicFunction(
399399
/// Creates a new <see cref="ExistsExpression" /> which represents an EXISTS operation in a SQL tree.
400400
/// </summary>
401401
/// <param name="subquery">A subquery to check existence of.</param>
402-
/// <param name="negated">A value indicating if the existence check is negated.</param>
403402
/// <returns>An expression representing an EXISTS operation in a SQL tree.</returns>
404-
ExistsExpression Exists(SelectExpression subquery, bool negated);
403+
ExistsExpression Exists(SelectExpression subquery);
405404

406405
/// <summary>
407406
/// Creates a new <see cref="InExpression" /> which represents an IN operation in a SQL tree.
408407
/// </summary>
409408
/// <param name="item">An item to look into values.</param>
410409
/// <param name="values">A list of values in which item is searched.</param>
411-
/// <param name="negated">A value indicating if the item should be present in the values or absent.</param>
412410
/// <returns>An expression representing an IN operation in a SQL tree.</returns>
413-
InExpression In(SqlExpression item, SqlExpression values, bool negated);
411+
InExpression In(SqlExpression item, SqlExpression values);
414412

415413
/// <summary>
416414
/// Creates a new <see cref="InExpression" /> which represents an IN operation in a SQL tree.
417415
/// </summary>
418416
/// <param name="item">An item to look into values.</param>
419417
/// <param name="subquery">A subquery in which item is searched.</param>
420-
/// <param name="negated">A value indicating if the item should be present in the values or absent.</param>
421418
/// <returns>An expression representing an IN operation in a SQL tree.</returns>
422-
InExpression In(SqlExpression item, SelectExpression subquery, bool negated);
419+
InExpression In(SqlExpression item, SelectExpression subquery);
423420

424421
/// <summary>
425422
/// Creates a new <see cref="InExpression" /> which represents a LIKE in a SQL tree.

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,15 +42,15 @@ public ContainsTranslator(ISqlExpressionFactory sqlExpressionFactory)
4242
&& method.GetGenericMethodDefinition().Equals(EnumerableMethods.Contains)
4343
&& ValidateValues(arguments[0]))
4444
{
45-
return _sqlExpressionFactory.In(RemoveObjectConvert(arguments[1]), arguments[0], negated: false);
45+
return _sqlExpressionFactory.In(RemoveObjectConvert(arguments[1]), arguments[0]);
4646
}
4747

4848
if (arguments.Count == 1
4949
&& method.IsContainsMethod()
5050
&& instance != null
5151
&& ValidateValues(instance))
5252
{
53-
return _sqlExpressionFactory.In(RemoveObjectConvert(arguments[0]), instance, negated: false);
53+
return _sqlExpressionFactory.In(RemoveObjectConvert(arguments[0]), instance);
5454
}
5555

5656
return null;

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

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -307,10 +307,16 @@ or ExpressionType.LessThan
307307
break;
308308
}
309309

310-
return _sqlExpressionFactory.In(
310+
var inExpression = _sqlExpressionFactory.In(
311311
leftCandidateInfo.ColumnExpression,
312-
_sqlExpressionFactory.Constant(resultArray, leftCandidateInfo.TypeMapping),
313-
leftCandidateInfo.OperationType == ExpressionType.NotEqual);
312+
_sqlExpressionFactory.Constant(resultArray, leftCandidateInfo.TypeMapping));
313+
314+
return leftCandidateInfo.OperationType switch
315+
{
316+
ExpressionType.Equal => inExpression,
317+
ExpressionType.NotEqual => _sqlExpressionFactory.Not(inExpression),
318+
_ => throw new InvalidOperationException("IMPOSSIBLE")
319+
};
314320
}
315321

316322
if (leftConstantIsEnumerable && rightConstantIsEnumerable)
@@ -321,10 +327,16 @@ or ExpressionType.LessThan
321327
(IEnumerable)leftCandidateInfo.ConstantValue,
322328
(IEnumerable)rightCandidateInfo.ConstantValue);
323329

324-
return _sqlExpressionFactory.In(
330+
var inExpression = _sqlExpressionFactory.In(
325331
leftCandidateInfo.ColumnExpression,
326-
_sqlExpressionFactory.Constant(resultArray, leftCandidateInfo.TypeMapping),
327-
leftCandidateInfo.OperationType == ExpressionType.NotEqual);
332+
_sqlExpressionFactory.Constant(resultArray, leftCandidateInfo.TypeMapping));
333+
334+
return leftCandidateInfo.OperationType switch
335+
{
336+
ExpressionType.Equal => inExpression,
337+
ExpressionType.NotEqual => _sqlExpressionFactory.Not(inExpression),
338+
_ => throw new InvalidOperationException("IMPOSSIBLE")
339+
};
328340
}
329341
}
330342
}
@@ -424,10 +436,9 @@ private static bool TryGetInExpressionCandidateInfo(
424436
else if (sqlExpression is InExpression
425437
{
426438
Item: ColumnExpression column, Subquery: null, Values: SqlConstantExpression valuesConstant
427-
} inExpression)
439+
})
428440
{
429-
candidateInfo = (column, valuesConstant.Value!, valuesConstant.TypeMapping!,
430-
inExpression.IsNegated ? ExpressionType.NotEqual : ExpressionType.Equal);
441+
candidateInfo = (column, valuesConstant.Value!, valuesConstant.TypeMapping!, ExpressionType.Equal);
431442

432443
return true;
433444
}

0 commit comments

Comments
 (0)