From 5fd8577562cd1bed5b6a76e3637b17d561aaa249 Mon Sep 17 00:00:00 2001 From: Smit Patel Date: Tue, 17 Mar 2020 16:04:01 -0700 Subject: [PATCH 1/3] Query: Make CreateReadValueExpression an extension method 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. --- ...jectionBindingRemovingExpressionVisitor.cs | 2 +- ...yExpressionTranslatingExpressionVisitor.cs | 3 +- .../Query/Internal/InMemoryQueryExpression.cs | 21 +++------ ...jectionBindingRemovingExpressionVisitor.cs | 14 +++--- ...jectionBindingRemovingExpressionVisitor.cs | 2 +- .../Infrastructure/ExpressionExtensions.cs | 45 +++++++++++++++++++ .../Metadata/PropertyParameterBinding.cs | 8 ++-- src/EFCore/Query/EntityMaterializerSource.cs | 23 +--------- src/EFCore/Query/IEntityMaterializerSource.cs | 20 --------- .../ShapedQueryCompilingExpressionVisitor.cs | 19 +++----- 10 files changed, 73 insertions(+), 84 deletions(-) diff --git a/src/EFCore.Cosmos/Query/Internal/CosmosShapedQueryCompilingExpressionVisitor.CosmosProjectionBindingRemovingExpressionVisitor.cs b/src/EFCore.Cosmos/Query/Internal/CosmosShapedQueryCompilingExpressionVisitor.CosmosProjectionBindingRemovingExpressionVisitor.cs index 3866035b7cc..2bf83954bd6 100644 --- a/src/EFCore.Cosmos/Query/Internal/CosmosShapedQueryCompilingExpressionVisitor.CosmosProjectionBindingRemovingExpressionVisitor.cs +++ b/src/EFCore.Cosmos/Query/Internal/CosmosShapedQueryCompilingExpressionVisitor.CosmosProjectionBindingRemovingExpressionVisitor.cs @@ -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; diff --git a/src/EFCore.InMemory/Query/Internal/InMemoryExpressionTranslatingExpressionVisitor.cs b/src/EFCore.InMemory/Query/Internal/InMemoryExpressionTranslatingExpressionVisitor.cs index edaee096156..b1a31ce5550 100644 --- a/src/EFCore.InMemory/Query/Internal/InMemoryExpressionTranslatingExpressionVisitor.cs +++ b/src/EFCore.InMemory/Query/Internal/InMemoryExpressionTranslatingExpressionVisitor.cs @@ -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 { @@ -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; } diff --git a/src/EFCore.InMemory/Query/Internal/InMemoryQueryExpression.cs b/src/EFCore.InMemory/Query/Internal/InMemoryQueryExpression.cs index 6bed3a3287b..f4efd3fedc4 100644 --- a/src/EFCore.InMemory/Query/Internal/InMemoryQueryExpression.cs +++ b/src/EFCore.InMemory/Query/Internal/InMemoryQueryExpression.cs @@ -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 { @@ -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; @@ -440,24 +442,15 @@ private Expression GetGroupingKey(Expression key, List 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, @@ -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); } diff --git a/src/EFCore.InMemory/Query/Internal/InMemoryShapedQueryCompilingExpressionVisitor.InMemoryProjectionBindingRemovingExpressionVisitor.cs b/src/EFCore.InMemory/Query/Internal/InMemoryShapedQueryCompilingExpressionVisitor.InMemoryProjectionBindingRemovingExpressionVisitor.cs index 6934d61f31d..677f64cbe07 100644 --- a/src/EFCore.InMemory/Query/Internal/InMemoryShapedQueryCompilingExpressionVisitor.InMemoryProjectionBindingRemovingExpressionVisitor.cs +++ b/src/EFCore.InMemory/Query/Internal/InMemoryShapedQueryCompilingExpressionVisitor.InMemoryProjectionBindingRemovingExpressionVisitor.cs @@ -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 { @@ -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) = @@ -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); @@ -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; } diff --git a/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.RelationalProjectionBindingRemovingExpressionVisitor.cs b/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.RelationalProjectionBindingRemovingExpressionVisitor.cs index f8735eba83f..d159e00482c 100644 --- a/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.RelationalProjectionBindingRemovingExpressionVisitor.cs +++ b/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.RelationalProjectionBindingRemovingExpressionVisitor.cs @@ -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 diff --git a/src/EFCore/Infrastructure/ExpressionExtensions.cs b/src/EFCore/Infrastructure/ExpressionExtensions.cs index 48d5a8b970e..e7f6c80df1c 100644 --- a/src/EFCore/Infrastructure/ExpressionExtensions.cs +++ b/src/EFCore/Infrastructure/ExpressionExtensions.cs @@ -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 @@ -225,5 +227,48 @@ var propertyPaths return propertyPaths; } + + /// + /// + /// Creates an tree representing reading a value from a + /// + /// + /// This method is typically used by database providers (and other extensions). It is generally + /// not used in application code. + /// + /// + /// The expression that exposes the . + /// The type to read. + /// The index in the buffer to read from. + /// The IPropertyBase being read if any. + /// An expression to read the value. + 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))); + + /// + /// + /// MethodInfo which is used to generate an tree representing reading a value from a + /// + /// + /// This method is typically used by database providers (and other extensions). It is generally + /// not used in application code. + /// + /// + public static readonly MethodInfo ValueBufferTryReadValueMethod + = typeof(ExpressionExtensions).GetTypeInfo() + .GetDeclaredMethod(nameof(ValueBufferTryReadValue)); + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static TValue ValueBufferTryReadValue( + in ValueBuffer valueBuffer, int index, IPropertyBase property) + => valueBuffer[index] is TValue value ? value : default; } } diff --git a/src/EFCore/Metadata/PropertyParameterBinding.cs b/src/EFCore/Metadata/PropertyParameterBinding.cs index 3d70dde51ad..73d02ca15fb 100644 --- a/src/EFCore/Metadata/PropertyParameterBinding.cs +++ b/src/EFCore/Metadata/PropertyParameterBinding.cs @@ -3,6 +3,7 @@ using System.Linq.Expressions; using JetBrains.Annotations; +using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.EntityFrameworkCore.Query; using Microsoft.EntityFrameworkCore.Storage; @@ -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); } } } diff --git a/src/EFCore/Query/EntityMaterializerSource.cs b/src/EFCore/Query/EntityMaterializerSource.cs index 84b25be969c..fd6b3896a42 100644 --- a/src/EFCore/Query/EntityMaterializerSource.cs +++ b/src/EFCore/Query/EntityMaterializerSource.cs @@ -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( - in ValueBuffer valueBuffer, int index, IPropertyBase property) - => valueBuffer[index] is TValue value ? value : default; - public virtual Expression CreateMaterializeExpression( IEntityType entityType, string entityInstanceName, @@ -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); diff --git a/src/EFCore/Query/IEntityMaterializerSource.cs b/src/EFCore/Query/IEntityMaterializerSource.cs index 3a2a52eb4cd..1dd57bd9fb0 100644 --- a/src/EFCore/Query/IEntityMaterializerSource.cs +++ b/src/EFCore/Query/IEntityMaterializerSource.cs @@ -27,26 +27,6 @@ namespace Microsoft.EntityFrameworkCore.Query /// public interface IEntityMaterializerSource { - /// - /// - /// Creates an tree representing reading a value from a - /// - /// - /// This method is typically used by database providers (and other extensions). It is generally - /// not used in application code. - /// - /// - /// The expression that exposes the . - /// The type to read. - /// The index in the buffer to read from. - /// The IPropertyBase being read if any. - /// An expression to read the value. - Expression CreateReadValueExpression( - [NotNull] Expression valueBuffer, - [NotNull] Type type, - int index, - [CanBeNull] IPropertyBase property); - /// /// /// Creates an tree representing creating an entity instance. diff --git a/src/EFCore/Query/ShapedQueryCompilingExpressionVisitor.cs b/src/EFCore/Query/ShapedQueryCompilingExpressionVisitor.cs index d55ffd4df5d..ce00d7250c8 100644 --- a/src/EFCore/Query/ShapedQueryCompilingExpressionVisitor.cs +++ b/src/EFCore/Query/ShapedQueryCompilingExpressionVisitor.cs @@ -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; @@ -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))), @@ -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( @@ -424,8 +423,7 @@ private Expression ProcessEntityShaper(EntityShaperExpression entityShaperExpres .Select( p => Expression.NotEqual( - _entityMaterializerSource.CreateReadValueExpression( - valueBufferExpression, + valueBufferExpression.CreateValueBufferReadValueExpression( typeof(object), p.GetIndex(), p), @@ -486,8 +484,7 @@ private Expression MaterializeEntity( expressions.Add( Expression.Assign( discriminatorValueVariable, - _entityMaterializerSource.CreateReadValueExpression( - valueBufferExpression, + valueBufferExpression.CreateValueBufferReadValueExpression( discriminatorProperty.ClrType, discriminatorProperty.GetIndex(), discriminatorProperty))); @@ -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 From cbe4cd524b913d0f07030721ad100c9c18203a1b Mon Sep 17 00:00:00 2001 From: Smit Patel Date: Tue, 17 Mar 2020 17:28:15 -0700 Subject: [PATCH 2/3] Query: Create Discriminator Condition on EntityShaper and use it in shaper Part of #18923 Part of #10140 --- src/EFCore/Query/EntityShaperExpression.cs | 84 ++++++++++++++++++- .../ShapedQueryCompilingExpressionVisitor.cs | 80 +++++++----------- 2 files changed, 114 insertions(+), 50 deletions(-) diff --git a/src/EFCore/Query/EntityShaperExpression.cs b/src/EFCore/Query/EntityShaperExpression.cs index c68aa5b45b6..2b45efec8a9 100644 --- a/src/EFCore/Query/EntityShaperExpression.cs +++ b/src/EFCore/Query/EntityShaperExpression.cs @@ -2,31 +2,111 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Generic; +using System.Linq; using System.Linq.Expressions; +using System.Reflection; using JetBrains.Annotations; +using Microsoft.EntityFrameworkCore.Diagnostics; +using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.EntityFrameworkCore.Metadata; +using Microsoft.EntityFrameworkCore.Metadata.Internal; +using Microsoft.EntityFrameworkCore.Storage; using Microsoft.EntityFrameworkCore.Utilities; namespace Microsoft.EntityFrameworkCore.Query { public class EntityShaperExpression : Expression, IPrintableExpression { + private static readonly MethodInfo _createUnableToDiscriminateException + = typeof(EntityShaperExpression).GetTypeInfo() + .GetDeclaredMethod(nameof(CreateUnableToDiscriminateException)); + + [UsedImplicitly] + private static Exception CreateUnableToDiscriminateException(IEntityType entityType, object discriminator) + => new InvalidOperationException(CoreStrings.UnableToDiscriminate(entityType.DisplayName(), discriminator?.ToString())); + public EntityShaperExpression( [NotNull] IEntityType entityType, [NotNull] Expression valueBufferExpression, bool nullable) + : this(entityType, valueBufferExpression, nullable, null) + { + } + + protected EntityShaperExpression( + [NotNull] IEntityType entityType, + [NotNull] Expression valueBufferExpression, + bool nullable, + [CanBeNull] LambdaExpression discriminatorCondition) { Check.NotNull(entityType, nameof(entityType)); Check.NotNull(valueBufferExpression, nameof(valueBufferExpression)); + if (discriminatorCondition == null) + { + // Generate condition to discriminator if TPH + discriminatorCondition = GenerateDiscriminatorCondition(entityType); + + } + else if (discriminatorCondition.Parameters.Count != 1 + || discriminatorCondition.Parameters[0].Type != typeof(ValueBuffer) + || discriminatorCondition.ReturnType != typeof(IEntityType)) + { + throw new InvalidOperationException( + "Discriminator condition must be lambda expression of type Func."); + } + EntityType = entityType; ValueBufferExpression = valueBufferExpression; IsNullable = nullable; + DiscriminatorCondition = discriminatorCondition; + } + + private LambdaExpression GenerateDiscriminatorCondition(IEntityType entityType) + { + var valueBufferParameter = Parameter(typeof(ValueBuffer)); + Expression body; + var concreteEntityTypes = entityType.GetConcreteDerivedTypesInclusive().ToList(); + var discriminatorProperty = entityType.GetDiscriminatorProperty(); + if (discriminatorProperty != null) + { + var discriminatorValueVariable = Variable(discriminatorProperty.ClrType, "discriminator"); + var expressions = new List + { + Assign( + discriminatorValueVariable, + valueBufferParameter.CreateValueBufferReadValueExpression( + discriminatorProperty.ClrType, discriminatorProperty.GetIndex(), discriminatorProperty)) + }; + + var switchCases = new SwitchCase[concreteEntityTypes.Count]; + for (var i = 0; i < concreteEntityTypes.Count; i++) + { + var discriminatorValue = Constant(concreteEntityTypes[i].GetDiscriminatorValue(), discriminatorProperty.ClrType); + switchCases[i] = SwitchCase(Constant(concreteEntityTypes[i], typeof(IEntityType)), discriminatorValue); + } + + var exception = Block( + Throw(Call( + _createUnableToDiscriminateException, Constant(entityType), Convert(discriminatorValueVariable, typeof(object)))), + Constant(null, typeof(IEntityType))); + + expressions.Add(Switch(discriminatorValueVariable, exception, switchCases)); + body = Block(new[] { discriminatorValueVariable }, expressions); + } + else + { + body = Constant(concreteEntityTypes.Count == 1 ? concreteEntityTypes[0] : entityType, typeof(IEntityType)); + } + + return Lambda(body, valueBufferParameter); } public virtual IEntityType EntityType { get; } public virtual Expression ValueBufferExpression { get; } public virtual bool IsNullable { get; } + public virtual LambdaExpression DiscriminatorCondition { get; } protected override Expression VisitChildren(ExpressionVisitor visitor) { @@ -48,7 +128,7 @@ public virtual EntityShaperExpression WithEntityType([NotNull] IEntityType entit public virtual EntityShaperExpression MarkAsNullable() => !IsNullable - ? new EntityShaperExpression(EntityType, ValueBufferExpression, true) + ? new EntityShaperExpression(EntityType, ValueBufferExpression, true, DiscriminatorCondition) : this; public virtual EntityShaperExpression Update([NotNull] Expression valueBufferExpression) @@ -56,7 +136,7 @@ public virtual EntityShaperExpression Update([NotNull] Expression valueBufferExp Check.NotNull(valueBufferExpression, nameof(valueBufferExpression)); return valueBufferExpression != ValueBufferExpression - ? new EntityShaperExpression(EntityType, valueBufferExpression, IsNullable) + ? new EntityShaperExpression(EntityType, valueBufferExpression, IsNullable, DiscriminatorCondition) : this; } diff --git a/src/EFCore/Query/ShapedQueryCompilingExpressionVisitor.cs b/src/EFCore/Query/ShapedQueryCompilingExpressionVisitor.cs index ce00d7250c8..d9a5eb5e970 100644 --- a/src/EFCore/Query/ShapedQueryCompilingExpressionVisitor.cs +++ b/src/EFCore/Query/ShapedQueryCompilingExpressionVisitor.cs @@ -396,14 +396,13 @@ private Expression ProcessEntityShaper(EntityShaperExpression entityShaperExpres Expression.MakeMemberAccess(entryVariable, _entityMemberInfo), entityType.ClrType))), MaterializeEntity( - entityType, materializationContextVariable, concreteEntityTypeVariable, instanceVariable, + entityShaperExpression, materializationContextVariable, concreteEntityTypeVariable, instanceVariable, entryVariable)))); } else { if (primaryKey != null) { - expressions.Add(Expression.IfThen( primaryKey.Properties.Select( p => Expression.NotEqual( @@ -411,7 +410,7 @@ private Expression ProcessEntityShaper(EntityShaperExpression entityShaperExpres Expression.Constant(null))) .Aggregate((a, b) => Expression.AndAlso(a, b)), MaterializeEntity( - entityType, materializationContextVariable, concreteEntityTypeVariable, instanceVariable, null))); + entityShaperExpression, materializationContextVariable, concreteEntityTypeVariable, instanceVariable, null))); } else { @@ -430,13 +429,13 @@ private Expression ProcessEntityShaper(EntityShaperExpression entityShaperExpres Expression.Constant(null))) .Aggregate((a, b) => Expression.OrElse(a, b)), MaterializeEntity( - entityType, materializationContextVariable, concreteEntityTypeVariable, instanceVariable, null))); + entityShaperExpression, materializationContextVariable, concreteEntityTypeVariable, instanceVariable, null))); } else { expressions.Add( MaterializeEntity( - entityType, materializationContextVariable, concreteEntityTypeVariable, instanceVariable, null)); + entityShaperExpression, materializationContextVariable, concreteEntityTypeVariable, instanceVariable, null)); } } } @@ -446,12 +445,14 @@ private Expression ProcessEntityShaper(EntityShaperExpression entityShaperExpres } private Expression MaterializeEntity( - IEntityType entityType, + EntityShaperExpression entityShaperExpression, ParameterExpression materializationContextVariable, ParameterExpression concreteEntityTypeVariable, ParameterExpression instanceVariable, ParameterExpression entryVariable) { + var entityType = entityShaperExpression.EntityType; + var expressions = new List(); var variables = new List(); @@ -468,47 +469,35 @@ private Expression MaterializeEntity( Expression materializationExpression; var valueBufferExpression = Expression.Call(materializationContextVariable, MaterializationContext.GetValueBufferMethod); var expressionContext = (returnType, materializationContextVariable, concreteEntityTypeVariable, shadowValuesVariable); + expressions.Add( + Expression.Assign(concreteEntityTypeVariable, + ReplacingExpressionVisitor.Replace( + entityShaperExpression.DiscriminatorCondition.Parameters[0], + valueBufferExpression, + entityShaperExpression.DiscriminatorCondition.Body))); + var concreteEntityTypes = entityType.GetConcreteDerivedTypesInclusive().ToList(); - var firstEntityType = concreteEntityTypes[0]; - if (concreteEntityTypes.Count == 1) + var discriminatorProperty = entityType.GetDiscriminatorProperty(); + if (discriminatorProperty != null) { - materializationExpression = CreateFullMaterializeExpression(firstEntityType, expressionContext); + var switchCases = new SwitchCase[concreteEntityTypes.Count]; + for (var i = 0; i < concreteEntityTypes.Count; i++) + { + switchCases[i] = Expression.SwitchCase( + CreateFullMaterializeExpression(concreteEntityTypes[i], expressionContext), + Expression.Constant(concreteEntityTypes[i], typeof(IEntityType))); + } + + materializationExpression = Expression.Switch( + concreteEntityTypeVariable, + Expression.Constant(null, returnType), + switchCases); } else { - var discriminatorProperty = firstEntityType.GetDiscriminatorProperty(); - var discriminatorValueVariable = Expression.Variable( - discriminatorProperty.ClrType, "discriminator" + _currentEntityIndex); - variables.Add(discriminatorValueVariable); - - expressions.Add( - Expression.Assign( - discriminatorValueVariable, - valueBufferExpression.CreateValueBufferReadValueExpression( - discriminatorProperty.ClrType, - discriminatorProperty.GetIndex(), - discriminatorProperty))); - - materializationExpression = Expression.Block( - Expression.Throw( - Expression.Call( - _createUnableToDiscriminateException, - Expression.Constant(entityType), - Expression.Convert(discriminatorValueVariable, typeof(object)))), - Expression.Constant(null, returnType)); - - foreach (var concreteEntityType in concreteEntityTypes) - { - var discriminatorValue - = Expression.Constant( - concreteEntityType.GetDiscriminatorValue(), - discriminatorProperty.ClrType); - - materializationExpression = Expression.Condition( - Expression.Equal(discriminatorValueVariable, discriminatorValue), - CreateFullMaterializeExpression(concreteEntityType, expressionContext), - materializationExpression); - } + materializationExpression = CreateFullMaterializeExpression( + concreteEntityTypes.Count == 1 ? concreteEntityTypes[0] : entityType, + expressionContext); } expressions.Add(Expression.Assign(instanceVariable, materializationExpression)); @@ -551,12 +540,7 @@ private BlockExpression CreateFullMaterializeExpression( concreteEntityTypeVariable, shadowValuesVariable) = materializeExpressionContext; - var blockExpressions = new List(3) - { - Expression.Assign( - concreteEntityTypeVariable, - Expression.Constant(concreteEntityType)) - }; + var blockExpressions = new List(2); var materializer = _entityMaterializerSource .CreateMaterializeExpression(concreteEntityType, "instance", materializationContextVariable); From 8e808f47d4c1ea1ce8eaa4051285fae4c2b76983 Mon Sep 17 00:00:00 2001 From: Smit Patel Date: Tue, 17 Mar 2020 18:30:09 -0700 Subject: [PATCH 3/3] Query: Make keyless entity type materialization checks part of DiscriminatorCondition If DiscriminatorCondition returns null for IEntityType then return null instance. Part of #18923 --- src/EFCore/Query/EntityShaperExpression.cs | 30 +++++++--- .../ShapedQueryCompilingExpressionVisitor.cs | 59 ++++++------------- 2 files changed, 39 insertions(+), 50 deletions(-) diff --git a/src/EFCore/Query/EntityShaperExpression.cs b/src/EFCore/Query/EntityShaperExpression.cs index 2b45efec8a9..d0e34bedd61 100644 --- a/src/EFCore/Query/EntityShaperExpression.cs +++ b/src/EFCore/Query/EntityShaperExpression.cs @@ -45,9 +45,7 @@ protected EntityShaperExpression( if (discriminatorCondition == null) { - // Generate condition to discriminator if TPH - discriminatorCondition = GenerateDiscriminatorCondition(entityType); - + discriminatorCondition = GenerateDiscriminatorCondition(entityType, nullable); } else if (discriminatorCondition.Parameters.Count != 1 || discriminatorCondition.Parameters[0].Type != typeof(ValueBuffer) @@ -63,11 +61,11 @@ protected EntityShaperExpression( DiscriminatorCondition = discriminatorCondition; } - private LambdaExpression GenerateDiscriminatorCondition(IEntityType entityType) + private LambdaExpression GenerateDiscriminatorCondition(IEntityType entityType, bool nullable) { var valueBufferParameter = Parameter(typeof(ValueBuffer)); Expression body; - var concreteEntityTypes = entityType.GetConcreteDerivedTypesInclusive().ToList(); + var concreteEntityTypes = entityType.GetConcreteDerivedTypesInclusive().ToArray(); var discriminatorProperty = entityType.GetDiscriminatorProperty(); if (discriminatorProperty != null) { @@ -80,8 +78,8 @@ private LambdaExpression GenerateDiscriminatorCondition(IEntityType entityType) discriminatorProperty.ClrType, discriminatorProperty.GetIndex(), discriminatorProperty)) }; - var switchCases = new SwitchCase[concreteEntityTypes.Count]; - for (var i = 0; i < concreteEntityTypes.Count; i++) + var switchCases = new SwitchCase[concreteEntityTypes.Length]; + for (var i = 0; i < concreteEntityTypes.Length; i++) { var discriminatorValue = Constant(concreteEntityTypes[i].GetDiscriminatorValue(), discriminatorProperty.ClrType); switchCases[i] = SwitchCase(Constant(concreteEntityTypes[i], typeof(IEntityType)), discriminatorValue); @@ -97,7 +95,20 @@ private LambdaExpression GenerateDiscriminatorCondition(IEntityType entityType) } else { - body = Constant(concreteEntityTypes.Count == 1 ? concreteEntityTypes[0] : entityType, typeof(IEntityType)); + body = Constant(concreteEntityTypes.Length == 1 ? concreteEntityTypes[0] : entityType, typeof(IEntityType)); + } + + if (entityType.FindPrimaryKey() == null + && nullable) + { + body = Condition( + entityType.GetProperties() + .Select(p => NotEqual( + valueBufferParameter.CreateValueBufferReadValueExpression(typeof(object), p.GetIndex(), p), + Constant(null))) + .Aggregate((a, b) => OrElse(a, b)), + body, + Default(typeof(IEntityType))); } return Lambda(body, valueBufferParameter); @@ -128,7 +139,8 @@ public virtual EntityShaperExpression WithEntityType([NotNull] IEntityType entit public virtual EntityShaperExpression MarkAsNullable() => !IsNullable - ? new EntityShaperExpression(EntityType, ValueBufferExpression, true, DiscriminatorCondition) + // Marking nullable requires recomputation of Discriminator condition + ? new EntityShaperExpression(EntityType, ValueBufferExpression, true) : this; public virtual EntityShaperExpression Update([NotNull] Expression valueBufferExpression) diff --git a/src/EFCore/Query/ShapedQueryCompilingExpressionVisitor.cs b/src/EFCore/Query/ShapedQueryCompilingExpressionVisitor.cs index d9a5eb5e970..2a6e633f5f7 100644 --- a/src/EFCore/Query/ShapedQueryCompilingExpressionVisitor.cs +++ b/src/EFCore/Query/ShapedQueryCompilingExpressionVisitor.cs @@ -414,29 +414,9 @@ private Expression ProcessEntityShaper(EntityShaperExpression entityShaperExpres } else { - if (entityShaperExpression.IsNullable) - { - expressions.Add( - Expression.IfThen( - entityType.GetProperties() - .Select( - p => - Expression.NotEqual( - valueBufferExpression.CreateValueBufferReadValueExpression( - typeof(object), - p.GetIndex(), - p), - Expression.Constant(null))) - .Aggregate((a, b) => Expression.OrElse(a, b)), - MaterializeEntity( - entityShaperExpression, materializationContextVariable, concreteEntityTypeVariable, instanceVariable, null))); - } - else - { - expressions.Add( - MaterializeEntity( - entityShaperExpression, materializationContextVariable, concreteEntityTypeVariable, instanceVariable, null)); - } + expressions.Add( + MaterializeEntity( + entityShaperExpression, materializationContextVariable, concreteEntityTypeVariable, instanceVariable, null)); } } @@ -476,30 +456,27 @@ private Expression MaterializeEntity( valueBufferExpression, entityShaperExpression.DiscriminatorCondition.Body))); - var concreteEntityTypes = entityType.GetConcreteDerivedTypesInclusive().ToList(); + var concreteEntityTypes = entityType.GetConcreteDerivedTypesInclusive().ToArray(); var discriminatorProperty = entityType.GetDiscriminatorProperty(); - if (discriminatorProperty != null) + if (discriminatorProperty == null + && concreteEntityTypes.Length > 1) { - var switchCases = new SwitchCase[concreteEntityTypes.Count]; - for (var i = 0; i < concreteEntityTypes.Count; i++) - { - switchCases[i] = Expression.SwitchCase( - CreateFullMaterializeExpression(concreteEntityTypes[i], expressionContext), - Expression.Constant(concreteEntityTypes[i], typeof(IEntityType))); - } - - materializationExpression = Expression.Switch( - concreteEntityTypeVariable, - Expression.Constant(null, returnType), - switchCases); + concreteEntityTypes = new [] { entityType }; } - else + + var switchCases = new SwitchCase[concreteEntityTypes.Length]; + for (var i = 0; i < concreteEntityTypes.Length; i++) { - materializationExpression = CreateFullMaterializeExpression( - concreteEntityTypes.Count == 1 ? concreteEntityTypes[0] : entityType, - expressionContext); + switchCases[i] = Expression.SwitchCase( + CreateFullMaterializeExpression(concreteEntityTypes[i], expressionContext), + Expression.Constant(concreteEntityTypes[i], typeof(IEntityType))); } + materializationExpression = Expression.Switch( + concreteEntityTypeVariable, + Expression.Constant(null, returnType), + switchCases); + expressions.Add(Expression.Assign(instanceVariable, materializationExpression)); if (_trackQueryResults