Skip to content

Commit

Permalink
Query: Improve extensibility of null semantics processor
Browse files Browse the repository at this point in the history
Resolves #20204

For table sources, there is no concept of nullability
- `Process(TableExpressionBase)` which works as dispatcher.
- `Process(SelectExpression)` as a special case to avoid casting.
- No additional methods for individual table sources.

For SQL expressions,
- `Process(SqlExpression, out bool nullable)` which defaults allowOptimizedExpansion to false.
- `Process(SqlExpression, allowOptimizedExpansion, out bool nullable)` which works as dispatcher.
- Individual Process* methods to change bahavior of any particular SqlExpression.

Notes:
- Each individual SQLExpression processes it's children with appropriately set allowOptimizedExpansion flag. It collects nullable flag for all children and return nullable for itself through out parameter.
- NonNullableColumns are being used only in 2 places and both usages are different. Rather than crystal balling an API to expose it, we should just keep it private till a provider asks for it then we can discuss with provider writer what information is needed and how would they like it. Making it private does not cause any bug, may just miss an optimization in SQL.
- The processor does not derive from ExpressionVisitor anymore. It has ExpressionVisitor like dispatch system but non Visit API.
- Since it is not deriving from SqlExpressionVisitor there is no longer abstract method to force implementing for new SqlExpression type. Hence unknown expression type throws exception. Making it pass through could cause incorrect result as we don't visit custom expression's components.
- Added DoNotCache method rather than exposing _canCache.
- Changed API about returning tuple for caching to an out parameter.

TODO: Once API is approved, I will add XML docs.
  • Loading branch information
smitpatel committed May 6, 2020
1 parent 7793514 commit d97c914
Show file tree
Hide file tree
Showing 7 changed files with 501 additions and 508 deletions.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions src/EFCore.Relational/Properties/RelationalStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -488,8 +488,8 @@
<data name="ClientGroupByNotSupported" xml:space="preserve">
<value>Client side GroupBy is not supported.</value>
</data>
<data name="UnexpectedJoinPredicateShape" xml:space="preserve">
<value>Unexpected join predicate shape: {predicate}.</value>
<data name="UnknownExpressionType" xml:space="preserve">
<value>Unknown expression '{expression}' of type - '{expressionType}' encountered in '{visitor}'.</value>
</data>
<data name="NoTypeMappingFoundForSubquery" xml:space="preserve">
<value>The subquery '{subquery}' references type '{type}' for which no type mapping could be found.</value>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ public virtual IRelationalCommand GetRelationalCommand([NotNull] IReadOnlyDictio

try
{
var (selectExpression, canCache) =
_relationalParameterBasedQueryTranslationPostprocessor.Optimize(_selectExpression, parameters);
var selectExpression = _relationalParameterBasedQueryTranslationPostprocessor.Optimize(
_selectExpression, parameters, out var canCache);
relationalCommand = _querySqlGeneratorFactory.Create().GetCommand(selectExpression);

if (canCache)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,34 +50,33 @@ public RelationalParameterBasedQueryTranslationPostprocessor(
/// </summary>
/// <param name="selectExpression"> A select expression to optimize. </param>
/// <param name="parametersValues"> A dictionary of parameter values to use. </param>
/// <returns> A tuple of optimized select expression and a bool value if it can be cached. </returns>
public virtual (SelectExpression, bool) Optimize(
/// <param name="canCache"> A bool value indicating if the select expression can be cached. </param>
/// <returns> An optimized select expression. </returns>
public virtual SelectExpression Optimize(
[NotNull] SelectExpression selectExpression,
[NotNull] IReadOnlyDictionary<string, object> parametersValues)
[NotNull] IReadOnlyDictionary<string, object> parametersValues,
out bool canCache)
{
Check.NotNull(selectExpression, nameof(selectExpression));
Check.NotNull(parametersValues, nameof(parametersValues));

var canCache = true;
var (sqlExpressionOptimized, optimizerCanCache) = new NullabilityBasedSqlProcessingExpressionVisitor(
selectExpression = new SqlNullabilityProcessor(
Dependencies.SqlExpressionFactory,
parametersValues,
UseRelationalNulls).Process(selectExpression);

canCache &= optimizerCanCache;
UseRelationalNulls).Process(selectExpression, out canCache);

var fromSqlParameterOptimized = new FromSqlParameterApplyingExpressionVisitor(
Dependencies.SqlExpressionFactory,
Dependencies.TypeMappingSource,
Dependencies.ParameterNameGeneratorFactory.Create(),
parametersValues).Visit(sqlExpressionOptimized);
parametersValues).Visit(selectExpression);

if (!ReferenceEquals(sqlExpressionOptimized, fromSqlParameterOptimized))
if (!ReferenceEquals(selectExpression, fromSqlParameterOptimized))
{
canCache = false;
}

return ((SelectExpression)fromSqlParameterOptimized, canCache);
return (SelectExpression)fromSqlParameterOptimized;
}
}
}
Loading

0 comments on commit d97c914

Please sign in to comment.