Skip to content

Commit

Permalink
Simplify AND and OR
Browse files Browse the repository at this point in the history
When constructing `AndAlso` and `OrElse` expressions, perform some basic local
simplifications.
  • Loading branch information
ranma42 committed Jul 2, 2024
1 parent 051c33a commit c8c940a
Show file tree
Hide file tree
Showing 2 changed files with 90 additions and 4 deletions.
4 changes: 3 additions & 1 deletion src/EFCore.Relational/Query/ISqlExpressionFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,14 @@ public interface ISqlExpressionFactory
/// <param name="left">The left operand of binary operation.</param>
/// <param name="right">The right operand of binary operation.</param>
/// <param name="typeMapping">A type mapping to be assigned to the created expression.</param>
/// <param name="existingExpr">An optional expression that can be re-used if it matches the new expression.</param>
/// <returns>A <see cref="SqlExpression" /> with the given arguments.</returns>
SqlExpression? MakeBinary(
ExpressionType operatorType,
SqlExpression left,
SqlExpression right,
RelationalTypeMapping? typeMapping);
RelationalTypeMapping? typeMapping,
SqlExpression? existingExpr = null);

// Comparison
/// <summary>
Expand Down
90 changes: 87 additions & 3 deletions src/EFCore.Relational/Query/SqlExpressionFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -395,8 +395,17 @@ private SqlExpression ApplyTypeMappingOnJsonScalar(
ExpressionType operatorType,
SqlExpression left,
SqlExpression right,
RelationalTypeMapping? typeMapping)
RelationalTypeMapping? typeMapping,
SqlExpression? existingExpr = null)
{
switch (operatorType)
{
case ExpressionType.AndAlso:
return ApplyTypeMapping(AndAlso(left, right, existingExpr), typeMapping);
case ExpressionType.OrElse:
return ApplyTypeMapping(OrElse(left, right, existingExpr), typeMapping);
}

if (!SqlBinaryExpression.IsValidOperator(operatorType))
{
return null;
Expand All @@ -411,8 +420,6 @@ private SqlExpression ApplyTypeMappingOnJsonScalar(
case ExpressionType.LessThan:
case ExpressionType.LessThanOrEqual:
case ExpressionType.NotEqual:
case ExpressionType.AndAlso:
case ExpressionType.OrElse:
returnType = typeof(bool);
break;
}
Expand Down Expand Up @@ -449,10 +456,87 @@ public virtual SqlExpression LessThanOrEqual(SqlExpression left, SqlExpression r
public virtual SqlExpression AndAlso(SqlExpression left, SqlExpression right)
=> MakeBinary(ExpressionType.AndAlso, left, right, null)!;

private SqlExpression AndAlso(SqlExpression left, SqlExpression right, SqlExpression? existingExpr)
{
// false && x -> false
// x && true -> x
// x && x -> x
if (left is SqlConstantExpression { Value: false }
|| right is SqlConstantExpression { Value: true }
|| left.Equals(right))
{
return left;
}
// true && x -> x
// x && false -> false
else if (left is SqlConstantExpression { Value: true } || right is SqlConstantExpression { Value: false })
{
return right;
}
// x is null && x is not null -> false
// x is not null && x is null -> false
else if (left is SqlUnaryExpression { OperatorType: ExpressionType.Equal or ExpressionType.NotEqual } leftUnary
&& right is SqlUnaryExpression { OperatorType: ExpressionType.Equal or ExpressionType.NotEqual } rightUnary
&& leftUnary.Operand.Equals(rightUnary.Operand))
{
// the case in which left and right are the same expression is handled above
return Constant(false);
}
else if (existingExpr is SqlBinaryExpression { OperatorType: ExpressionType.AndAlso } binaryExpr
&& left == binaryExpr.Left
&& right == binaryExpr.Right)
{
return existingExpr;
}
else
{
return new SqlBinaryExpression(ExpressionType.AndAlso, left, right, typeof(bool), null);
}
}

/// <inheritdoc />
public virtual SqlExpression OrElse(SqlExpression left, SqlExpression right)
=> MakeBinary(ExpressionType.OrElse, left, right, null)!;

private SqlExpression OrElse(SqlExpression left, SqlExpression right, SqlExpression? existingExpr = null)
{
// true || x -> true
// x || false -> x
// x || x -> x
if (left is SqlConstantExpression { Value: true }
|| right is SqlConstantExpression { Value: false }
|| left.Equals(right))
{
return left;
}
// false || x -> x
// x || true -> true
else if (left is SqlConstantExpression { Value: false }
|| right is SqlConstantExpression { Value: true })
{
return right;
}
// x is null || x is not null -> true
// x is not null || x is null -> true
else if (left is SqlUnaryExpression { OperatorType: ExpressionType.Equal or ExpressionType.NotEqual } leftUnary
&& right is SqlUnaryExpression { OperatorType: ExpressionType.Equal or ExpressionType.NotEqual } rightUnary
&& leftUnary.Operand.Equals(rightUnary.Operand))
{
// the case in which left and right are the same expression is handled above
return Constant(true);
}
else if (existingExpr is SqlBinaryExpression { OperatorType: ExpressionType.OrElse } binaryExpr
&& left == binaryExpr.Left
&& right == binaryExpr.Right)
{
return existingExpr;
}
else
{
return new SqlBinaryExpression(ExpressionType.OrElse, left, right, typeof(bool), null);
}
}

/// <inheritdoc />
public virtual SqlExpression Add(SqlExpression left, SqlExpression right, RelationalTypeMapping? typeMapping = null)
=> MakeBinary(ExpressionType.Add, left, right, typeMapping)!;
Expand Down

0 comments on commit c8c940a

Please sign in to comment.