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,
- preventing NulSemantics from performing double visitation when computing non-nullable columns.

Resolves #11464
Resolves #15722
Resolves #18338
Resolves #18597
Resolves #18689
Resolves #19019
  • Loading branch information
maumar committed Dec 13, 2019
1 parent af8ff54 commit ff0f085
Show file tree
Hide file tree
Showing 23 changed files with 2,044 additions and 1,500 deletions.

This file was deleted.

1,474 changes: 1,474 additions & 0 deletions src/EFCore.Relational/Query/Internal/NullabilityHandlingExpressionVisitor.cs

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections;
using System.Collections.Generic;
using System.Data.Common;
using System.Linq.Expressions;
Expand Down Expand Up @@ -46,17 +45,19 @@ public virtual (SelectExpression selectExpression, bool canCache) Optimize(
Check.NotNull(parametersValues, nameof(parametersValues));

var canCache = true;
var nullabilityHandlingExpressionVisitor = new NullabilityHandlingExpressionVisitor(
UseRelationalNulls,
Dependencies.SqlExpressionFactory,
parametersValues);

var inExpressionOptimized = new InExpressionValuesExpandingExpressionVisitor(
Dependencies.SqlExpressionFactory, parametersValues).Visit(selectExpression);

if (!ReferenceEquals(selectExpression, inExpressionOptimized))
var nullabilityHandled = nullabilityHandlingExpressionVisitor.Visit(selectExpression);
if (!nullabilityHandlingExpressionVisitor.CanCache)
{
canCache = false;
}

var nullParametersOptimized = new ParameterNullabilityBasedSqlExpressionOptimizingExpressionVisitor(
Dependencies.SqlExpressionFactory, UseRelationalNulls, parametersValues).Visit(inExpressionOptimized);
var nullParametersOptimized = new SqlExpressionOptimizingExpressionVisitor(
Dependencies.SqlExpressionFactory, UseRelationalNulls, parametersValues).Visit(nullabilityHandled);

var fromSqlParameterOptimized = new FromSqlParameterApplyingExpressionVisitor(
Dependencies.SqlExpressionFactory,
Expand All @@ -71,163 +72,6 @@ public virtual (SelectExpression selectExpression, bool canCache) Optimize(
return (selectExpression: (SelectExpression)fromSqlParameterOptimized, canCache);
}

private sealed class ParameterNullabilityBasedSqlExpressionOptimizingExpressionVisitor : SqlExpressionOptimizingExpressionVisitor
{
private readonly IReadOnlyDictionary<string, object> _parametersValues;

public ParameterNullabilityBasedSqlExpressionOptimizingExpressionVisitor(
ISqlExpressionFactory sqlExpressionFactory,
bool useRelationalNulls,
IReadOnlyDictionary<string, object> parametersValues)
: base(sqlExpressionFactory, useRelationalNulls)
{
_parametersValues = parametersValues;
}

protected override Expression VisitSqlUnaryExpression(SqlUnaryExpression sqlUnaryExpression)
{
var result = base.VisitSqlUnaryExpression(sqlUnaryExpression);
if (result is SqlUnaryExpression newUnaryExpression
&& newUnaryExpression.Operand is SqlParameterExpression parameterOperand)
{
var parameterValue = _parametersValues[parameterOperand.Name];
if (sqlUnaryExpression.OperatorType == ExpressionType.Equal)
{
return SqlExpressionFactory.Constant(parameterValue == null, sqlUnaryExpression.TypeMapping);
}

if (sqlUnaryExpression.OperatorType == ExpressionType.NotEqual)
{
return SqlExpressionFactory.Constant(parameterValue != null, sqlUnaryExpression.TypeMapping);
}
}

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 sealed class InExpressionValuesExpandingExpressionVisitor : ExpressionVisitor
{
private readonly ISqlExpressionFactory _sqlExpressionFactory;
private readonly IReadOnlyDictionary<string, object> _parametersValues;

public InExpressionValuesExpandingExpressionVisitor(
ISqlExpressionFactory sqlExpressionFactory, IReadOnlyDictionary<string, object> parametersValues)
{
_sqlExpressionFactory = sqlExpressionFactory;
_parametersValues = parametersValues;
}

public override Expression Visit(Expression expression)
{
if (expression is InExpression inExpression
&& inExpression.Values != null)
{
var inValues = new List<object>();
var hasNullValue = false;
RelationalTypeMapping typeMapping = null;

switch (inExpression.Values)
{
case SqlConstantExpression sqlConstant:
{
typeMapping = sqlConstant.TypeMapping;
var values = (IEnumerable)sqlConstant.Value;
foreach (var value in values)
{
if (value == null)
{
hasNullValue = true;
continue;
}

inValues.Add(value);
}

break;
}

case SqlParameterExpression sqlParameter:
{
typeMapping = sqlParameter.TypeMapping;
var values = (IEnumerable)_parametersValues[sqlParameter.Name];
foreach (var value in values)
{
if (value == null)
{
hasNullValue = true;
continue;
}

inValues.Add(value);
}

break;
}
}

var updatedInExpression = inValues.Count > 0
? _sqlExpressionFactory.In(
(SqlExpression)Visit(inExpression.Item),
_sqlExpressionFactory.Constant(inValues, typeMapping),
inExpression.IsNegated)
: null;

var nullCheckExpression = hasNullValue
? inExpression.IsNegated
? _sqlExpressionFactory.IsNotNull(inExpression.Item)
: _sqlExpressionFactory.IsNull(inExpression.Item)
: null;

if (updatedInExpression != null
&& nullCheckExpression != null)
{
return inExpression.IsNegated
? _sqlExpressionFactory.AndAlso(updatedInExpression, nullCheckExpression)
: _sqlExpressionFactory.OrElse(updatedInExpression, nullCheckExpression);
}

if (updatedInExpression == null
&& nullCheckExpression == null)
{
return _sqlExpressionFactory.Equal(
_sqlExpressionFactory.Constant(true), _sqlExpressionFactory.Constant(inExpression.IsNegated));
}

return (SqlExpression)updatedInExpression ?? nullCheckExpression;
}

return base.Visit(expression);
}
}

private sealed class FromSqlParameterApplyingExpressionVisitor : ExpressionVisitor
{
private readonly IDictionary<FromSqlExpression, Expression> _visitedFromSqlExpressions
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ namespace Microsoft.EntityFrameworkCore.Query
{
public class RelationalQueryTranslationPostprocessor : QueryTranslationPostprocessor
{
private readonly SqlExpressionOptimizingExpressionVisitor _sqlExpressionOptimizingExpressionVisitor;

public RelationalQueryTranslationPostprocessor(
[NotNull] QueryTranslationPostprocessorDependencies dependencies,
[NotNull] RelationalQueryTranslationPostprocessorDependencies relationalDependencies,
Expand All @@ -25,8 +23,6 @@ public RelationalQueryTranslationPostprocessor(
RelationalDependencies = relationalDependencies;
UseRelationalNulls = RelationalOptionsExtension.Extract(queryCompilationContext.ContextOptions).UseRelationalNulls;
SqlExpressionFactory = relationalDependencies.SqlExpressionFactory;
_sqlExpressionOptimizingExpressionVisitor
= new SqlExpressionOptimizingExpressionVisitor(SqlExpressionFactory, UseRelationalNulls);
}

protected virtual RelationalQueryTranslationPostprocessorDependencies RelationalDependencies { get; }
Expand All @@ -42,22 +38,12 @@ public override Expression Process(Expression query)
query = new CollectionJoinApplyingExpressionVisitor().Visit(query);
query = new TableAliasUniquifyingExpressionVisitor().Visit(query);
query = new CaseWhenFlatteningExpressionVisitor(SqlExpressionFactory).Visit(query);

if (!UseRelationalNulls)
{
query = new NullSemanticsRewritingExpressionVisitor(SqlExpressionFactory).Visit(query);
}

query = OptimizeSqlExpression(query);

return query;
}

protected virtual Expression OptimizeSqlExpression([NotNull] Expression query)
{
Check.NotNull(query, nameof(query));

return _sqlExpressionOptimizingExpressionVisitor.Visit(query);
}
=> query;
}
}
4 changes: 2 additions & 2 deletions src/EFCore.Relational/Query/SqlExpressions/InExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,11 @@ protected override Expression VisitChildren(ExpressionVisitor visitor)
{
Check.NotNull(visitor, nameof(visitor));

var newItem = (SqlExpression)visitor.Visit(Item);
var item = (SqlExpression)visitor.Visit(Item);
var subquery = (SelectExpression)visitor.Visit(Subquery);
var values = (SqlExpression)visitor.Visit(Values);

return Update(newItem, values, subquery);
return Update(item, values, subquery);
}

public virtual InExpression Negate() => new InExpression(Item, !IsNegated, Values, Subquery, TypeMapping);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.Collections.Generic;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Query;
using Microsoft.EntityFrameworkCore.Query.Internal;
using Microsoft.EntityFrameworkCore.Query.SqlExpressions;
using Microsoft.EntityFrameworkCore.Utilities;

Expand All @@ -30,7 +31,10 @@ public override (SelectExpression selectExpression, bool canCache) Optimize(
var searchConditionOptimized = (SelectExpression)new SearchConditionConvertingExpressionVisitor(Dependencies.SqlExpressionFactory)
.Visit(optimizedSelectExpression);

return (searchConditionOptimized, canCache);
var optimized = (SelectExpression)new SqlExpressionOptimizingExpressionVisitor(
Dependencies.SqlExpressionFactory, UseRelationalNulls, parametersValues).Visit(searchConditionOptimized);

return (optimized, canCache);
}
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.Linq.Expressions;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Query;

Expand All @@ -16,13 +15,5 @@ public SqlServerQueryTranslationPostprocessor(
: base(dependencies, relationalDependencies, queryCompilationContext)
{
}

public override Expression Process(Expression query)
{
query = base.Process(query);
query = new SearchConditionConvertingExpressionVisitor(SqlExpressionFactory).Visit(query);

return query;
}
}
}
Loading

0 comments on commit ff0f085

Please sign in to comment.