Skip to content

Commit

Permalink
Query cleanup: Avoid static parameters
Browse files Browse the repository at this point in the history
  • Loading branch information
@smitpatel
smitpatel committed Jun 21, 2019
1 parent 74f9c4d commit ab1c1f4
Show file tree
Hide file tree
Showing 8 changed files with 57 additions and 40 deletions.
23 changes: 13 additions & 10 deletions src/EFCore.Cosmos/Query/Pipeline/CosmosShapedQueryCompilingExpressionVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,15 @@ protected override Expression VisitShapedQueryExpression(ShapedQueryExpression s
var shaperBody = InjectEntityMaterializer(shapedQueryExpression.ShaperExpression);
var selectExpression = (SelectExpression)shapedQueryExpression.QueryExpression;
selectExpression.ApplyProjection();
var jObjectParameter = Expression.Parameter(typeof(JObject), "jObject");

shaperBody = new CosmosProjectionBindingRemovingExpressionVisitor(selectExpression).Visit(shaperBody);
shaperBody = new CosmosProjectionBindingRemovingExpressionVisitor(selectExpression, jObjectParameter)
.Visit(shaperBody);

var shaperLambda = Expression.Lambda(
shaperBody,
QueryCompilationContext.QueryContextParameter,
CosmosProjectionBindingRemovingExpressionVisitor.jObjectParameter);
jObjectParameter);

