Skip to content

Commit

Permalink
Additional refactoring of Null Semantics:
Browse files Browse the repository at this point in the history 
- moving NullSemantics visitor after 2nd level cache - we need to know the parameter values to properly handle IN expressions wrt null semantics,
- NullSemantics visitor needs to go before SqlExpressionOptimizer and SearchCondition, so those two are also moved after 2nd level cache,
- moving optimizations that depend on knowing the nullability to NullSemantics visitor - optimizer now only contains optimizations that also work in 3-value logic, or when we know nulls can't happen,
- merging InExpressionValuesExpandingExpressionVisitor int NullSemantics visitor, so that we don't apply the rewrite for UseRelationalNulls.

Resolves #11464
Resolves #15722
Resolves #18597
Resolves #18689
  • Loading branch information
@maumar
maumar committed Dec 5, 2019
1 parent fb0a1d8 commit 42a8e1b
Show file tree
Hide file tree
Showing 18 changed files with 834 additions and 635 deletions.
799 changes: 630 additions & 169 deletions src/EFCore.Relational/Query/Internal/NullSemanticsRewritingExpressionVisitor.cs
View file

Large diffs are not rendered by default.

236 changes: 6 additions & 230 deletions src/EFCore.Relational/Query/Internal/SqlExpressionOptimizingExpressionVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,11 @@ private SqlExpression SimplifyUnaryExpression(

break;

case SqlBinaryExpression binaryOperand:
// these optimizations are only valid in 2-value logic
// NullSemantics removes all nulls from expressions wrapped around Not
// so the optimizations are safe to do as long as UseRelationalNulls = false
case SqlBinaryExpression binaryOperand
when !_useRelationalNulls:
{
// De Morgan's
if (binaryOperand.OperatorType == ExpressionType.AndAlso
Expand All @@ -166,11 +170,7 @@ private SqlExpression SimplifyUnaryExpression(
binaryOperand.TypeMapping);
}

// those optimizations are only valid in 2-value logic
// they are safe to do here because if we apply null semantics
// because null semantics removes possibility of nulls in the tree when the comparison is wrapped around NOT
if (!_useRelationalNulls
&& TryNegate(binaryOperand.OperatorType, out var negated))
if (TryNegate(binaryOperand.OperatorType, out var negated))
{
return SimplifyBinaryExpression(
negated,
Expand All @@ -183,98 +183,6 @@ private SqlExpression SimplifyUnaryExpression(
}
break;
}

case ExpressionType.Equal:
case ExpressionType.NotEqual:
return SimplifyNullNotNullExpression(
operatorType,
operand,
type,
typeMapping);
}

return SqlExpressionFactory.MakeUnary(operatorType, operand, type, typeMapping);
}

private SqlExpression SimplifyNullNotNullExpression(
ExpressionType operatorType,
SqlExpression operand,
Type type,
RelationalTypeMapping typeMapping)
{
switch (operatorType)
{
case ExpressionType.Equal:
case ExpressionType.NotEqual:
switch (operand)
{
case SqlConstantExpression constantOperand:
return SqlExpressionFactory.Constant(
operatorType == ExpressionType.Equal
? constantOperand.Value == null
: constantOperand.Value != null,
typeMapping);

case ColumnExpression columnOperand
when !columnOperand.IsNullable:
return SqlExpressionFactory.Constant(operatorType == ExpressionType.NotEqual, typeMapping);

case SqlUnaryExpression sqlUnaryOperand:
if (sqlUnaryOperand.OperatorType == ExpressionType.Convert
|| sqlUnaryOperand.OperatorType == ExpressionType.Not
|| sqlUnaryOperand.OperatorType == ExpressionType.Negate)
{
// op(a) is null -> a is null
// op(a) is not null -> a is not null
return SimplifyNullNotNullExpression(operatorType, sqlUnaryOperand.Operand, type, typeMapping);
}

if (sqlUnaryOperand.OperatorType == ExpressionType.Equal
|| sqlUnaryOperand.OperatorType == ExpressionType.NotEqual)
{
// (a is null) is null -> false
// (a is not null) is null -> false
// (a is null) is not null -> true
// (a is not null) is not null -> true
return SqlExpressionFactory.Constant(operatorType == ExpressionType.NotEqual, typeMapping);
}
break;

case SqlBinaryExpression sqlBinaryOperand:
// in general:
// binaryOp(a, b) == null -> a == null || b == null
// binaryOp(a, b) != null -> a != null && b != null
// for coalesce:
// (a ?? b) == null -> a == null && b == null
// (a ?? b) != null -> a != null || b != null
// for AndAlso, OrElse we can't do this optimization
// we could do something like this, but it seems too complicated:
// (a && b) == null -> a == null && b != 0 || a != 0 && b == null
if (sqlBinaryOperand.OperatorType != ExpressionType.AndAlso
&& sqlBinaryOperand.OperatorType != ExpressionType.OrElse)
{
var newLeft = SimplifyNullNotNullExpression(operatorType, sqlBinaryOperand.Left, typeof(bool), typeMapping);
var newRight = SimplifyNullNotNullExpression(operatorType, sqlBinaryOperand.Right, typeof(bool), typeMapping);

return sqlBinaryOperand.OperatorType == ExpressionType.Coalesce
? SimplifyLogicalSqlBinaryExpression(
operatorType == ExpressionType.Equal
? ExpressionType.AndAlso
: ExpressionType.OrElse,
newLeft,
newRight,
typeMapping)
: SimplifyLogicalSqlBinaryExpression(
operatorType == ExpressionType.Equal
? ExpressionType.OrElse
: ExpressionType.AndAlso,
newLeft,
newRight,
typeMapping);
}
break;
}
break;
}

return SqlExpressionFactory.MakeUnary(operatorType, operand, type, typeMapping);
Expand Down Expand Up @@ -326,143 +234,11 @@ private SqlExpression SimplifyBinaryExpression(
left,
right,
typeMapping);

case ExpressionType.Equal:
case ExpressionType.NotEqual:
var leftConstant = left as SqlConstantExpression;
var rightConstant = right as SqlConstantExpression;
var leftNullConstant = leftConstant != null && leftConstant.Value == null;
var rightNullConstant = rightConstant != null && rightConstant.Value == null;
if (leftNullConstant || rightNullConstant)
{
return SimplifyNullComparisonExpression(
operatorType,
left,
right,
leftNullConstant,
rightNullConstant,
typeMapping);
}

var leftBoolConstant = left.Type == typeof(bool) ? leftConstant : null;
var rightBoolConstant = right.Type == typeof(bool) ? rightConstant : null;
if (leftBoolConstant != null || rightBoolConstant != null)
{
return SimplifyBoolConstantComparisonExpression(
operatorType,
left,
right,
leftBoolConstant,
rightBoolConstant,
typeMapping);
}

// only works when a is not nullable
// a == a -> true
// a != a -> false
if ((left is LikeExpression
|| left is ColumnExpression columnExpression && !columnExpression.IsNullable)
&& left.Equals(right))
{
return SqlExpressionFactory.Constant(operatorType == ExpressionType.Equal, typeMapping);
}

break;
}

return SqlExpressionFactory.MakeBinary(operatorType, left, right, typeMapping);
}

