From b7fbe4b573c6d62e7e20ee29f598db708adf0f91 Mon Sep 17 00:00:00 2001 From: Smit Patel Date: Wed, 19 Jun 2019 17:26:44 -0700 Subject: [PATCH] Query cleanup: Avoid static parameters --- ...osShapedQueryCompilingExpressionVisitor.cs | 23 +++++++++++-------- ...emoryProjectionBindingExpressionVisitor.cs | 6 ++--- .../Query/Pipeline/InMemoryQueryExpression.cs | 4 ++-- .../InMemoryShapedQueryExpressionVisitor.cs | 4 ++-- .../Query/Pipeline/Translator.cs | 2 +- .../IncludeCompilingExpressionVisitor.cs | 21 +++++++++++------ ...jectionBindingRemovingExpressionVisitor.cs | 22 ++++++++++-------- ...alShapedQueryCompilingExpressionVisitor.cs | 15 +++++++----- 8 files changed, 57 insertions(+), 40 deletions(-) diff --git a/src/EFCore.Cosmos/Query/Pipeline/CosmosShapedQueryCompilingExpressionVisitor.cs b/src/EFCore.Cosmos/Query/Pipeline/CosmosShapedQueryCompilingExpressionVisitor.cs index 06253d17d7f..ecf61db76e8 100644 --- a/src/EFCore.Cosmos/Query/Pipeline/CosmosShapedQueryCompilingExpressionVisitor.cs +++ b/src/EFCore.Cosmos/Query/Pipeline/CosmosShapedQueryCompilingExpressionVisitor.cs @@ -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 @@ -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)) @@ -89,9 +90,11 @@ private static readonly MethodInfo _isNullMethodInfo private readonly IDictionary _materializationContextBindings = new Dictionary(); - public CosmosProjectionBindingRemovingExpressionVisitor(SelectExpression selectExpression) + public CosmosProjectionBindingRemovingExpressionVisitor( + SelectExpression selectExpression, ParameterExpression jObjectParameter) { _selectExpression = selectExpression; + _jObjectParameter = jObjectParameter; } protected override Expression VisitBinary(BinaryExpression binaryExpression) @@ -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, @@ -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 @@ -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); @@ -435,8 +438,8 @@ public async ValueTask 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( diff --git a/src/EFCore.InMemory/Query/Pipeline/InMemoryProjectionBindingExpressionVisitor.cs b/src/EFCore.InMemory/Query/Pipeline/InMemoryProjectionBindingExpressionVisitor.cs index eb69b794a49..027042699cf 100644 --- a/src/EFCore.InMemory/Query/Pipeline/InMemoryProjectionBindingExpressionVisitor.cs +++ b/src/EFCore.InMemory/Query/Pipeline/InMemoryProjectionBindingExpressionVisitor.cs @@ -34,7 +34,7 @@ public Expression Translate(InMemoryQueryExpression queryExpression, Expression var result = Visit(expression); - _queryExpression.ApplyProjection(_projectionMapping); + _queryExpression.ReplaceProjectionMapping(_projectionMapping); _queryExpression = null; _projectionMapping.Clear(); @@ -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); @@ -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( diff --git a/src/EFCore.InMemory/Query/Pipeline/InMemoryQueryExpression.cs b/src/EFCore.InMemory/Query/Pipeline/InMemoryQueryExpression.cs index 1db5160bd47..b6525be6a04 100644 --- a/src/EFCore.InMemory/Query/Pipeline/InMemoryQueryExpression.cs +++ b/src/EFCore.InMemory/Query/Pipeline/InMemoryQueryExpression.cs @@ -102,7 +102,7 @@ public Expression BindProperty(Expression projectionExpression, IProperty proper return _valueBufferSlots[offset + property.GetIndex()]; } - public void ApplyProjection(IDictionary projectionMappings) + public void ReplaceProjectionMapping(IDictionary projectionMappings) { _valueBufferSlots.Clear(); _projectionMapping.Clear(); @@ -130,7 +130,7 @@ public void ApplyProjection(IDictionary projection } } - public Expression GetProjectionExpression(ProjectionMember member) + public Expression GetMappedProjection(ProjectionMember member) { var projection = _projectionMapping[member]; if (projection is EntityProjectionExpression entityValues) diff --git a/src/EFCore.InMemory/Query/Pipeline/InMemoryShapedQueryExpressionVisitor.cs b/src/EFCore.InMemory/Query/Pipeline/InMemoryShapedQueryExpressionVisitor.cs index a748cf04cc0..3eb7d80aa57 100644 --- a/src/EFCore.InMemory/Query/Pipeline/InMemoryShapedQueryExpressionVisitor.cs +++ b/src/EFCore.InMemory/Query/Pipeline/InMemoryShapedQueryExpressionVisitor.cs @@ -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( @@ -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); diff --git a/src/EFCore.InMemory/Query/Pipeline/Translator.cs b/src/EFCore.InMemory/Query/Pipeline/Translator.cs index aa17f646615..b7c8de9cbaf 100644 --- a/src/EFCore.InMemory/Query/Pipeline/Translator.cs +++ b/src/EFCore.InMemory/Query/Pipeline/Translator.cs @@ -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) diff --git a/src/EFCore.Relational/Query/Pipeline/IncludeCompilingExpressionVisitor.cs b/src/EFCore.Relational/Query/Pipeline/IncludeCompilingExpressionVisitor.cs index bee96f1ba58..31d3e798278 100644 --- a/src/EFCore.Relational/Query/Pipeline/IncludeCompilingExpressionVisitor.cs +++ b/src/EFCore.Relational/Query/Pipeline/IncludeCompilingExpressionVisitor.cs @@ -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; } @@ -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)), @@ -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)), diff --git a/src/EFCore.Relational/Query/Pipeline/RelationalProjectionBindingRemovingExpressionVisitor.cs b/src/EFCore.Relational/Query/Pipeline/RelationalProjectionBindingRemovingExpressionVisitor.cs index 1081f946896..3cdbaf679eb 100644 --- a/src/EFCore.Relational/Query/Pipeline/RelationalProjectionBindingRemovingExpressionVisitor.cs +++ b/src/EFCore.Relational/Query/Pipeline/RelationalProjectionBindingRemovingExpressionVisitor.cs @@ -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> _materializationContextBindings = new Dictionary>(); - public RelationalProjectionBindingRemovingExpressionVisitor(SelectExpression selectExpression) + public RelationalProjectionBindingRemovingExpressionVisitor( + SelectExpression selectExpression, ParameterExpression dbDataReaderParameter) { _selectExpression = selectExpression; + _dbDataReaderParameter = dbDataReaderParameter; } protected override Expression VisitBinary(BinaryExpression binaryExpression) @@ -76,6 +80,7 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp var projection = _selectExpression.Projection[projectionIndex]; return CreateGetValueExpression( + _dbDataReaderParameter, projectionIndex, IsNullableProjection(projection), property.FindRelationalMapping(), @@ -93,6 +98,7 @@ protected override Expression VisitExtension(Expression extensionExpression) var projection = _selectExpression.Projection[projectionIndex]; return CreateGetValueExpression( + _dbDataReaderParameter, projectionIndex, IsNullableProjection(projection), projection.Expression.TypeMapping, @@ -127,6 +133,7 @@ private static bool IsNullableProjection(ProjectionExpression projection) } private static Expression CreateGetValueExpression( + Expression dbDataReader, int index, bool nullable, RelationalTypeMapping typeMapping, @@ -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); @@ -199,7 +206,7 @@ Expression valueExpression { valueExpression = Expression.Condition( - Expression.Call(DataReaderParameter, _isDbNullMethod, indexExpression), + Expression.Call(dbDataReader, _isDbNullMethod, indexExpression), Expression.Default(valueExpression.Type), valueExpression); } @@ -207,10 +214,7 @@ Expression valueExpression return valueExpression; } - private static readonly MethodInfo _isDbNullMethod = - typeof(DbDataReader).GetRuntimeMethod(nameof(DbDataReader.IsDBNull), new[] { typeof(int) }); - private readonly SelectExpression _selectExpression; } } } diff --git a/src/EFCore.Relational/Query/Pipeline/RelationalShapedQueryCompilingExpressionVisitor.cs b/src/EFCore.Relational/Query/Pipeline/RelationalShapedQueryCompilingExpressionVisitor.cs index 56863001504..b2398f86061 100644 --- a/src/EFCore.Relational/Query/Pipeline/RelationalShapedQueryCompilingExpressionVisitor.cs +++ b/src/EFCore.Relational/Query/Pipeline/RelationalShapedQueryCompilingExpressionVisitor.cs @@ -21,8 +21,6 @@ public partial class RelationalShapedQueryCompilingExpressionVisitor : ShapedQue private readonly IParameterNameGeneratorFactory _parameterNameGeneratorFactory; private readonly Type _contextType; private readonly IDiagnosticsLogger _logger; - private static readonly ParameterExpression _resultCoordinatorParameter - = Expression.Parameter(typeof(ResultCoordinator), "resultCoordinator"); public RelationalShapedQueryCompilingExpressionVisitor( IEntityMaterializerSource entityMaterializerSource, @@ -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()) { @@ -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