return Expression.New(
(Async
Expand All @@ -72,9 +74,8 @@ protected override Expression VisitShapedQueryExpression(ShapedQueryExpression s

private class CosmosProjectionBindingRemovingExpressionVisitor : ExpressionVisitor
{
public static readonly ParameterExpression jObjectParameter
= Expression.Parameter(typeof(JObject), "jObject");
private SelectExpression _selectExpression;
private readonly ParameterExpression _jObjectParameter;
private static readonly MethodInfo _getItemMethodInfo
= typeof(JObject).GetTypeInfo().GetRuntimeProperties()
.Single(pi => pi.Name == "Item" && pi.GetIndexParameters()[0].ParameterType == typeof(string))
Expand All @@ -89,9 +90,11 @@ private static readonly MethodInfo _isNullMethodInfo
private readonly IDictionary<ParameterExpression, Expression> _materializationContextBindings
= new Dictionary<ParameterExpression, Expression>();

public CosmosProjectionBindingRemovingExpressionVisitor(SelectExpression selectExpression)
public CosmosProjectionBindingRemovingExpressionVisitor(
SelectExpression selectExpression, ParameterExpression jObjectParameter)
{
_selectExpression = selectExpression;
_jObjectParameter = jObjectParameter;
}

protected override Expression VisitBinary(BinaryExpression binaryExpression)
Expand All @@ -106,7 +109,7 @@ protected override Expression VisitBinary(BinaryExpression binaryExpression)
var projection = _selectExpression.Projection[projectionIndex];

_materializationContextBindings[parameterExpression] = Expression.Convert(
CreateReadJTokenExpression(jObjectParameter, projection.Alias),
CreateReadJTokenExpression(_jObjectParameter, projection.Alias),
typeof(JObject));

var updatedExpression = Expression.New(newExpression.Constructor,
Expand Down Expand Up @@ -141,7 +144,7 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
var projection = _selectExpression.Projection[projectionIndex];

innerExpression = Expression.Convert(
CreateReadJTokenExpression(jObjectParameter, projection.Alias),
CreateReadJTokenExpression(_jObjectParameter, projection.Alias),
typeof(JObject));
}
else
Expand Down Expand Up @@ -171,7 +174,7 @@ protected override Expression VisitExtension(Expression extensionExpression)
var projection = _selectExpression.Projection[projectionIndex];

return CreateGetStoreValueExpression(
jObjectParameter,
_jObjectParameter,
projection.Alias,
((SqlExpression)projection.Expression).TypeMapping,
projectionBindingExpression.Type);
Expand Down Expand Up @@ -435,8 +438,8 @@ public async ValueTask<bool> MoveNextAsync()
{
if (_enumerator == null)
{
var selectExpression = (SelectExpression)new InExpressionValuesExpandingExpressionVisitor(
_sqlExpressionFactory, _cosmosQueryContext.ParameterValues).Visit(_selectExpression);
var selectExpression = (SelectExpression)new InExpressionValuesExpandingExpressionVisitor(
_sqlExpressionFactory, _cosmosQueryContext.ParameterValues).Visit(_selectExpression);

_enumerator = _cosmosQueryContext.CosmosClient
.ExecuteSqlQueryAsync(
Expand Down
6 changes: 3 additions & 3 deletions src/EFCore.InMemory/Query/Pipeline/InMemoryProjectionBindingExpressionVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public Expression Translate(InMemoryQueryExpression queryExpression, Expression

var result = Visit(expression);

_queryExpression.ApplyProjection(_projectionMapping);
_queryExpression.ReplaceProjectionMapping(_projectionMapping);

_queryExpression = null;
_projectionMapping.Clear();
Expand Down Expand Up @@ -70,7 +70,7 @@ public override Expression Visit(Expression expression)
VerifyQueryExpression(entityShaperExpression.ValueBufferExpression);

_projectionMapping[_projectionMembers.Peek()]
= _queryExpression.GetProjectionExpression(
= _queryExpression.GetMappedProjection(
entityShaperExpression.ValueBufferExpression.ProjectionMember);

return new EntityValuesExpression(entityShaperExpression.EntityType, entityShaperExpression.ValueBufferExpression);
Expand All @@ -93,7 +93,7 @@ protected override Expression VisitExtension(Expression extensionExpression)
VerifyQueryExpression(entityShaperExpression.ValueBufferExpression);

_projectionMapping[_projectionMembers.Peek()]
= _queryExpression.GetProjectionExpression(
= _queryExpression.GetMappedProjection(
entityShaperExpression.ValueBufferExpression.ProjectionMember);

return entityShaperExpression.Update(
Expand Down
4 changes: 2 additions & 2 deletions src/EFCore.InMemory/Query/Pipeline/InMemoryQueryExpression.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ public Expression BindProperty(Expression projectionExpression, IProperty proper
return _valueBufferSlots[offset + property.GetIndex()];
}

public void ApplyProjection(IDictionary<ProjectionMember, Expression> projectionMappings)
public void ReplaceProjectionMapping(IDictionary<ProjectionMember, Expression> projectionMappings)
{
_valueBufferSlots.Clear();
_projectionMapping.Clear();
Expand Down Expand Up @@ -130,7 +130,7 @@ public void ApplyProjection(IDictionary<ProjectionMember, Expression> projection
}
}

public Expression GetProjectionExpression(ProjectionMember member)
public Expression GetMappedProjection(ProjectionMember member)
{
var projection = _projectionMapping[member];
if (projection is EntityProjectionExpression entityValues)
Expand Down
4 changes: 2 additions & 2 deletions src/EFCore.InMemory/Query/Pipeline/InMemoryShapedQueryExpressionVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
{
var originalIndex = (int)((ConstantExpression)methodCallExpression.Arguments[1]).Value;
var indexOffset = methodCallExpression.Arguments[0] is ProjectionBindingExpression projectionBindingExpression
? ((EntityProjectionExpression)_queryExpression.GetProjectionExpression(projectionBindingExpression.ProjectionMember)).StartIndex
? ((EntityProjectionExpression)_queryExpression.GetMappedProjection(projectionBindingExpression.ProjectionMember)).StartIndex
: _materializationContextBindings[(ParameterExpression)((MethodCallExpression)methodCallExpression.Arguments[0]).Object];

return Expression.Call(
Expand All @@ -321,7 +321,7 @@ protected override Expression VisitExtension(Expression extensionExpression)
{
if (extensionExpression is ProjectionBindingExpression projectionBindingExpression)
{
return _queryExpression.GetProjectionExpression(projectionBindingExpression.ProjectionMember);
return _queryExpression.GetMappedProjection(projectionBindingExpression.ProjectionMember);
}

return base.VisitExtension(extensionExpression);
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore.InMemory/Query/Pipeline/Translator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ protected override Expression VisitExtension(Expression extensionExpression)

if (extensionExpression is ProjectionBindingExpression projectionBindingExpression)
{
return _inMemoryQueryExpression.GetProjectionExpression(projectionBindingExpression.ProjectionMember);
return _inMemoryQueryExpression.GetMappedProjection(projectionBindingExpression.ProjectionMember);
}

if (extensionExpression is NullConditionalExpression nullConditionalExpression)
Expand Down
21 changes: 14 additions & 7 deletions src/EFCore.Relational/Query/Pipeline/IncludeCompilingExpressionVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,17 @@ public partial class RelationalShapedQueryCompilingExpressionVisitor
{
private class IncludeCompilingExpressionVisitor : ExpressionVisitor
{
private readonly ParameterExpression _dbDataReaderParameter;
private readonly ParameterExpression _resultCoordinatorParameter;
private readonly bool _tracking;

public IncludeCompilingExpressionVisitor(bool tracking)
public IncludeCompilingExpressionVisitor(
ParameterExpression dbDataReaderParameter,
ParameterExpression resultCoordinatorParameter,
bool tracking)
{
_dbDataReaderParameter = dbDataReaderParameter;
_resultCoordinatorParameter = resultCoordinatorParameter;
_tracking = tracking;
}

Expand Down Expand Up @@ -173,24 +180,24 @@ protected override Expression VisitExtension(Expression extensionExpression)
return Expression.Call(
_includeCollectionMethodInfo.MakeGenericMethod(entityClrType, relatedEntityClrType),
QueryCompilationContext.QueryContextParameter,
RelationalProjectionBindingRemovingExpressionVisitor.DataReaderParameter,
_dbDataReaderParameter,
// We don't need to visit entityExpression since it is supposed to be a parameterExpression only
includeExpression.EntityExpression,
Expression.Constant(
Expression.Lambda(
collectionShaper.OuterKeySelector,
QueryCompilationContext.QueryContextParameter,
RelationalProjectionBindingRemovingExpressionVisitor.DataReaderParameter).Compile()),
_dbDataReaderParameter).Compile()),
Expression.Constant(
Expression.Lambda(
collectionShaper.InnerKeySelector,
QueryCompilationContext.QueryContextParameter,
RelationalProjectionBindingRemovingExpressionVisitor.DataReaderParameter).Compile()),
_dbDataReaderParameter).Compile()),
Expression.Constant(
Expression.Lambda(
innerShaper,
QueryCompilationContext.QueryContextParameter,
RelationalProjectionBindingRemovingExpressionVisitor.DataReaderParameter,
_dbDataReaderParameter,
_resultCoordinatorParameter).Compile()),
Expression.Constant(includeExpression.Navigation),
Expression.Constant(inverseNavigation, typeof(INavigation)),
Expand All @@ -207,14 +214,14 @@ protected override Expression VisitExtension(Expression extensionExpression)
return Expression.Call(
_includeReferenceMethodInfo.MakeGenericMethod(entityClrType, relatedEntityClrType),
QueryCompilationContext.QueryContextParameter,
RelationalProjectionBindingRemovingExpressionVisitor.DataReaderParameter,
_dbDataReaderParameter,
// We don't need to visit entityExpression since it is supposed to be a parameterExpression only
includeExpression.EntityExpression,
Expression.Constant(
Expression.Lambda(
Visit(includeExpression.NavigationExpression),
QueryCompilationContext.QueryContextParameter,
RelationalProjectionBindingRemovingExpressionVisitor.DataReaderParameter,
_dbDataReaderParameter,
_resultCoordinatorParameter).Compile()),
Expression.Constant(includeExpression.Navigation),
Expression.Constant(inverseNavigation, typeof(INavigation)),
Expand Down
22 changes: 13 additions & 9 deletions src/EFCore.Relational/Query/Pipeline/RelationalProjectionBindingRemovingExpressionVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,19 @@ public partial class RelationalShapedQueryCompilingExpressionVisitor
{
private class RelationalProjectionBindingRemovingExpressionVisitor : ExpressionVisitor
{
public static readonly ParameterExpression DataReaderParameter
= Expression.Parameter(typeof(DbDataReader), "dataReader");
private static readonly MethodInfo _isDbNullMethod =
typeof(DbDataReader).GetRuntimeMethod(nameof(DbDataReader.IsDBNull), new[] { typeof(int) });

private readonly SelectExpression _selectExpression;
private readonly ParameterExpression _dbDataReaderParameter;
private readonly IDictionary<ParameterExpression, IDictionary<IProperty, int>> _materializationContextBindings
= new Dictionary<ParameterExpression, IDictionary<IProperty, int>>();

public RelationalProjectionBindingRemovingExpressionVisitor(SelectExpression selectExpression)
public RelationalProjectionBindingRemovingExpressionVisitor(
SelectExpression selectExpression, ParameterExpression dbDataReaderParameter)
{
_selectExpression = selectExpression;
_dbDataReaderParameter = dbDataReaderParameter;
}

protected override Expression VisitBinary(BinaryExpression binaryExpression)
Expand Down Expand Up @@ -76,6 +80,7 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
var projection = _selectExpression.Projection[projectionIndex];

return CreateGetValueExpression(
_dbDataReaderParameter,
projectionIndex,
IsNullableProjection(projection),
property.GetRelationalTypeMapping(),
Expand All @@ -93,6 +98,7 @@ protected override Expression VisitExtension(Expression extensionExpression)
var projection = _selectExpression.Projection[projectionIndex];

return CreateGetValueExpression(
_dbDataReaderParameter,
projectionIndex,
IsNullableProjection(projection),
projection.Expression.TypeMapping,
Expand Down Expand Up @@ -127,6 +133,7 @@ private static bool IsNullableProjection(ProjectionExpression projection)
}

private static Expression CreateGetValueExpression(
Expression dbDataReader,
int index,
bool nullable,
RelationalTypeMapping typeMapping,
Expand All @@ -139,8 +146,8 @@ private static Expression CreateGetValueExpression(
Expression valueExpression
= Expression.Call(
getMethod.DeclaringType != typeof(DbDataReader)
? Expression.Convert(DataReaderParameter, getMethod.DeclaringType)
: (Expression)DataReaderParameter,
? Expression.Convert(dbDataReader, getMethod.DeclaringType)
: (Expression)dbDataReader,
getMethod,
indexExpression);

Expand Down Expand Up @@ -199,18 +206,15 @@ Expression valueExpression
{
valueExpression
= Expression.Condition(
Expression.Call(DataReaderParameter, _isDbNullMethod, indexExpression),
Expression.Call(dbDataReader, _isDbNullMethod, indexExpression),
Expression.Default(valueExpression.Type),
valueExpression);
}

return valueExpression;
}

private static readonly MethodInfo _isDbNullMethod =
typeof(DbDataReader).GetRuntimeMethod(nameof(DbDataReader.IsDBNull), new[] { typeof(int) });

private readonly SelectExpression _selectExpression;
}
}
}
15 changes: 9 additions & 6 deletions src/EFCore.Relational/Query/Pipeline/RelationalShapedQueryCompilingExpressionVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ public partial class RelationalShapedQueryCompilingExpressionVisitor : ShapedQue
private readonly IParameterNameGeneratorFactory _parameterNameGeneratorFactory;
private readonly Type _contextType;
private readonly IDiagnosticsLogger<DbLoggerCategory.Query> _logger;
private static readonly ParameterExpression _resultCoordinatorParameter
= Expression.Parameter(typeof(ResultCoordinator), "resultCoordinator");

public RelationalShapedQueryCompilingExpressionVisitor(
IEntityMaterializerSource entityMaterializerSource,
Expand All @@ -48,9 +46,14 @@ protected override Expression VisitShapedQueryExpression(ShapedQueryExpression s

var selectExpression = (SelectExpression)shapedQueryExpression.QueryExpression;

shaperBody = new RelationalProjectionBindingRemovingExpressionVisitor(selectExpression).Visit(shaperBody);
shaperBody = new IncludeCompilingExpressionVisitor(TrackQueryResults).Visit(shaperBody);
var dataReaderParameter = Expression.Parameter(typeof(DbDataReader), "dataReader");
var indexMapParameter = Expression.Parameter(typeof(int[]), "indexMap");
var resultCoordinatorParameter = Expression.Parameter(typeof(ResultCoordinator), "resultCoordinator");

shaperBody = new RelationalProjectionBindingRemovingExpressionVisitor(selectExpression, dataReaderParameter)
.Visit(shaperBody);
shaperBody = new IncludeCompilingExpressionVisitor(dataReaderParameter, resultCoordinatorParameter, TrackQueryResults)
.Visit(shaperBody);

if (selectExpression.IsNonComposedFromSql())
{
Expand All @@ -60,9 +63,9 @@ protected override Expression VisitShapedQueryExpression(ShapedQueryExpression s
var shaperLambda = Expression.Lambda(
shaperBody,
QueryCompilationContext.QueryContextParameter,
RelationalProjectionBindingRemovingExpressionVisitor.DataReaderParameter,
dataReaderParameter,
indexMapParameter,
_resultCoordinatorParameter);
resultCoordinatorParameter);

return Expression.New(
(Async
Expand Down

0 comments on commit ab1c1f4

Please sign in to comment.