protected virtual SqlExpression SimplifyNullComparisonExpression(
ExpressionType operatorType,
SqlExpression left,
SqlExpression right,
bool leftNull,
bool rightNull,
RelationalTypeMapping typeMapping)
{
if ((operatorType == ExpressionType.Equal || operatorType == ExpressionType.NotEqual)
&& (leftNull || rightNull))
{
if (leftNull && rightNull)
{
return SqlExpressionFactory.Constant(operatorType == ExpressionType.Equal, typeMapping);
}

if (leftNull)
{
return SimplifyNullNotNullExpression(operatorType, right, typeof(bool), typeMapping);
}

if (rightNull)
{
return SimplifyNullNotNullExpression(operatorType, left, typeof(bool), typeMapping);
}
}

return SqlExpressionFactory.MakeBinary(operatorType, left, right, typeMapping);
}

private SqlExpression SimplifyBoolConstantComparisonExpression(
ExpressionType operatorType,
SqlExpression left,
SqlExpression right,
SqlConstantExpression leftBoolConstant,
SqlConstantExpression rightBoolConstant,
RelationalTypeMapping typeMapping)
{
if (leftBoolConstant != null && rightBoolConstant != null)
{
return operatorType == ExpressionType.Equal
? SqlExpressionFactory.Constant((bool)leftBoolConstant.Value == (bool)rightBoolConstant.Value, typeMapping)
: SqlExpressionFactory.Constant((bool)leftBoolConstant.Value != (bool)rightBoolConstant.Value, typeMapping);
}

if (rightBoolConstant != null
&& CanOptimize(left))
{
// a == true -> a
// a == false -> !a
// a != true -> !a
// a != false -> a
// only correct when f(x) can't be null
return operatorType == ExpressionType.Equal
? (bool)rightBoolConstant.Value
? left
: SimplifyUnaryExpression(ExpressionType.Not, left, typeof(bool), typeMapping)
: (bool)rightBoolConstant.Value
? SimplifyUnaryExpression(ExpressionType.Not, left, typeof(bool), typeMapping)
: left;
}

if (leftBoolConstant != null
&& CanOptimize(right))
{
// true == a -> a
// false == a -> !a
// true != a -> !a
// false != a -> a
// only correct when a can't be null
return operatorType == ExpressionType.Equal
? (bool)leftBoolConstant.Value
? right
: SimplifyUnaryExpression(ExpressionType.Not, right, typeof(bool), typeMapping)
: (bool)leftBoolConstant.Value
? SimplifyUnaryExpression(ExpressionType.Not, right, typeof(bool), typeMapping)
: right;
}

return SqlExpressionFactory.MakeBinary(operatorType, left, right, typeMapping);

static bool CanOptimize(SqlExpression operand)
=> operand is LikeExpression
|| (operand is SqlUnaryExpression sqlUnary
&& (sqlUnary.OperatorType == ExpressionType.Equal
|| sqlUnary.OperatorType == ExpressionType.NotEqual
// TODO: #18689
/*|| sqlUnary.OperatorType == ExpressionType.Not*/));
}

private SqlExpression SimplifyLogicalSqlBinaryExpression(
ExpressionType operatorType,
SqlExpression left,
Expand Down
Loading

0 comments on commit 42a8e1b

Please sign in to comment.