Skip to content

Commit

Permalink
Query: Make CreateReadValueExpression an extension method
Browse files Browse the repository at this point in the history
This method is to read a particular index from valueBuffer for a property.
It was in EntityMaterializerSource as an instance method in order for provider to influence how values are read from valueBuffer.
In new query pipeline, providers are supposed to convert valueBuffer read to their database object read. Further, even if they want to create a ValueBuffer for read, the valueBuffer should have values after all custom processing.
Hence there is no particular need to make this overridable behavior. Making it extension method simplify quite a lot of things.

(note: functionality to use this was available anyway since the method it invoke was also publicly exposed as static.
  • Loading branch information
smitpatel committed Mar 17, 2020
1 parent 0573af8 commit 5fd8577
Show file tree
Hide file tree
Showing 10 changed files with 73 additions and 84 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp

var method = methodCallExpression.Method;
var genericMethod = method.IsGenericMethod ? method.GetGenericMethodDefinition() : null;
if (genericMethod == EntityMaterializerSource.TryReadValueMethod)
if (genericMethod == EntityFrameworkCore.Infrastructure.ExpressionExtensions.ValueBufferTryReadValueMethod)
{
var property = (IProperty)((ConstantExpression)methodCallExpression.Arguments[2]).Value;
Expression innerExpression;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
using Microsoft.EntityFrameworkCore.Query;
using Microsoft.EntityFrameworkCore.Storage;
using Microsoft.EntityFrameworkCore.Utilities;
using ExpressionExtensions = Microsoft.EntityFrameworkCore.Infrastructure.ExpressionExtensions;

namespace Microsoft.EntityFrameworkCore.InMemory.Query.Internal
{
Expand Down Expand Up @@ -399,7 +400,7 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
Check.NotNull(methodCallExpression, nameof(methodCallExpression));

if (methodCallExpression.Method.IsGenericMethod
&& methodCallExpression.Method.GetGenericMethodDefinition() == EntityMaterializerSource.TryReadValueMethod)
&& methodCallExpression.Method.GetGenericMethodDefinition() == ExpressionExtensions.ValueBufferTryReadValueMethod)
{
return methodCallExpression;
}
Expand Down
21 changes: 7 additions & 14 deletions src/EFCore.InMemory/Query/Internal/InMemoryQueryExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,14 @@
using System.Reflection;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.InMemory.Internal;
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Metadata.Internal;
using Microsoft.EntityFrameworkCore.Query;
using Microsoft.EntityFrameworkCore.Storage;
using Microsoft.EntityFrameworkCore.Utilities;
using ExpressionExtensions = Microsoft.EntityFrameworkCore.Infrastructure.ExpressionExtensions;

namespace Microsoft.EntityFrameworkCore.InMemory.Query.Internal
{
Expand Down Expand Up @@ -324,7 +326,7 @@ public virtual void ApplyDefaultIfEmpty()
private static IPropertyBase InferPropertyFromInner(Expression expression)
=> expression is MethodCallExpression methodCallExpression
&& methodCallExpression.Method.IsGenericMethod
&& methodCallExpression.Method.GetGenericMethodDefinition() == EntityMaterializerSource.TryReadValueMethod
&& methodCallExpression.Method.GetGenericMethodDefinition() == ExpressionExtensions.ValueBufferTryReadValueMethod
? (IPropertyBase)((ConstantExpression)methodCallExpression.Arguments[2]).Value
: null;

Expand Down Expand Up @@ -440,24 +442,15 @@ private Expression GetGroupingKey(Expression key, List<Expression> groupingExpre
default:
var index = groupingExpressions.Count;
groupingExpressions.Add(key);
return CreateReadValueExpression(
groupingKeyAccessExpression,
return groupingKeyAccessExpression.CreateValueBufferReadValueExpression(
key.Type,
index,
InferPropertyFromInner(key));
}
}

private static Expression CreateReadValueExpression(
Expression valueBufferParameter, Type type, int index, IPropertyBase property)
=> Call(
EntityMaterializerSource.TryReadValueMethod.MakeGenericMethod(type),
valueBufferParameter,
Constant(index),
Constant(property, typeof(IPropertyBase)));

private Expression CreateReadValueExpression(Type type, int index, IPropertyBase property)
=> CreateReadValueExpression(_valueBufferParameter, type, index, property);
=> _valueBufferParameter.CreateValueBufferReadValueExpression(type, index, property);

public virtual void AddInnerJoin(
[NotNull] InMemoryQueryExpression innerQueryExpression,
Expand Down Expand Up @@ -943,11 +936,11 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
Check.NotNull(methodCallExpression, nameof(methodCallExpression));

if (methodCallExpression.Method.IsGenericMethod
&& methodCallExpression.Method.GetGenericMethodDefinition() == EntityMaterializerSource.TryReadValueMethod
&& methodCallExpression.Method.GetGenericMethodDefinition() == ExpressionExtensions.ValueBufferTryReadValueMethod
&& !methodCallExpression.Type.IsNullableType())
{
return Call(
EntityMaterializerSource.TryReadValueMethod.MakeGenericMethod(methodCallExpression.Type.MakeNullable()),
ExpressionExtensions.ValueBufferTryReadValueMethod.MakeGenericMethod(methodCallExpression.Type.MakeNullable()),
methodCallExpression.Arguments);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using Microsoft.EntityFrameworkCore.Query;
using Microsoft.EntityFrameworkCore.Storage;
using Microsoft.EntityFrameworkCore.Utilities;
using ExpressionExtensions = Microsoft.EntityFrameworkCore.Infrastructure.ExpressionExtensions;

namespace Microsoft.EntityFrameworkCore.InMemory.Query.Internal
{
Expand Down Expand Up @@ -61,7 +62,7 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
Check.NotNull(methodCallExpression, nameof(methodCallExpression));

if (methodCallExpression.Method.IsGenericMethod
&& methodCallExpression.Method.GetGenericMethodDefinition() == EntityMaterializerSource.TryReadValueMethod)
&& methodCallExpression.Method.GetGenericMethodDefinition() == ExpressionExtensions.ValueBufferTryReadValueMethod)
{
var property = (IProperty)((ConstantExpression)methodCallExpression.Arguments[2]).Value;
var (indexMap, valueBuffer) =
Expand All @@ -88,11 +89,10 @@ protected override Expression VisitExtension(Expression extensionExpression)
var projectionIndex = (int)GetProjectionIndex(queryExpression, projectionBindingExpression);
var valueBuffer = queryExpression.CurrentParameter;

return Expression.Call(
EntityMaterializerSource.TryReadValueMethod.MakeGenericMethod(projectionBindingExpression.Type),
valueBuffer,
Expression.Constant(projectionIndex),
Expression.Constant(InferPropertyFromInner(queryExpression.Projection[projectionIndex]), typeof(IPropertyBase)));
return valueBuffer.CreateValueBufferReadValueExpression(
projectionBindingExpression.Type,
projectionIndex,
InferPropertyFromInner(queryExpression.Projection[projectionIndex]));
}

return base.VisitExtension(extensionExpression);
Expand All @@ -102,7 +102,7 @@ private IPropertyBase InferPropertyFromInner(Expression expression)
{
if (expression is MethodCallExpression methodCallExpression
&& methodCallExpression.Method.IsGenericMethod
&& methodCallExpression.Method.GetGenericMethodDefinition() == EntityMaterializerSource.TryReadValueMethod)
&& methodCallExpression.Method.GetGenericMethodDefinition() == ExpressionExtensions.ValueBufferTryReadValueMethod)
{
return (IPropertyBase)((ConstantExpression)methodCallExpression.Arguments[2]).Value;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
Check.NotNull(methodCallExpression, nameof(methodCallExpression));

if (methodCallExpression.Method.IsGenericMethod
&& methodCallExpression.Method.GetGenericMethodDefinition() == EntityMaterializerSource.TryReadValueMethod)
&& methodCallExpression.Method.GetGenericMethodDefinition() == Infrastructure.ExpressionExtensions.ValueBufferTryReadValueMethod)
{
var property = (IProperty)((ConstantExpression)methodCallExpression.Arguments[2]).Value;
var propertyProjectionMap = methodCallExpression.Arguments[0] is ProjectionBindingExpression projectionBindingExpression
Expand Down
45 changes: 45 additions & 0 deletions src/EFCore/Infrastructure/ExpressionExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@
using System.Linq;
using System.Linq.Expressions;
using System.Reflection;
using System.Runtime.CompilerServices;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.EntityFrameworkCore.Internal;
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Query;
using Microsoft.EntityFrameworkCore.Storage;
using Microsoft.EntityFrameworkCore.Utilities;

namespace Microsoft.EntityFrameworkCore.Infrastructure
Expand Down Expand Up @@ -225,5 +227,48 @@ var propertyPaths

return propertyPaths;
}

/// <summary>
/// <para>
/// Creates an <see cref="Expression" /> tree representing reading a value from a <see cref="ValueBuffer" />
/// </para>
/// <para>
/// This method is typically used by database providers (and other extensions). It is generally
/// not used in application code.
/// </para>
/// </summary>
/// <param name="valueBuffer"> The expression that exposes the <see cref="ValueBuffer" />. </param>
/// <param name="type"> The type to read. </param>
/// <param name="index"> The index in the buffer to read from. </param>
/// <param name="property"> The IPropertyBase being read if any. </param>
/// <returns> An expression to read the value. </returns>
public static Expression CreateValueBufferReadValueExpression(
[NotNull] this Expression valueBuffer,
[NotNull] Type type,
int index,
[CanBeNull] IPropertyBase property)
=> Expression.Call(
ValueBufferTryReadValueMethod.MakeGenericMethod(type),
valueBuffer,
Expression.Constant(index),
Expression.Constant(property, typeof(IPropertyBase)));

/// <summary>
/// <para>
/// MethodInfo which is used to generate an <see cref="Expression" /> tree representing reading a value from a <see cref="ValueBuffer" />
/// </para>
/// <para>
/// This method is typically used by database providers (and other extensions). It is generally
/// not used in application code.
/// </para>
/// </summary>
public static readonly MethodInfo ValueBufferTryReadValueMethod
= typeof(ExpressionExtensions).GetTypeInfo()
.GetDeclaredMethod(nameof(ValueBufferTryReadValue));

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static TValue ValueBufferTryReadValue<TValue>(
in ValueBuffer valueBuffer, int index, IPropertyBase property)
=> valueBuffer[index] is TValue value ? value : default;
}
}
8 changes: 3 additions & 5 deletions src/EFCore/Metadata/PropertyParameterBinding.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

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

Expand Down Expand Up @@ -33,11 +34,8 @@ public override Expression BindToParameter(ParameterBindingInfo bindingInfo)
{
var property = ConsumedProperties[0];

return Expression.Call(
EntityMaterializerSource.TryReadValueMethod.MakeGenericMethod(property.ClrType),
Expression.Call(bindingInfo.MaterializationContextExpression, MaterializationContext.GetValueBufferMethod),
Expression.Constant(bindingInfo.GetValueBufferIndex(property)),
Expression.Constant(property, typeof(IPropertyBase)));
return Expression.Call(bindingInfo.MaterializationContextExpression, MaterializationContext.GetValueBufferMethod)
.CreateValueBufferReadValueExpression(property.ClrType, bindingInfo.GetValueBufferIndex(property), property);
}
}
}
23 changes: 1 addition & 22 deletions src/EFCore/Query/EntityMaterializerSource.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,26 +34,6 @@ public EntityMaterializerSource([NotNull] EntityMaterializerSourceDependencies d
{
}

public virtual Expression CreateReadValueExpression(
Expression valueBufferExpression,
Type type,
int index,
IPropertyBase property)
=> Expression.Call(
TryReadValueMethod.MakeGenericMethod(type),
valueBufferExpression,
Expression.Constant(index),
Expression.Constant(property, typeof(IPropertyBase)));

public static readonly MethodInfo TryReadValueMethod
= typeof(EntityMaterializerSource).GetTypeInfo()
.GetDeclaredMethod(nameof(TryReadValue));

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static TValue TryReadValue<TValue>(
in ValueBuffer valueBuffer, int index, IPropertyBase property)
=> valueBuffer[index] is TValue value ? value : default;

public virtual Expression CreateMaterializeExpression(
IEntityType entityType,
string entityInstanceName,
Expand Down Expand Up @@ -127,8 +107,7 @@ var blockExpressions
var readValueExpression
= property is IServiceProperty serviceProperty
? serviceProperty.GetParameterBinding().BindToParameter(bindingInfo)
: CreateReadValueExpression(
valueBufferExpression,
: valueBufferExpression.CreateValueBufferReadValueExpression(
memberInfo.GetMemberType(),
property.GetIndex(),
property);
Expand Down
20 changes: 0 additions & 20 deletions src/EFCore/Query/IEntityMaterializerSource.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,26 +27,6 @@ namespace Microsoft.EntityFrameworkCore.Query
/// </summary>
public interface IEntityMaterializerSource
{
/// <summary>
/// <para>
/// Creates an <see cref="Expression" /> tree representing reading a value from a <see cref="ValueBuffer" />
/// </para>
/// <para>
/// This method is typically used by database providers (and other extensions). It is generally
/// not used in application code.
/// </para>
/// </summary>
/// <param name="valueBuffer"> The expression that exposes the <see cref="ValueBuffer" />. </param>
/// <param name="type"> The type to read. </param>
/// <param name="index"> The index in the buffer to read from. </param>
/// <param name="property"> The IPropertyBase being read if any. </param>
/// <returns> An expression to read the value. </returns>
Expression CreateReadValueExpression(
[NotNull] Expression valueBuffer,
[NotNull] Type type,
int index,
[CanBeNull] IPropertyBase property);

/// <summary>
/// <para>
/// Creates an <see cref="Expression" /> tree representing creating an entity instance.
Expand Down
19 changes: 6 additions & 13 deletions src/EFCore/Query/ShapedQueryCompilingExpressionVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.ChangeTracking.Internal;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.Internal;
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Metadata.Internal;
Expand Down Expand Up @@ -372,8 +373,7 @@ private Expression ProcessEntityShaper(EntityShaperExpression entityShaperExpres
typeof(object),
primaryKey.Properties
.Select(
p => _entityMaterializerSource.CreateReadValueExpression(
valueBufferExpression,
p => valueBufferExpression.CreateValueBufferReadValueExpression(
typeof(object),
p.GetIndex(),
p))),
Expand Down Expand Up @@ -407,8 +407,7 @@ private Expression ProcessEntityShaper(EntityShaperExpression entityShaperExpres
expressions.Add(Expression.IfThen(
primaryKey.Properties.Select(
p => Expression.NotEqual(
_entityMaterializerSource.CreateReadValueExpression(
valueBufferExpression, typeof(object), p.GetIndex(), p),
valueBufferExpression.CreateValueBufferReadValueExpression(typeof(object), p.GetIndex(), p),
Expression.Constant(null)))
.Aggregate((a, b) => Expression.AndAlso(a, b)),
MaterializeEntity(
Expand All @@ -424,8 +423,7 @@ private Expression ProcessEntityShaper(EntityShaperExpression entityShaperExpres
.Select(
p =>
Expression.NotEqual(
_entityMaterializerSource.CreateReadValueExpression(
valueBufferExpression,
valueBufferExpression.CreateValueBufferReadValueExpression(
typeof(object),
p.GetIndex(),
p),
Expand Down Expand Up @@ -486,8 +484,7 @@ private Expression MaterializeEntity(
expressions.Add(
Expression.Assign(
discriminatorValueVariable,
_entityMaterializerSource.CreateReadValueExpression(
valueBufferExpression,
valueBufferExpression.CreateValueBufferReadValueExpression(
discriminatorProperty.ClrType,
discriminatorProperty.GetIndex(),
discriminatorProperty)));
Expand Down Expand Up @@ -578,11 +575,7 @@ private BlockExpression CreateFullMaterializeExpression(
Expression.NewArrayInit(
typeof(object),
shadowProperties.Select(
p => _entityMaterializerSource.CreateReadValueExpression(
valueBufferExpression,
typeof(object),
p.GetIndex(),
p))))));
p => valueBufferExpression.CreateValueBufferReadValueExpression(typeof(object), p.GetIndex(), p))))));
}

materializer = materializer.Type == returnType
Expand Down

0 comments on commit 5fd8577

Please sign in to comment.