Skip to content

Commit

Permalink
Generate condition for required property in optional dependents
Browse files Browse the repository at this point in the history
Resolves #29142
  • Loading branch information
smitpatel committed Sep 26, 2022
1 parent 5076987 commit ed71bc5
Show file tree
Hide file tree
Showing 7 changed files with 252 additions and 105 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,11 @@ public static IEnumerable<ITableMappingBase> GetViewOrTableMappings(this IEntity
/// 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 IEnumerable<IProperty> GetNonPrincipalSharedNonPkProperties(this IEntityType entityType, ITableBase table)
public static List<IProperty> GetNonPrincipalSharedNonPkProperties(this IEntityType entityType, ITableBase table)
{
var principalEntityTypes = new HashSet<IEntityType>();
PopulatePrincipalEntityTypes(table, entityType, principalEntityTypes);
var properties = new List<IProperty>();
foreach (var property in entityType.GetProperties())
{
if (property.IsPrimaryKey())
Expand All @@ -104,9 +105,11 @@ public static IEnumerable<IProperty> GetNonPrincipalSharedNonPkProperties(this I
continue;
}

yield return property;
properties.Add(property);
}

return properties;

static void PopulatePrincipalEntityTypes(ITableBase table, IEntityType entityType, HashSet<IEntityType> entityTypes)
{
foreach (var linkingFk in table.GetRowInternalForeignKeys(entityType))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ protected override LambdaExpression GenerateMaterializationCondition(IEntityType

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.Any()
if (allNonPrincipalSharedNonPkProperties.Count != 0
&& allNonPrincipalSharedNonPkProperties.All(p => p.IsNullable))
{
var atLeastOneNonNullValueInNullablePropertyCondition = allNonPrincipalSharedNonPkProperties
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System.Collections;
using System.Diagnostics.CodeAnalysis;
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Metadata.Internal;
using Microsoft.EntityFrameworkCore.Query.SqlExpressions;

Expand Down Expand Up @@ -1184,18 +1185,43 @@ protected override Expression VisitUnary(UnaryExpression unaryExpression)
}

// this is optional dependent sharing table
var nonPrincipalSharedNonPkProperties = entityType.GetNonPrincipalSharedNonPkProperties(table).ToList();
var nonPrincipalSharedNonPkProperties = entityType.GetNonPrincipalSharedNonPkProperties(table);
if (nonPrincipalSharedNonPkProperties.Contains(property))
{
// The column is not being shared with principal side so we can always use directly
return propertyAccess;
}

var condition = nonPrincipalSharedNonPkProperties
.Where(e => !e.IsNullable)
.Select(p => entityProjectionExpression.BindProperty(p))
.Select(c => (SqlExpression)_sqlExpressionFactory.NotEqual(c, _sqlExpressionFactory.Constant(null)))
.Aggregate((a, b) => _sqlExpressionFactory.AndAlso(a, b));
SqlExpression? condition = null;
// Property is being shared with principal side, so we need to make it conditional access
var allRequiredNonPkProperties =
entityType.GetProperties().Where(p => !p.IsNullable && !p.IsPrimaryKey()).ToList();
if (allRequiredNonPkProperties.Count > 0)
{
condition = allRequiredNonPkProperties.Select(p => entityProjectionExpression.BindProperty(p))
.Select(c => (SqlExpression)_sqlExpressionFactory.NotEqual(c, _sqlExpressionFactory.Constant(null)))
.Aggregate((a, b) => _sqlExpressionFactory.AndAlso(a, b));
}

if (nonPrincipalSharedNonPkProperties.Count != 0
&& nonPrincipalSharedNonPkProperties.All(p => p.IsNullable))
{
// If all non principal shared properties are nullable then we need additional condition
var atLeastOneNonNullValueInNullableColumnsCondition = nonPrincipalSharedNonPkProperties
.Select(p => entityProjectionExpression.BindProperty(p))
.Select(c => (SqlExpression)_sqlExpressionFactory.NotEqual(c, _sqlExpressionFactory.Constant(null)))
.Aggregate((a, b) => _sqlExpressionFactory.OrElse(a, b));

condition = condition == null
? atLeastOneNonNullValueInNullableColumnsCondition
: _sqlExpressionFactory.AndAlso(condition, atLeastOneNonNullValueInNullableColumnsCondition);
}

if (condition == null)
{
// if we cannot compute condition then we just return property access (and hope for the best)
return propertyAccess;
}

return _sqlExpressionFactory.Case(
new List<CaseWhenClause> { new(condition, propertyAccess) },
Expand Down Expand Up @@ -1610,17 +1636,47 @@ private bool TryRewriteEntityEquality(
?? nullComparedEntityType.GetDefaultMappings().Single().Table;
if (table.IsOptional(nullComparedEntityType))
{
var condition = nullComparedEntityType.GetNonPrincipalSharedNonPkProperties(table)
.Where(e => !e.IsNullable)
.Select(
p => Infrastructure.ExpressionExtensions.CreateEqualsExpression(
CreatePropertyAccessExpression(nonNullEntityReference, p),
Expression.Constant(null, p.ClrType.MakeNullable()),
nodeType != ExpressionType.Equal))
.Aggregate((l, r) => nodeType == ExpressionType.Equal ? Expression.OrElse(l, r) : Expression.AndAlso(l, r));

result = Visit(condition);
return true;
Expression? condition = null;
// Optional dependent sharing table
var requiredNonPkProperties = nullComparedEntityType.GetProperties().Where(p => !p.IsNullable && !p.IsPrimaryKey()).ToList();
if (requiredNonPkProperties.Count > 0)
{
condition = requiredNonPkProperties.Select(
p => Infrastructure.ExpressionExtensions.CreateEqualsExpression(
CreatePropertyAccessExpression(nonNullEntityReference, p),
Expression.Constant(null, p.ClrType.MakeNullable()),
nodeType != ExpressionType.Equal))
.Aggregate((l, r) => nodeType == ExpressionType.Equal ? Expression.OrElse(l, r) : Expression.AndAlso(l, r));
}

var allNonPrincipalSharedNonPkProperties = nullComparedEntityType.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 atLeastOneNonNullValueInNullablePropertyCondition = allNonPrincipalSharedNonPkProperties
.Select(
p => Infrastructure.ExpressionExtensions.CreateEqualsExpression(
CreatePropertyAccessExpression(nonNullEntityReference, p),
Expression.Constant(null, p.ClrType.MakeNullable()),
nodeType != ExpressionType.Equal))
.Aggregate((l, r) => nodeType == ExpressionType.Equal ? Expression.OrElse(l, r) : Expression.AndAlso(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;
}

result = null;
return false;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,55 @@ public virtual async Task Owned_entity_with_all_null_properties_materializes_whe
t =>
{
Assert.Equal("Buyer2", t.Buyer);
// Cannot verify owned entities here since they differ between relational/in-memory
Assert.Null(t.Rot);
Assert.Null(t.Rut);
});
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual async Task Owned_entity_with_all_null_properties_entity_equality_when_not_containing_another_owned_entity(bool async)
{
var contextFactory = await InitializeAsync<MyContext28247>(seed: c => c.Seed());

using var context = contextFactory.CreateContext();
var query = context.RotRutCases.AsNoTracking().Select(e => e.Rot).Where(e => e != null);

var result = async
? await query.ToListAsync()
: query.ToList();

Assert.Collection(
result,
t =>
{
Assert.Equal(1, t.ServiceType);
Assert.Equal("1", t.ApartmentNo);
});
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual async Task Owned_entity_with_all_null_properties_property_access_when_not_containing_another_owned_entity(bool async)
{
var contextFactory = await InitializeAsync<MyContext28247>(seed: c => c.Seed());

using var context = contextFactory.CreateContext();
var query = context.RotRutCases.AsNoTracking().Select(e => e.Rot.ApartmentNo);

var result = async
? await query.ToListAsync()
: query.ToList();

Assert.Collection(
result,
t =>
{
Assert.Equal("1", t);
},
t =>
{
Assert.Null(t);
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,12 +182,13 @@ public override async Task Normal_entity_owning_a_split_reference_with_main_frag

AssertSql(
@"SELECT [e].[Id], CASE
WHEN [e].[OwnedReference_Id] IS NOT NULL AND [e].[OwnedReference_OwnedIntValue1] IS NOT NULL AND [e].[OwnedReference_OwnedIntValue2] IS NOT NULL THEN [o].[OwnedIntValue4]
WHEN [e].[OwnedReference_Id] IS NOT NULL AND [e].[OwnedReference_OwnedIntValue1] IS NOT NULL AND [e].[OwnedReference_OwnedIntValue2] IS NOT NULL AND [o0].[OwnedIntValue3] IS NOT NULL AND [o].[OwnedIntValue4] IS NOT NULL THEN [o].[OwnedIntValue4]
END AS [OwnedIntValue4], CASE
WHEN [e].[OwnedReference_Id] IS NOT NULL AND [e].[OwnedReference_OwnedIntValue1] IS NOT NULL AND [e].[OwnedReference_OwnedIntValue2] IS NOT NULL THEN [o].[OwnedStringValue4]
WHEN [e].[OwnedReference_Id] IS NOT NULL AND [e].[OwnedReference_OwnedIntValue1] IS NOT NULL AND [e].[OwnedReference_OwnedIntValue2] IS NOT NULL AND [o0].[OwnedIntValue3] IS NOT NULL AND [o].[OwnedIntValue4] IS NOT NULL THEN [o].[OwnedStringValue4]
END AS [OwnedStringValue4]
FROM [EntityOnes] AS [e]
LEFT JOIN [OwnedReferenceExtras2] AS [o] ON [e].[Id] = [o].[EntityOneId]");
LEFT JOIN [OwnedReferenceExtras2] AS [o] ON [e].[Id] = [o].[EntityOneId]
LEFT JOIN [OwnedReferenceExtras1] AS [o0] ON [e].[Id] = [o0].[EntityOneId]");
}

[ConditionalTheory(Skip = "Issue29075")]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,4 +139,23 @@ public override async Task Owned_entity_with_all_null_properties_materializes_wh
FROM [RotRutCases] AS [r]
ORDER BY [r].[Buyer]");
}

public override async Task Owned_entity_with_all_null_properties_entity_equality_when_not_containing_another_owned_entity(bool async)
{
await base.Owned_entity_with_all_null_properties_entity_equality_when_not_containing_another_owned_entity(async);

AssertSql(
@"SELECT [r].[Id], [r].[Rot_ApartmentNo], [r].[Rot_ServiceType]
FROM [RotRutCases] AS [r]
WHERE [r].[Rot_ApartmentNo] IS NOT NULL AND [r].[Rot_ServiceType] IS NOT NULL");
}

public override async Task Owned_entity_with_all_null_properties_property_access_when_not_containing_another_owned_entity(bool async)
{
await base.Owned_entity_with_all_null_properties_property_access_when_not_containing_another_owned_entity(async);

AssertSql(
@"SELECT [r].[Rot_ApartmentNo]
FROM [RotRutCases] AS [r]");
}
}
Loading

0 comments on commit ed71bc5

Please sign in to comment.