Skip to content

Commit

Permalink
Query: Do not generate server side joins for optional dependents
Browse files Browse the repository at this point in the history
Fix involves:
- Adding additional materialization conditions for relational layer when table splitting is used. See #20332 for conditions
- Create EntityProjection using the same table when both types are sharing the table.
- Fix a bug in shaper when null entityType is returned from materialization condition, don't call StartTracking.
- Assign nullability to PK column correctly for optional dependents so we don't project it twice.
- Disabled Sql assertion in OwnedQuerySqlServerTest. Will enable after model update as described in #20336, #20343

Resolves #18299
Resolves #20338
Resolves #20332
  • Loading branch information
smitpatel committed Mar 19, 2020
1 parent 747195b commit 5fe3628
Show file tree
Hide file tree
Showing 13 changed files with 327 additions and 346 deletions.
164 changes: 164 additions & 0 deletions src/EFCore.Relational/Query/RelationalEntityShaperExpression.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,164 @@
// Copyright (c) .NET Foundation. All rights reserved.
// 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 RelationalEntityShaperExpression : EntityShaperExpression
{
private static readonly MethodInfo _createUnableToDiscriminateException
= typeof(RelationalEntityShaperExpression).GetTypeInfo()
.GetDeclaredMethod(nameof(CreateUnableToDiscriminateException));

[UsedImplicitly]
private static Exception CreateUnableToDiscriminateException(IEntityType entityType, object discriminator)
=> new InvalidOperationException(CoreStrings.UnableToDiscriminate(entityType.DisplayName(), discriminator?.ToString()));
public RelationalEntityShaperExpression([NotNull] IEntityType entityType, [NotNull] Expression valueBufferExpression, bool nullable)
: base(entityType, valueBufferExpression, nullable,
GenerateMaterializationCondition(Check.NotNull(entityType, nameof(entityType)), nullable))
{
}

public RelationalEntityShaperExpression(
[NotNull] IEntityType entityType,
[NotNull] Expression valueBufferExpression,
bool nullable,
[NotNull] LambdaExpression discriminatorCondition)
: base(entityType, valueBufferExpression, nullable,
discriminatorCondition ?? GenerateMaterializationCondition(Check.NotNull(entityType, nameof(entityType)), nullable))
{
}

private static LambdaExpression GenerateMaterializationCondition(IEntityType entityType, bool nullable)
{
var valueBufferParameter = Parameter(typeof(ValueBuffer));

var keyless = entityType.FindPrimaryKey() == null;
var optionalDependent = false;
if (!keyless)
{
var linkingFks = entityType.GetViewOrTableMappings().Single().Table.GetInternalForeignKeys(entityType);
if (linkingFks != null
&& linkingFks.Any())
{
optionalDependent = true;
}
}

Expression body;
var concreteEntityTypes = entityType.GetConcreteDerivedTypesInclusive().ToArray();
var discriminatorProperty = entityType.GetDiscriminatorProperty();
if (discriminatorProperty != null)
{
var discriminatorValueVariable = Variable(discriminatorProperty.ClrType, "discriminator");
var expressions = new List<Expression>
{
Assign(
discriminatorValueVariable,
valueBufferParameter.CreateValueBufferReadValueExpression(
discriminatorProperty.ClrType, discriminatorProperty.GetIndex(), discriminatorProperty))
};

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);
}

var defaultBlock = Block(
Throw(Call(
_createUnableToDiscriminateException, Constant(entityType), Convert(discriminatorValueVariable, typeof(object)))),
Default(typeof(IEntityType)));

expressions.Add(Switch(discriminatorValueVariable, defaultBlock, switchCases));
body = Block(new[] { discriminatorValueVariable }, expressions);
}
else
{
body = Constant(concreteEntityTypes.Length == 1 ? concreteEntityTypes[0] : entityType, typeof(IEntityType));
}

if (optionalDependent)
{
var requiredNonPkProperties = entityType.GetProperties().Where(p => !p.IsNullable && !p.IsPrimaryKey()).ToList();
if (requiredNonPkProperties.Count > 0)
{
body = Condition(
requiredNonPkProperties
.Select(p => NotEqual(
valueBufferParameter.CreateValueBufferReadValueExpression(typeof(object), p.GetIndex(), p),
Constant(null)))
.Aggregate((a, b) => AndAlso(a, b)),
body,
Default(typeof(IEntityType)));
}
else
{
var allNonPkProperties = entityType.GetProperties().Where(p => !p.IsPrimaryKey()).ToList();
if (allNonPkProperties.Count > 0)
{
body = Condition(
allNonPkProperties
.Select(p => NotEqual(
valueBufferParameter.CreateValueBufferReadValueExpression(typeof(object), p.GetIndex(), p),
Constant(null)))
.Aggregate((a, b) => OrElse(a, b)),
body,
Default(typeof(IEntityType)));
}
}
}
else if (keyless
&& 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);
}

