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 4b42c7a commit 64be016
Show file tree
Hide file tree
Showing 18 changed files with 852 additions and 660 deletions.
807 changes: 638 additions & 169 deletions src/EFCore.Relational/Query/Internal/NullSemanticsRewritingExpressionVisitor.cs
View file

Large diffs are not rendered by default.

244 changes: 13 additions & 231 deletions src/EFCore.Relational/Query/Internal/SqlExpressionOptimizingExpressionVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Linq.Expressions;
using Microsoft.EntityFrameworkCore.Query.SqlExpressions;
Expand All @@ -12,6 +13,7 @@ namespace Microsoft.EntityFrameworkCore.Query.Internal
public class SqlExpressionOptimizingExpressionVisitor : ExpressionVisitor
{
private readonly bool _useRelationalNulls;
private readonly IReadOnlyDictionary<string, object> _parametersValues;

private static bool TryNegate(ExpressionType expressionType, out ExpressionType result)
{
Expand All @@ -33,10 +35,14 @@ private static bool TryNegate(ExpressionType expressionType, out ExpressionType
return negated.HasValue;
}

public SqlExpressionOptimizingExpressionVisitor(ISqlExpressionFactory sqlExpressionFactory, bool useRelationalNulls)
public SqlExpressionOptimizingExpressionVisitor(
ISqlExpressionFactory sqlExpressionFactory,
bool useRelationalNulls,
IReadOnlyDictionary<string, object> parametersValues)
{
SqlExpressionFactory = sqlExpressionFactory;
_useRelationalNulls = useRelationalNulls;
_parametersValues = parametersValues;
}

protected virtual ISqlExpressionFactory SqlExpressionFactory { get; }
Expand Down Expand Up @@ -148,7 +154,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 +176,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 +189,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 +240,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 64be016

Please sign in to comment.