Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Query cleanup: Avoid static parameters #16167

Merged
merged 1 commit into from
Jun 21, 2019
Merged

Query cleanup: Avoid static parameters #16167

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 13 additions & 10 deletions src/EFCore.Cosmos/Query/Pipeline/CosmosShapedQueryCompilingExpressionVisitor.cs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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.FindRelationalMapping(),
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
Open in desktop
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