Skip to content

Commit

Permalink
Fix to #30996 - Incorrect translation of comparison of current value …
Browse files Browse the repository at this point in the history
…with owned type default value (#31742)

In optional dependent table sharing scenario, when comparing the dependent to null we first look for any required properties (at least one of them needs to be null), and if we don't have any required properties we look at optional ones (even though this is somewhat ambiguous). Error was that we did similar check as with required properties (i.e. at least one of them must be null, but what we should be doing is checking that all of them are null.

Fixes #30996
  • Loading branch information
maumar authored Oct 4, 2023
1 parent 8166e14 commit 748bfee
Show file tree
Hide file tree
Showing 3 changed files with 122 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ namespace Microsoft.EntityFrameworkCore.Query;
/// </summary>
public class RelationalSqlTranslatingExpressionVisitor : ExpressionVisitor
{
private static readonly bool UseOldBehavior30996
= AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue30996", out var enabled30996) && enabled30996;

private const string RuntimeParameterPrefix = QueryCompilationContext.QueryParameterPrefix + "entity_equality_";

private static readonly List<MethodInfo> SingleResultMethodInfos = new()
Expand Down Expand Up @@ -1671,19 +1674,33 @@ private bool TryRewriteEntityEquality(
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));
Expression? optionalPropertiesCondition;
if (!UseOldBehavior30996)
{
optionalPropertiesCondition = 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.AndAlso(l, r) : Expression.OrElse(l, r));
}
else
{
optionalPropertiesCondition = 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
? optionalPropertiesCondition
: nodeType == ExpressionType.Equal
? Expression.OrElse(condition, atLeastOneNonNullValueInNullablePropertyCondition)
: Expression.AndAlso(condition, atLeastOneNonNullValueInNullablePropertyCondition);
? Expression.OrElse(condition, optionalPropertiesCondition)
: Expression.AndAlso(condition, optionalPropertiesCondition);
}

if (condition != null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,64 @@ public virtual async Task Owned_entity_with_all_null_properties_entity_equality_
});
}

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

using var context = contextFactory.CreateContext();
var query = context.RotRutCases
.AsNoTracking()
.OrderBy(e => e.Id)
.Select(e => e.Rot == null ? null : new RotDto { MyApartmentNo = e.Rot.ApartmentNo, MyServiceType = e.Rot.ServiceType });

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

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

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

using var context = contextFactory.CreateContext();
var query = context.RotRutCases
.AsNoTracking()
.OrderBy(e => e.Id)
.Select(e => e.Rot != null ? new RotDto { MyApartmentNo = e.Rot.ApartmentNo, MyServiceType = e.Rot.ServiceType } : null);

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

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

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual async Task Owned_entity_with_all_null_properties_property_access_when_not_containing_another_owned_entity(bool async)
Expand Down Expand Up @@ -364,6 +422,12 @@ public class Rot
public string ApartmentNo { get; set; }
}

public class RotDto
{
public int? MyServiceType { get; set; }
public string MyApartmentNo { get; set; }
}

public class Rut
{
public int? Value { get; set; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,37 @@ public override async Task Owned_entity_with_all_null_properties_entity_equality
"""
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)
WHERE ([r].[Rot_ApartmentNo] IS NOT NULL) OR ([r].[Rot_ServiceType] IS NOT NULL)
""");
}

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

AssertSql(
"""
SELECT CASE
WHEN ([r].[Rot_ApartmentNo] IS NULL) AND ([r].[Rot_ServiceType] IS NULL) THEN CAST(1 AS bit)
ELSE CAST(0 AS bit)
END, [r].[Rot_ApartmentNo], [r].[Rot_ServiceType]
FROM [RotRutCases] AS [r]
ORDER BY [r].[Id]
""");
}

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

AssertSql(
"""
SELECT CASE
WHEN ([r].[Rot_ApartmentNo] IS NOT NULL) OR ([r].[Rot_ServiceType] IS NOT NULL) THEN CAST(1 AS bit)
ELSE CAST(0 AS bit)
END, [r].[Rot_ApartmentNo], [r].[Rot_ServiceType]
FROM [RotRutCases] AS [r]
ORDER BY [r].[Id]
""");
}

Expand Down

0 comments on commit 748bfee

Please sign in to comment.