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: Make CreateReadValueExpression an extension method #20321

Merged
merged 3 commits into from
Mar 18, 2020
Merged

Query: Make CreateReadValueExpression an extension method #20321

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5fd8577
Query: Make CreateReadValueExpression an extension method
smitpatel Mar 17, 2020
cbe4cd5
Query: Create Discriminator Condition on EntityShaper and use it in s…
smitpatel Mar 18, 2020
8e808f4
Query: Make keyless entity type materialization checks part of Discri…
smitpatel Mar 18, 2020
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
2 changes: 1 addition & 1 deletion ...ShapedQueryCompilingExpressionVisitor.CosmosProjectionBindingRemovingExpressionVisitor.cs
View file
Open in desktop
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
3 changes: 2 additions & 1 deletion src/EFCore.InMemory/Query/Internal/InMemoryExpressionTranslatingExpressionVisitor.cs
View file
Open in desktop
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
View file
Open in desktop
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
14 changes: 7 additions & 7 deletions ...apedQueryCompilingExpressionVisitor.InMemoryProjectionBindingRemovingExpressionVisitor.cs
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion ...edQueryCompilingExpressionVisitor.RelationalProjectionBindingRemovingExpressionVisitor.cs
View file
Open in desktop
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
View file
Open in desktop
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
Copy link
Member

@AndriySvyryd AndriySvyryd Mar 18, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wish this class would have been named something like InternalExpressionExtensions

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

me too. Such a clash between our own code. 😢

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically we could rename all internal extension method classes without it being a breaking change.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but .Infrastructure is not .Internal

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
View file
Open in desktop
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
View file
Open in desktop
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
Loading