Skip to content

Commit

Permalink
Remove SimplifyLogicalSqlBinaryExpression
Browse files Browse the repository at this point in the history
Its simplifications are already performed by the `SqlExpressionFactory`.
  • Loading branch information
ranma42 committed Jul 2, 2024
1 parent c8c940a commit d3e7787
Showing 1 changed file with 36 additions and 103 deletions.
139 changes: 36 additions & 103 deletions src/EFCore.Relational/Query/SqlNullabilityProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1358,8 +1358,14 @@ or ExpressionType.LessThan
}

return result is SqlBinaryExpression sqlBinaryResult
&& sqlBinaryExpression.OperatorType is ExpressionType.AndAlso or ExpressionType.OrElse
? SimplifyLogicalSqlBinaryExpression(sqlBinaryResult)
&& sqlBinaryResult.OperatorType is ExpressionType.AndAlso or ExpressionType.OrElse
? _sqlExpressionFactory.MakeBinary( // invoke MakeBinary simplifications
sqlBinaryResult.OperatorType,
sqlBinaryResult.Left,
sqlBinaryResult.Right,
sqlBinaryResult.TypeMapping,
sqlBinaryResult
)!
: result;

SqlExpression AddNullConcatenationProtection(SqlExpression argument, RelationalTypeMapping typeMapping)
Expand Down Expand Up @@ -1796,26 +1802,16 @@ private SqlExpression RewriteNullSemantics(
{
nullable = leftNullable || rightNullable;

return SimplifyLogicalSqlBinaryExpression(
_sqlExpressionFactory.OrElse(
body,
SimplifyLogicalSqlBinaryExpression(
_sqlExpressionFactory.AndAlso(leftIsNull, rightIsNull))));
return _sqlExpressionFactory.OrElse(body, _sqlExpressionFactory.AndAlso(leftIsNull, rightIsNull));
}

// doing a full null semantics rewrite - removing all nulls from truth table
nullable = false;

// (a == b && (a != null && b != null)) || (a == null && b == null)
body = SimplifyLogicalSqlBinaryExpression(
_sqlExpressionFactory.OrElse(
SimplifyLogicalSqlBinaryExpression(
_sqlExpressionFactory.AndAlso(
body,
SimplifyLogicalSqlBinaryExpression(
_sqlExpressionFactory.AndAlso(leftIsNotNull, rightIsNotNull)))),
SimplifyLogicalSqlBinaryExpression(
_sqlExpressionFactory.AndAlso(leftIsNull, rightIsNull))));
body = _sqlExpressionFactory.OrElse(
_sqlExpressionFactory.AndAlso(body, _sqlExpressionFactory.AndAlso(leftIsNotNull, rightIsNotNull)),
_sqlExpressionFactory.AndAlso(leftIsNull, rightIsNull));

if (sqlBinaryExpression.OperatorType == ExpressionType.NotEqual)
{
Expand All @@ -1826,65 +1822,6 @@ private SqlExpression RewriteNullSemantics(
return body;
}

private SqlExpression SimplifyLogicalSqlBinaryExpression(SqlExpression expression)
{
if (expression is not SqlBinaryExpression sqlBinaryExpression)
{
return expression;
}

if (sqlBinaryExpression is
{
Left: SqlUnaryExpression { OperatorType: ExpressionType.Equal or ExpressionType.NotEqual } leftUnary,
Right: SqlUnaryExpression { OperatorType: ExpressionType.Equal or ExpressionType.NotEqual } rightUnary
}
&& leftUnary.Operand.Equals(rightUnary.Operand))
{
// a is null || a is null -> a is null
// a is not null || a is not null -> a is not null
// a is null && a is null -> a is null
// a is not null && a is not null -> a is not null
// a is null || a is not null -> true
// a is null && a is not null -> false
return leftUnary.OperatorType == rightUnary.OperatorType
? leftUnary
: _sqlExpressionFactory.Constant(
sqlBinaryExpression.OperatorType == ExpressionType.OrElse, sqlBinaryExpression.TypeMapping);
}

// true && a -> a
// true || a -> true
// false && a -> false
// false || a -> a
if (sqlBinaryExpression.Left is SqlConstantExpression { Value: bool leftBoolValue } newLeftConstant)
{
return sqlBinaryExpression.OperatorType == ExpressionType.AndAlso
? leftBoolValue
? sqlBinaryExpression.Right
: newLeftConstant
: leftBoolValue
? newLeftConstant
: sqlBinaryExpression.Right;
}

if (sqlBinaryExpression.Right is SqlConstantExpression { Value: bool rightBoolValue } newRightConstant)
{
// a && true -> a
// a || true -> true
// a && false -> false
// a || false -> a
return sqlBinaryExpression.OperatorType == ExpressionType.AndAlso
? rightBoolValue
? sqlBinaryExpression.Left
: newRightConstant
: rightBoolValue
? newRightConstant
: sqlBinaryExpression.Left;
}

return sqlBinaryExpression;
}

/// <summary>
/// Attempts to simplify a unary not operation on a non-nullable operand.
/// </summary>
Expand Down Expand Up @@ -1938,14 +1875,13 @@ protected virtual SqlExpression OptimizeNonNullableNotExpression(SqlExpression e
var left = OptimizeNonNullableNotExpression(_sqlExpressionFactory.Not(sqlBinaryOperand.Left));
var right = OptimizeNonNullableNotExpression(_sqlExpressionFactory.Not(sqlBinaryOperand.Right));

return SimplifyLogicalSqlBinaryExpression(
_sqlExpressionFactory.MakeBinary(
sqlBinaryOperand.OperatorType == ExpressionType.AndAlso
? ExpressionType.OrElse
: ExpressionType.AndAlso,
left,
right,
sqlBinaryOperand.TypeMapping)!);
return _sqlExpressionFactory.MakeBinary(
sqlBinaryOperand.OperatorType == ExpressionType.AndAlso
? ExpressionType.OrElse
: ExpressionType.AndAlso,
left,
right,
sqlBinaryOperand.TypeMapping)!;
}

// use equality where possible
Expand Down Expand Up @@ -2266,14 +2202,13 @@ private SqlExpression ProcessNullNotNull(SqlExpression sqlExpression, bool opera
sqlUnaryExpression.TypeMapping)!,
operandNullable);

return SimplifyLogicalSqlBinaryExpression(
_sqlExpressionFactory.MakeBinary(
sqlUnaryExpression.OperatorType == ExpressionType.Equal
? ExpressionType.OrElse
: ExpressionType.AndAlso,
left,
right,
sqlUnaryExpression.TypeMapping)!);
return _sqlExpressionFactory.MakeBinary(
sqlUnaryExpression.OperatorType == ExpressionType.Equal
? ExpressionType.OrElse
: ExpressionType.AndAlso,
left,
right,
sqlUnaryExpression.TypeMapping)!;
}