public override EntityShaperExpression WithEntityType(IEntityType entityType)
{
Check.NotNull(entityType, nameof(entityType));

return entityType != EntityType
? new RelationalEntityShaperExpression(entityType, ValueBufferExpression, IsNullable)
: this;
}

public override EntityShaperExpression MarkAsNullable()
=> !IsNullable
// Marking nullable requires recomputation of Discriminator condition
? new RelationalEntityShaperExpression(EntityType, ValueBufferExpression, true)
: this;

public override EntityShaperExpression Update(Expression valueBufferExpression)
{
Check.NotNull(valueBufferExpression, nameof(valueBufferExpression));

return valueBufferExpression != ValueBufferExpression
? new RelationalEntityShaperExpression(EntityType, valueBufferExpression, IsNullable, DiscriminatorCondition)
: this;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ protected override ShapedQueryExpression CreateShapedQueryExpression(IEntityType
private static ShapedQueryExpression CreateShapedQueryExpression(IEntityType entityType, SelectExpression selectExpression)
=> new ShapedQueryExpression(
selectExpression,
new EntityShaperExpression(
new RelationalEntityShaperExpression(
entityType,
new ProjectionBindingExpression(
selectExpression,
Expand Down Expand Up @@ -1223,37 +1223,96 @@ private Expression TryExpand(Expression source, MemberIdentity member)
var innerShaper = entityProjectionExpression.BindNavigation(navigation);
if (innerShaper == null)
{
var innerSelectExpression = _sqlExpressionFactory.Select(targetEntityType);
var innerShapedQuery = CreateShapedQueryExpression(targetEntityType, innerSelectExpression);

var makeNullable = foreignKey.PrincipalKey.Properties
.Concat(foreignKey.Properties)
.Select(p => p.ClrType)
.Any(t => t.IsNullableType());

var outerKey = entityShaperExpression.CreateKeyAccessExpression(
navigation.IsOnDependent
? foreignKey.Properties
: foreignKey.PrincipalKey.Properties,
makeNullable);
var innerKey = innerShapedQuery.ShaperExpression.CreateKeyAccessExpression(
navigation.IsOnDependent
? foreignKey.PrincipalKey.Properties
: foreignKey.Properties,
makeNullable);
if (entityType.GetViewOrTableMappings().Single().Table
.GetReferencingInternalForeignKeys(foreignKey.PrincipalEntityType)?.Contains(foreignKey) == true)
{
// Since we are not going to update table or visit, we always generate propertyExpressions
// We just first column of PK to figure out the base table
var identifyingColumn = entityProjectionExpression.BindProperty(entityType.FindPrimaryKey().Properties.First());
var propertyExpressions = identifyingColumn.Table is TableExpression innerTable
? GetPropertyExpressionsFromTable(targetEntityType, innerTable, identifyingColumn.IsNullable)
// Pull columns out of inner subquery
: GetPropertyExpressionsFromSubquery(targetEntityType, identifyingColumn, identifyingColumn.IsNullable);

innerShaper = new RelationalEntityShaperExpression(
targetEntityType, new EntityProjectionExpression(targetEntityType, propertyExpressions), true);
}
else
{
var innerSelectExpression = _sqlExpressionFactory.Select(targetEntityType);
var innerShapedQuery = CreateShapedQueryExpression(targetEntityType, innerSelectExpression);

var makeNullable = foreignKey.PrincipalKey.Properties
.Concat(foreignKey.Properties)
.Select(p => p.ClrType)
.Any(t => t.IsNullableType());

var outerKey = entityShaperExpression.CreateKeyAccessExpression(
navigation.IsOnDependent
? foreignKey.Properties
: foreignKey.PrincipalKey.Properties,
makeNullable);
var innerKey = innerShapedQuery.ShaperExpression.CreateKeyAccessExpression(
navigation.IsOnDependent
? foreignKey.PrincipalKey.Properties
: foreignKey.Properties,
makeNullable);

var joinPredicate = _sqlTranslator.Translate(Expression.Equal(outerKey, innerKey));
_selectExpression.AddLeftJoin(innerSelectExpression, joinPredicate, null);
var leftJoinTable = ((LeftJoinExpression)_selectExpression.Tables.Last()).Table;
innerShaper = new RelationalEntityShaperExpression(
targetEntityType,
new EntityProjectionExpression(targetEntityType, leftJoinTable, true),
true);
}

var joinPredicate = _sqlTranslator.Translate(Expression.Equal(outerKey, innerKey));
_selectExpression.AddLeftJoin(innerSelectExpression, joinPredicate, null);
var leftJoinTable = ((LeftJoinExpression)_selectExpression.Tables.Last()).Table;
innerShaper = new EntityShaperExpression(
targetEntityType,
new EntityProjectionExpression(targetEntityType, leftJoinTable, true),
true);
entityProjectionExpression.AddNavigationBinding(navigation, innerShaper);
}

return innerShaper;
}



private static IDictionary<IProperty, ColumnExpression> LiftPropertyExpressionsFromSubquery(
IDictionary<IProperty, ColumnExpression> propertyExpressions, SelectExpression subquery)
{
var newPropertyExpressions = new Dictionary<IProperty, ColumnExpression>();
foreach (var item in propertyExpressions)
{
newPropertyExpressions[item.Key] = new ColumnExpression(
subquery.Projection[subquery.AddToProjection(item.Value)], subquery);
}

return newPropertyExpressions;
}

private static IDictionary<IProperty, ColumnExpression> GetPropertyExpressionsFromSubquery(
IEntityType entityType, ColumnExpression identifyingColumn, bool nullable)
{
var subquery = (SelectExpression)identifyingColumn.Table;
var subqueryIdentifyingColumn = (ColumnExpression)subquery.Projection
.SingleOrDefault(e => string.Equals(e.Alias, identifyingColumn.Name, StringComparison.OrdinalIgnoreCase)).Expression;

var subqueryPropertyExpressions = subqueryIdentifyingColumn.Table is TableExpression innerTable
? GetPropertyExpressionsFromTable(entityType, innerTable, nullable)
: GetPropertyExpressionsFromSubquery(entityType, subqueryIdentifyingColumn, nullable);

return LiftPropertyExpressionsFromSubquery(subqueryPropertyExpressions, subquery);
}

private static IDictionary<IProperty, ColumnExpression> GetPropertyExpressionsFromTable(
IEntityType entityType, TableExpression table, bool nullable)
{
var propertyExpressions = new Dictionary<IProperty, ColumnExpression>();
foreach (var property in entityType.GetTypesInHierarchy().SelectMany(EntityTypeExtensions.GetDeclaredProperties))
{
propertyExpressions[property] = new ColumnExpression(property, table, nullable || !property.IsPrimaryKey());
}

return propertyExpressions;
}
}

private ShapedQueryExpression AggregateResultShaper(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1029,8 +1029,8 @@ public ShaperRemappingExpressionVisitor(
{
_queryExpression = queryExpression;
_innerSelectExpression = innerSelectExpression;
_indexMap = indexMap;
_pendingCollectionOffset = pendingCollectionOffset;
_indexMap = indexMap;
}

protected override Expression VisitExtension(Expression extensionExpression)
Expand All @@ -1052,7 +1052,7 @@ protected override Expression VisitExtension(Expression extensionExpression)
indexMap[keyValuePair.Key] = _indexMap[keyValuePair.Value];
}

return new EntityShaperExpression(
return new RelationalEntityShaperExpression(
entityShaperExpression.EntityType,
new ProjectionBindingExpression(_queryExpression, indexMap),
nullable: true);
Expand Down
18 changes: 11 additions & 7 deletions src/EFCore/Query/ShapedQueryCompilingExpressionVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ private Expression ProcessEntityShaper(EntityShaperExpression entityShaperExpres
Expression.IfThenElse(
Expression.NotEqual(
entryVariable,
Expression.Constant(default(InternalEntityEntry), typeof(InternalEntityEntry))),
Expression.Default(typeof(InternalEntityEntry))),
Expression.Block(
Expression.Assign(
concreteEntityTypeVariable,
Expand Down Expand Up @@ -489,12 +489,16 @@ private Expression MaterializeEntity(

expressions.Add(
Expression.Assign(
entryVariable, Expression.Call(
QueryCompilationContext.QueryContextParameter,
_startTrackingMethodInfo,
concreteEntityTypeVariable,
instanceVariable,
shadowValuesVariable)));
entryVariable,
Expression.Condition(
Expression.Equal(concreteEntityTypeVariable, Expression.Default(typeof(IEntityType))),
Expression.Default(typeof(InternalEntityEntry)),
Expression.Call(
QueryCompilationContext.QueryContextParameter,
_startTrackingMethodInfo,
concreteEntityTypeVariable,
instanceVariable,
shadowValuesVariable))));
}

expressions.Add(instanceVariable);
Expand Down
4 changes: 2 additions & 2 deletions test/EFCore.Specification.Tests/Query/OwnedQueryTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public virtual Task Query_for_branch_type_loads_all_owned_navs(bool async)
ss => ss.Set<Branch>());
}

[ConditionalTheory]
[ConditionalTheory(Skip = "Issue#20336")]
[MemberData(nameof(IsAsyncData))]
public virtual Task Query_for_branch_type_loads_all_owned_navs_tracking(bool async)
{
Expand Down Expand Up @@ -408,7 +408,7 @@ public virtual async Task Preserve_includes_when_applying_skip_take_after_anonym
}
}

[ConditionalTheory]
[ConditionalTheory(Skip = "Issue#20336")]
[MemberData(nameof(IsAsyncData))]
public virtual Task Unmapped_property_projection_loads_owned_navigations(bool async)
{
Expand Down
Loading

0 comments on commit 5fe3628

Please sign in to comment.