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
Resolves #21488
Resolves #23198
  • Loading branch information
@smitpatel
smitpatel committed Dec 9, 2020
1 parent 82847c7 commit d00b4ec
Show file tree
Hide file tree
Showing 11 changed files with 311 additions and 118 deletions.
11 changes: 5 additions & 6 deletions src/EFCore.Relational/Query/RelationalEntityShaperExpression.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,12 +120,11 @@ 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 allNonSharedNonPkProperties = GetNonSharedNonPkProperties(table, entityType);
if (allNonSharedNonPkProperties.Count != 0
&& allNonSharedNonPkProperties.All(p => p.IsNullable))
{
var allNonSharedNullableProperties = allNonSharedProperties.Where(p => p.IsNullable).ToList();
var atLeastOneNonNullValueInNullablePropertyCondition = allNonSharedNullableProperties
var atLeastOneNonNullValueInNullablePropertyCondition = allNonSharedNonPkProperties
.Select(
p => NotEqual(
valueBufferParameter.CreateValueBufferReadValueExpression(typeof(object), p.GetIndex(), p),
Expand Down Expand Up @@ -183,7 +182,7 @@ public override EntityShaperExpression Update(Expression valueBufferExpression)
: this;
}

private IReadOnlyList<IProperty> GetNonSharedProperties(ITableBase table, IEntityType entityType)
private IReadOnlyList<IProperty> GetNonSharedNonPkProperties(ITableBase table, IEntityType entityType)
{
var nonSharedProperties = new List<IProperty>();
var principalEntityTypes = new HashSet<IEntityType>();
Expand Down
91 changes: 67 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,35 @@ 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 allNonSharedNonPkProperties = GetNonSharedNonPkProperties(table, entityType1);
if (allNonSharedNonPkProperties.Count != 0
&& allNonSharedNonPkProperties.All(p => p.IsNullable))
{
result = Visit(
allNonPkProperties.Select(
var atLeastOneNonNullValueInNullablePropertyCondition = allNonSharedNonPkProperties
.Select(
p =>
{
var comparison = Expression.Call(
Expand All @@ -1322,9 +1321,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 Expand Up @@ -1411,6 +1420,40 @@ private bool TryRewriteEntityEquality(
return true;
}

private IReadOnlyList<IProperty> GetNonSharedNonPkProperties(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);
}
}

private Expression CreatePropertyAccessExpression(Expression target, IProperty property)
{
switch (target)
Expand Down
91 changes: 44 additions & 47 deletions src/EFCore.Relational/Query/SqlExpressionFactory.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -941,66 +941,63 @@ 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);
predicate = requiredNonPkProperties.Select(e => IsNotNull(e, entityProjectionExpression)).Aggregate((l, r) => AndAlso(l, r));
}

if (requiredNonPkProperties.Count > 1)
{
predicate
= requiredNonPkProperties
.Skip(1)
.Aggregate(
predicate, (current, property) =>
AndAlso(
IsNotNull(property, entityProjectionExpression),
current));
}
var allNonSharedNonPkProperties = GetNonSharedNonPkProperties(table, entityType);
if (allNonSharedNonPkProperties.Count != 0
&& allNonSharedNonPkProperties.All(p => p.IsNullable))
{
var atLeastOneNonNullValueInNullablePropertyCondition = allNonSharedNonPkProperties
.Select(e => IsNotNull(e, entityProjectionExpression))
.Aggregate((a, b) => OrElse(a, b));

predicate = predicate == null
? atLeastOneNonNullValueInNullablePropertyCondition
: AndAlso(predicate, atLeastOneNonNullValueInNullablePropertyCondition);
}

if (predicate != null)
{
selectExpression.ApplyPredicate(predicate);
}
else
}

private IReadOnlyList<IProperty> GetNonSharedNonPkProperties(ITableBase table, IEntityType entityType)
{
var nonSharedProperties = new List<IProperty>();
var principalEntityTypes = new HashSet<IEntityType>();
GetPrincipalEntityTypes(table, entityType, principalEntityTypes);
foreach (var property in entityType.GetProperties())
{
var allNonPkProperties = entityType.GetProperties().Where(p => !p.IsPrimaryKey()).ToList();
if (allNonPkProperties.Count > 0)
if (property.IsPrimaryKey())
{
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);
continue;
}

// 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 propertyMappings = table.FindColumn(property)!.PropertyMappings;
if (propertyMappings.Count() > 1
&& propertyMappings.Any(pm => principalEntityTypes.Contains(pm.TableMapping.EntityType)))
{
continue;
}

var otherSelectExpression = new SelectExpression(entityType, this);
nonSharedProperties.Add(property);
}

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

selectExpression.ApplyUnion(otherSelectExpression, distinct: true);
}
}
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);
}
}

Expand Down
18 changes: 18 additions & 0 deletions test/EFCore.Relational.Specification.Tests/TableSplittingTestBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -549,6 +549,24 @@ public virtual void Can_change_principal_and_dependent_instance_non_derived()
}
}

[ConditionalFact]
public virtual void Optional_dependent_materialized_when_no_properties()
{
using (CreateTestStore(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 virtual string DatabaseName { get; } = "TableSplittingTest";
protected TestStore TestStore { get; set; }
protected abstract ITestStoreFactory TestStoreFactory { get; }
Expand Down
7 changes: 0 additions & 7 deletions test/EFCore.Specification.Tests/GraphUpdates/GraphUpdatesTestBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1413,7 +1413,6 @@ public override int GetHashCode()
protected class RequiredSingle1 : NotifyingEntity
{
private int _id;
private bool _bool;
private Root _root;
private RequiredSingle2 _single;

Expand All @@ -1423,12 +1422,6 @@ public int Id
set => SetWithNotify(value, ref _id);
}

public bool Bool
{
get => _bool;
set => SetWithNotify(value, ref _bool);
}

public Root Root
{
get => _root;
Expand Down
2 changes: 2 additions & 0 deletions test/EFCore.SqlServer.FunctionalTests/GraphUpdates/GraphUpdatesSqlServerTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,8 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
r.OwnsOne(e => e.Single)
.WithOwner(e => e.Back)
.HasForeignKey(e => e.Id);

r.Navigation(e => e.Single).IsRequired();
});

b.HasOne(e => e.OptionalSingle)
Expand Down
Loading

0 comments on commit d00b4ec

Please sign in to comment.