From f2e016105908d9a0ce2d77ea9ff68b4442065871 Mon Sep 17 00:00:00 2001 From: maumar Date: Tue, 12 Sep 2023 16:16:08 -0700 Subject: [PATCH] Fix to #30996 - Incorrect translation of comparison of current value with owned type default value 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 --- ...lationalSqlTranslatingExpressionVisitor.cs | 2 +- .../OwnedEntityQueryRelationalTestBase.cs | 64 +++++++++++++++++++ .../Query/OwnedEntityQuerySqlServerTest.cs | 32 +++++++++- 3 files changed, 96 insertions(+), 2 deletions(-) diff --git a/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs b/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs index 3a96b93fb13..fd634c95101 100644 --- a/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs +++ b/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs @@ -1797,7 +1797,7 @@ bool TryRewriteEntityEquality([NotNullWhen(true)] out Expression? result) 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)); + .Aggregate((l, r) => nodeType == ExpressionType.Equal ? Expression.AndAlso(l, r) : Expression.OrElse(l, r)); condition = condition == null ? atLeastOneNonNullValueInNullablePropertyCondition diff --git a/test/EFCore.Relational.Specification.Tests/Query/OwnedEntityQueryRelationalTestBase.cs b/test/EFCore.Relational.Specification.Tests/Query/OwnedEntityQueryRelationalTestBase.cs index 54e78c7f789..f559e7266ea 100644 --- a/test/EFCore.Relational.Specification.Tests/Query/OwnedEntityQueryRelationalTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/Query/OwnedEntityQueryRelationalTestBase.cs @@ -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(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(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) @@ -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; } diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/OwnedEntityQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/OwnedEntityQuerySqlServerTest.cs index 804c9ea3db0..12c52e09b30 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/OwnedEntityQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/OwnedEntityQuerySqlServerTest.cs @@ -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] """); }