Skip to content

Commit

Permalink
Additional perf improvements around null semantics scenarios
Browse files Browse the repository at this point in the history
Resolves #17543 - Queries really slow due to null checks
Resolves #18525 - Query: optimize binary expression AndAlso and OrElse where left and right are the same
Resolves #18547 - DbFunctions compared to NULL are ignored and break the query builder

#17543

Before when rewriting null comparisons we would always remove possibility of nulls in the resulting expression. This is not always needed, specifically in predicates, where it doesn't really matter if expression returns false or null - the result is the same.
Now we detect those cases and apply "simplified" null expansion which should lead to better performance.

#18525

Null semantics initially expands expressions to a verbose form which then gets simplified in query optimizer. One of the optimizations added is when we do AND/OR where both sides are the same. Other small improvements have been added in this change also.

#18525

Previously we assumed that function can only be null if at least one of its arguments is null. This is the case for most built-in functions, but not all. This also goes for user defined functions which can return arbitrary results.
Fix is to treat all functions as potentially nullable. This leads to worse queries is some cases, but is also mitigated by other improvements added along side.
In the future we will provide metadata to better determine given function's nullability.

Also fixed a few small bug found along the way - incorrect Equals comparison for SqlExpression, for cases when TypeMapping was null.
  • Loading branch information
maumar committed Nov 1, 2019
1 parent 0a7c02e commit 5feb423
Show file tree
Hide file tree
Showing 39 changed files with 1,761 additions and 1,384 deletions.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -80,46 +80,49 @@ public ParameterNullabilityBasedSqlExpressionOptimizingExpressionVisitor(

protected override Expression VisitExtension(Expression extensionExpression)
{
if (extensionExpression is SelectExpression selectExpression)
// workaround for issue #18492
var newExpression = base.VisitExtension(extensionExpression);
if (newExpression is SelectExpression newSelectExpression)
{
var newSelectExpression = (SelectExpression)base.VisitExtension(extensionExpression);

// if predicate is optimized to true, we can simply remove it
var newPredicate = newSelectExpression.Predicate is SqlConstantExpression newSelectPredicateConstant
&& !(selectExpression.Predicate is SqlConstantExpression)
? (bool)newSelectPredicateConstant.Value
? null
: SqlExpressionFactory.Equal(
newSelectPredicateConstant,
SqlExpressionFactory.Constant(true, newSelectPredicateConstant.TypeMapping))
: newSelectExpression.Predicate;

var newHaving = newSelectExpression.Having is SqlConstantExpression newSelectHavingConstant
&& !(selectExpression.Having is SqlConstantExpression)
? (bool)newSelectHavingConstant.Value
? null
: SqlExpressionFactory.Equal(
newSelectHavingConstant,
SqlExpressionFactory.Constant(true, newSelectHavingConstant.TypeMapping))
: newSelectExpression.Having;

return !ReferenceEquals(newPredicate, newSelectExpression.Predicate)
|| !ReferenceEquals(newHaving, newSelectExpression.Having)
? newSelectExpression.Update(
newSelectExpression.Projection.ToList(),
newSelectExpression.Tables.ToList(),
newPredicate,
newSelectExpression.GroupBy.ToList(),
newHaving,
newSelectExpression.Orderings.ToList(),
newSelectExpression.Limit,
newSelectExpression.Offset,
newSelectExpression.IsDistinct,
newSelectExpression.Alias)
: newSelectExpression;
var changed = false;
var newPredicate = newSelectExpression.Predicate;
var newHaving = newSelectExpression.Having;
if (newSelectExpression.Predicate is SqlConstantExpression predicateConstantExpression
&& predicateConstantExpression.Value is bool predicateBoolValue
&& !predicateBoolValue)
{
changed = true;
newPredicate = SqlExpressionFactory.Equal(
predicateConstantExpression,
SqlExpressionFactory.Constant(true, predicateConstantExpression.TypeMapping));
}

if (newSelectExpression.Having is SqlConstantExpression havingConstantExpression
&& havingConstantExpression.Value is bool havingBoolValue
&& !havingBoolValue)
{
changed = true;
newHaving = SqlExpressionFactory.Equal(
havingConstantExpression,
SqlExpressionFactory.Constant(true, havingConstantExpression.TypeMapping));
}

return changed
? newSelectExpression.Update(
newSelectExpression.Projection.ToList(),
newSelectExpression.Tables.ToList(),
newPredicate,
newSelectExpression.GroupBy.ToList(),
newHaving,
newSelectExpression.Orderings.ToList(),
newSelectExpression.Limit,
newSelectExpression.Offset,
newSelectExpression.IsDistinct,
newSelectExpression.Alias)
: newSelectExpression;
}

return base.VisitExtension(extensionExpression);
return newExpression;
}

protected override Expression VisitSqlUnaryExpression(SqlUnaryExpression sqlUnaryExpression)
Expand All @@ -142,6 +145,33 @@ protected override Expression VisitSqlUnaryExpression(SqlUnaryExpression sqlUnar

return result;
}

protected override Expression VisitSqlBinaryExpression(SqlBinaryExpression sqlBinaryExpression)
{
var result = base.VisitSqlBinaryExpression(sqlBinaryExpression);
if (result is SqlBinaryExpression sqlBinaryResult)
{
var leftNullParameter = sqlBinaryResult.Left is SqlParameterExpression leftParameter
&& _parametersValues[leftParameter.Name] == null;

var rightNullParameter = sqlBinaryResult.Right is SqlParameterExpression rightParameter
&& _parametersValues[rightParameter.Name] == null;

if ((sqlBinaryResult.OperatorType == ExpressionType.Equal || sqlBinaryResult.OperatorType == ExpressionType.NotEqual)
&& (leftNullParameter || rightNullParameter))
{
return SimplifyNullComparisonExpression(
sqlBinaryResult.OperatorType,
sqlBinaryResult.Left,
sqlBinaryResult.Right,
leftNullParameter,
rightNullParameter,
sqlBinaryResult.TypeMapping);
}
}

return result;
}
}

private class InExpressionValuesExpandingExpressionVisitor : ExpressionVisitor
Expand Down
Loading

0 comments on commit 5feb423

Please sign in to comment.