Skip to content

Commit

Permalink
Query: Implement optional dependent conditions
Browse files Browse the repository at this point in the history 
An optional dependent considered present
If all non-nullable non-PK properties have a non-null value, or
If all non-nullable non-PK properties are shared with principal then at least one nullable non shared property has a non-null value
If there are no non-shared properties then it is always present and act like a required dependent

Resolves #23564
  • Loading branch information
@smitpatel
smitpatel committed Apr 1, 2021
1 parent 0c3d9f1 commit e9f3751
Show file tree
Hide file tree
Showing 10 changed files with 287 additions and 156 deletions.
40 changes: 40 additions & 0 deletions src/EFCore.Relational/Metadata/Internal/RelationalEntityTypeExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,5 +42,45 @@ public static IEnumerable<ITableMappingBase> GetViewOrTableMappings(this IEntity
/// </summary>
public static IReadOnlyList<string> GetTptDiscriminatorValues(this IReadOnlyEntityType entityType)
=> entityType.GetConcreteDerivedTypesInclusive().Select(et => et.ShortName()).ToList();

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public static IReadOnlyList<IProperty> GetNonPrincipalSharedNonPkProperties(this IEntityType entityType, ITableBase table)
{
var nonPrincipalSharedProperties = new List<IProperty>();
var principalEntityTypes = new HashSet<IEntityType>();
PopulatePrincipalEntityTypes(table, entityType, principalEntityTypes);
foreach (var property in entityType.GetProperties())
{
if (property.IsPrimaryKey())
{
continue;
}

var propertyMappings = table.FindColumn(property)!.PropertyMappings;
if (propertyMappings.Count() > 1
&& propertyMappings.Any(pm => principalEntityTypes.Contains(pm.TableMapping.EntityType)))
{
continue;
}

nonPrincipalSharedProperties.Add(property);
}

return nonPrincipalSharedProperties;

static void PopulatePrincipalEntityTypes(ITableBase table, IEntityType entityType, HashSet<IEntityType> entityTypes)
{
foreach (var linkingFk in table.GetRowInternalForeignKeys(entityType))
{
entityTypes.Add(linkingFk.PrincipalEntityType);
PopulatePrincipalEntityTypes(table, linkingFk.PrincipalEntityType, entityTypes);
}
}
}
}
}
44 changes: 5 additions & 39 deletions src/EFCore.Relational/Query/RelationalEntityShaperExpression.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,12 +117,12 @@ protected override LambdaExpression GenerateMaterializationCondition(IEntityType
.Aggregate((a, b) => AndAlso(a, b));
}