case SqlFunctionExpression sqlFunctionExpression:
Expand All @@ -2295,14 +2230,13 @@ private SqlExpression ProcessNullNotNull(SqlExpression sqlExpression, bool opera
sqlUnaryExpression.TypeMapping)!,
operandNullable))
.Aggregate(
(l, r) => SimplifyLogicalSqlBinaryExpression(
_sqlExpressionFactory.MakeBinary(
sqlUnaryExpression.OperatorType == ExpressionType.Equal
? ExpressionType.AndAlso
: ExpressionType.OrElse,
l,
r,
sqlUnaryExpression.TypeMapping)!));
(l, r) => _sqlExpressionFactory.MakeBinary(
sqlUnaryExpression.OperatorType == ExpressionType.Equal
? ExpressionType.AndAlso
: ExpressionType.OrElse,
l,
r,
sqlUnaryExpression.TypeMapping)!);
}

if (!sqlFunctionExpression.IsNullable)
Expand Down Expand Up @@ -2348,10 +2282,9 @@ private SqlExpression ProcessNullNotNull(SqlExpression sqlExpression, bool opera
sqlUnaryExpression.TypeMapping)!,
operandNullable))
.Aggregate(
(r, e) => SimplifyLogicalSqlBinaryExpression(
sqlUnaryExpression.OperatorType == ExpressionType.Equal
? _sqlExpressionFactory.OrElse(r, e)
: _sqlExpressionFactory.AndAlso(r, e)));
(r, e) => sqlUnaryExpression.OperatorType == ExpressionType.Equal
? _sqlExpressionFactory.OrElse(r, e)
: _sqlExpressionFactory.AndAlso(r, e));

return result;
}
Expand Down

0 comments on commit d3e7787

Please sign in to comment.