var allNonSharedProperties = GetNonSharedProperties(table, entityType);
if (allNonSharedProperties.Count != 0
&& allNonSharedProperties.All(p => p.IsNullable))
var allNonPrincipalSharedNonPkProperties = entityType.GetNonPrincipalSharedNonPkProperties(table);
// We don't need condition for nullable property if there exist at least one required property which is non shared.
if (allNonPrincipalSharedNonPkProperties.Count != 0
&& allNonPrincipalSharedNonPkProperties.All(p => p.IsNullable))
{
var allNonSharedNullableProperties = allNonSharedProperties.Where(p => p.IsNullable).ToList();
var atLeastOneNonNullValueInNullablePropertyCondition = allNonSharedNullableProperties
var atLeastOneNonNullValueInNullablePropertyCondition = allNonPrincipalSharedNonPkProperties
.Select(
p => NotEqual(
valueBufferParameter.CreateValueBufferReadValueExpression(typeof(object), p.GetIndex(), p),
Expand Down Expand Up @@ -179,39 +179,5 @@ public override EntityShaperExpression Update(Expression valueBufferExpression)
? new RelationalEntityShaperExpression(EntityType, valueBufferExpression, IsNullable, MaterializationCondition)
: this;
}

private IReadOnlyList<IProperty> GetNonSharedProperties(ITableBase table, IEntityType entityType)
{
var nonSharedProperties = new List<IProperty>();
var principalEntityTypes = new HashSet<IEntityType>();
GetPrincipalEntityTypes(table, entityType, principalEntityTypes);
foreach (var property in entityType.GetProperties())
{
if (property.IsPrimaryKey())
{
continue;
}

var propertyMappings = table.FindColumn(property)!.PropertyMappings;
if (propertyMappings.Count() > 1
&& propertyMappings.Any(pm => principalEntityTypes.Contains(pm.TableMapping.EntityType)))
{
continue;
}

nonSharedProperties.Add(property);
}

return nonSharedProperties;
}

private void GetPrincipalEntityTypes(ITableBase table, IEntityType entityType, HashSet<IEntityType> entityTypes)
{
foreach (var linkingFk in table.GetRowInternalForeignKeys(entityType))
{
entityTypes.Add(linkingFk.PrincipalEntityType);
GetPrincipalEntityTypes(table, linkingFk.PrincipalEntityType, entityTypes);
}
}
}
}
58 changes: 34 additions & 24 deletions src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1282,36 +1282,36 @@ private bool TryRewriteEntityEquality(
{
var nonNullEntityReference = (IsNullSqlConstantExpression(left) ? rightEntityReference : leftEntityReference)!;
var entityType1 = nonNullEntityReference.EntityType;

if (entityType1.GetViewOrTableMappings().FirstOrDefault()?.Table.IsOptional(entityType1) == true)
var table = entityType1.GetViewOrTableMappings().FirstOrDefault()?.Table;
if (table?.IsOptional(entityType1) == true)
{
Expression? condition = null;
// Optional dependent sharing table
var requiredNonPkProperties = entityType1.GetProperties().Where(p => !p.IsNullable && !p.IsPrimaryKey()).ToList();
if (requiredNonPkProperties.Count > 0)
{
result = Visit(
requiredNonPkProperties.Select(
p =>
{
var comparison = Expression.Call(
_objectEqualsMethodInfo,
Expression.Convert(CreatePropertyAccessExpression(nonNullEntityReference, p), typeof(object)),
Expression.Convert(Expression.Constant(null, p.ClrType.MakeNullable()), typeof(object)));
return nodeType == ExpressionType.Equal
? (Expression)comparison
: Expression.Not(comparison);
}).Aggregate(
(l, r) => nodeType == ExpressionType.Equal ? Expression.OrElse(l, r) : Expression.AndAlso(l, r)));

return true;
condition = requiredNonPkProperties.Select(
p =>
{
var comparison = Expression.Call(
_objectEqualsMethodInfo,
Expression.Convert(CreatePropertyAccessExpression(nonNullEntityReference, p), typeof(object)),
Expression.Convert(Expression.Constant(null, p.ClrType.MakeNullable()), typeof(object)));
return nodeType == ExpressionType.Equal
? (Expression)comparison
: Expression.Not(comparison);
})
.Aggregate((l, r) => nodeType == ExpressionType.Equal ? Expression.OrElse(l, r) : Expression.AndAlso(l, r));
}

var allNonPkProperties = entityType1.GetProperties().Where(p => !p.IsPrimaryKey()).ToList();
if (allNonPkProperties.Count > 0)
var allNonPrincipalSharedNonPkProperties = entityType1.GetNonPrincipalSharedNonPkProperties(table);
// We don't need condition for nullable property if there exist at least one required property which is non shared.
if (allNonPrincipalSharedNonPkProperties.Count != 0
&& allNonPrincipalSharedNonPkProperties.All(p => p.IsNullable))
{
result = Visit(
allNonPkProperties.Select(
var atLeastOneNonNullValueInNullablePropertyCondition = allNonPrincipalSharedNonPkProperties
.Select(
p =>
{
var comparison = Expression.Call(
Expand All @@ -1322,9 +1322,19 @@ private bool TryRewriteEntityEquality(
return nodeType == ExpressionType.Equal
? (Expression)comparison
: Expression.Not(comparison);
}).Aggregate(
(l, r) => nodeType == ExpressionType.Equal ? Expression.AndAlso(l, r) : Expression.OrElse(l, r)));
})
.Aggregate((l, r) => nodeType == ExpressionType.Equal ? Expression.AndAlso(l, r) : Expression.OrElse(l, r));

condition = condition == null
? atLeastOneNonNullValueInNullablePropertyCondition
: nodeType == ExpressionType.Equal
? Expression.OrElse(condition, atLeastOneNonNullValueInNullablePropertyCondition)
: Expression.AndAlso(condition, atLeastOneNonNullValueInNullablePropertyCondition);
}

if (condition != null)
{
result = Visit(condition);
return true;
}

Expand Down
70 changes: 17 additions & 53 deletions src/EFCore.Relational/Query/SqlExpressionFactory.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -938,66 +938,30 @@ private void AddOptionalDependentConditions(
ITableBase table)
{
SqlExpression? predicate = null;
var entityProjectionExpression = GetMappedEntityProjectionExpression(selectExpression);
var requiredNonPkProperties = entityType.GetProperties().Where(p => !p.IsNullable && !p.IsPrimaryKey()).ToList();
if (requiredNonPkProperties.Count > 0)
{
var entityProjectionExpression = GetMappedEntityProjectionExpression(selectExpression);
predicate = IsNotNull(requiredNonPkProperties[0], entityProjectionExpression);

if (requiredNonPkProperties.Count > 1)
{
predicate
= requiredNonPkProperties
.Skip(1)
.Aggregate(
predicate, (current, property) =>
AndAlso(
IsNotNull(property, entityProjectionExpression),
current));
}

selectExpression.ApplyPredicate(predicate);
predicate = requiredNonPkProperties.Select(e => IsNotNull(e, entityProjectionExpression)).Aggregate((l, r) => AndAlso(l, r));
}
else
{
var allNonPkProperties = entityType.GetProperties().Where(p => !p.IsPrimaryKey()).ToList();
if (allNonPkProperties.Count > 0)
{
var entityProjectionExpression = GetMappedEntityProjectionExpression(selectExpression);
predicate = IsNotNull(allNonPkProperties[0], entityProjectionExpression);

if (allNonPkProperties.Count > 1)
{
predicate
= allNonPkProperties
.Skip(1)
.Aggregate(
predicate, (current, property) =>
OrElse(
IsNotNull(property, entityProjectionExpression),
current));
}

selectExpression.ApplyPredicate(predicate);

// If there is no non-nullable property then we also need to add optional dependents which are acting as principal for
// other dependents.
foreach (var referencingFk in entityType.GetReferencingForeignKeys())
{
if (referencingFk.PrincipalEntityType.IsAssignableFrom(entityType))
{
continue;
}

var otherSelectExpression = new SelectExpression(entityType, this);
var allNonSharedNonPkProperties = entityType.GetNonPrincipalSharedNonPkProperties(table);
// We don't need condition for nullable property if there exist at least one required property which is non shared.
if (allNonSharedNonPkProperties.Count != 0
&& allNonSharedNonPkProperties.All(p => p.IsNullable))
{
var atLeastOneNonNullValueInNullablePropertyCondition = allNonSharedNonPkProperties
.Select(e => IsNotNull(e, entityProjectionExpression))
.Aggregate((a, b) => OrElse(a, b));

var sameTable = table.EntityTypeMappings.Any(m => m.EntityType == referencingFk.DeclaringEntityType)
&& table.IsOptional(referencingFk.DeclaringEntityType);
AddInnerJoin(otherSelectExpression, referencingFk, sameTable ? table : null);
predicate = predicate == null
? atLeastOneNonNullValueInNullablePropertyCondition
: AndAlso(predicate, atLeastOneNonNullValueInNullablePropertyCondition);
}

selectExpression.ApplyUnion(otherSelectExpression, distinct: true);
}
}
if (predicate != null)
{
selectExpression.ApplyPredicate(predicate);
}
}

Expand Down
17 changes: 17 additions & 0 deletions test/EFCore.Relational.Specification.Tests/TableSplittingTestBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,23 @@ await InitializeAsync(
}
}

[ConditionalFact]
public virtual async Task Optional_dependent_materialized_when_no_properties()
{
await InitializeAsync(OnModelCreating);

using (var context = CreateContext())
{
var vehicle = context.Set<Vehicle>()
.Where(e => e.Name == "AIM-9M Sidewinder")
.OrderBy(e => e.Name)
.Include(e => e.Operator.Details).First();
Assert.Equal(0, vehicle.SeatingCapacity);
Assert.Equal("Heat-seeking", vehicle.Operator.Details.Type);
Assert.Null(vehicle.Operator.Name);
}
}

protected override string StoreName { get; } = "TableSplittingTest";
protected TestSqlLoggerFactory TestSqlLoggerFactory
=> (TestSqlLoggerFactory)ListLoggerFactory;
Expand Down
Loading

0 comments on commit e9f3751

Please sign in to comment.