From cb004a3bc143eb6e3056a595ffd878faf943b517 Mon Sep 17 00:00:00 2001 From: Smit Patel Date: Wed, 19 Aug 2020 00:22:24 -0700 Subject: [PATCH] =?UTF-8?q?Schr=C3=B6dinger's=20Cat=20(#22123)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Query: New set of conditions to materialize optional dependents Let A be the set of properties which are not sharing column with any principal. B be the subset of A where property is required. C be subset of A where property is optional. (hence A = B + C) The necessary condition for materialization without anything else be C1 - all required properties (excluding PK) must have a value (with or without sharing). - If A is empty, then C1 will be sufficient condition as it will be required dependent. - If B is non-empty, then C1 will cover those properties and be sufficient condition. - If B is empty and then C is non-empty, then materialize if there is at least 1 non-null value in C along with C1 being true. - Otherwise null. If there are no required properties other than PK (essentially C1 does not exist). B is by definition empty. - If A is empty then always materialize. - If A is non-empty implies C is non-empty, then materialize if there is at least 1 non-null value in C. Resolves #22054 --- .../Query/RelationalEntityShaperExpression.cs | 85 +++++++--- .../Query/QueryBugsTest.cs | 145 ++++++++++++++++++ 2 files changed, 208 insertions(+), 22 deletions(-) diff --git a/src/EFCore.Relational/Query/RelationalEntityShaperExpression.cs b/src/EFCore.Relational/Query/RelationalEntityShaperExpression.cs index 5dd4c9e5fb6..8685a259319 100644 --- a/src/EFCore.Relational/Query/RelationalEntityShaperExpression.cs +++ b/src/EFCore.Relational/Query/RelationalEntityShaperExpression.cs @@ -6,6 +6,7 @@ using System.Linq; using System.Linq.Expressions; using System.Reflection; +using System.Runtime.CompilerServices; using JetBrains.Annotations; using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.EntityFrameworkCore.Infrastructure; @@ -103,37 +104,43 @@ protected override LambdaExpression GenerateMaterializationCondition(IEntityType if (entityType.FindPrimaryKey() != null) { - if (entityType.GetViewOrTableMappings().FirstOrDefault()?.Table.IsOptional(entityType) == true) + var table = entityType.GetViewOrTableMappings().FirstOrDefault()?.Table; + if (table != null + && table.IsOptional(entityType)) { // Optional dependent var body = baseCondition.Body; var valueBufferParameter = baseCondition.Parameters[0]; + Expression condition = null; var requiredNonPkProperties = entityType.GetProperties().Where(p => !p.IsNullable && !p.IsPrimaryKey()).ToList(); if (requiredNonPkProperties.Count > 0) { - body = Condition( - requiredNonPkProperties - .Select(p => NotEqual( - valueBufferParameter.CreateValueBufferReadValueExpression(typeof(object), p.GetIndex(), p), - Constant(null))) - .Aggregate((a, b) => AndAlso(a, b)), - body, - Default(typeof(IEntityType))); + condition = requiredNonPkProperties + .Select(p => NotEqual( + valueBufferParameter.CreateValueBufferReadValueExpression(typeof(object), p.GetIndex(), p), + Constant(null))) + .Aggregate((a, b) => AndAlso(a, b)); } - else + + var allNonSharedProperties = GetNonSharedProperties(table, entityType); + if (allNonSharedProperties.Count != 0 + && allNonSharedProperties.All(p => p.IsNullable)) + { + var allNonSharedNullableProperties = allNonSharedProperties.Where(p => p.IsNullable).ToList(); + var atLeastOneNonNullValueInNullablePropertyCondition = allNonSharedNullableProperties + .Select(p => NotEqual( + valueBufferParameter.CreateValueBufferReadValueExpression(typeof(object), p.GetIndex(), p), + Constant(null))) + .Aggregate((a, b) => OrElse(a, b)); + + condition = condition == null + ? atLeastOneNonNullValueInNullablePropertyCondition + : AndAlso(condition, atLeastOneNonNullValueInNullablePropertyCondition); + } + + if (condition != null) { - var allNonPkProperties = entityType.GetProperties().Where(p => !p.IsPrimaryKey()).ToList(); - if (allNonPkProperties.Count > 0) - { - body = Condition( - allNonPkProperties - .Select(p => NotEqual( - valueBufferParameter.CreateValueBufferReadValueExpression(typeof(object), p.GetIndex(), p), - Constant(null))) - .Aggregate((a, b) => OrElse(a, b)), - body, - Default(typeof(IEntityType))); - } + body = Condition(condition, body, Default(typeof(IEntityType))); } return Lambda(body, valueBufferParameter); @@ -169,5 +176,39 @@ public override EntityShaperExpression Update(Expression valueBufferExpression) ? new RelationalEntityShaperExpression(EntityType, valueBufferExpression, IsNullable, MaterializationCondition) : this; } + + private IReadOnlyList GetNonSharedProperties(ITableBase table, IEntityType entityType) + { + var nonSharedProperties = new List(); + var principalEntityTypes = new HashSet(); + 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 entityTypes) + { + foreach (var linkingFk in table.GetRowInternalForeignKeys(entityType)) + { + entityTypes.Add(linkingFk.PrincipalEntityType); + GetPrincipalEntityTypes(table, linkingFk.PrincipalEntityType, entityTypes); + } + } } } diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/QueryBugsTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/QueryBugsTest.cs index 9e6707da8e1..4ff4cb33eaa 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/QueryBugsTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/QueryBugsTest.cs @@ -8375,6 +8375,151 @@ private SqlServerTestStore CreateDatabase21807() #endregion + #region Issue22054 + + [ConditionalFact] + public virtual void Optional_dependent_is_null_when_sharing_required_column_with_principal() + { + using (CreateDatabase22054()) + { + using var context = new MyContext22054(_options); + + var query = context.Set().OrderByDescending(e => e.Id).ToList(); + + Assert.Equal(3, query.Count); + + Assert.Null(query[0].Contact); + Assert.Null(query[0].Data); + Assert.NotNull(query[1].Data); + Assert.NotNull(query[1].Contact); + Assert.Null(query[1].Contact.Address); + Assert.NotNull(query[2].Data); + Assert.NotNull(query[2].Contact); + Assert.NotNull(query[2].Contact.Address); + + AssertSql( + @"SELECT [u].[Id], [u].[RowVersion], [u].[Contact_MobileNumber], [u].[SharedProperty], [u].[RowVersion], [u].[Contact_Address_City], [u].[Contact_Address_Zip], [u].[Data_Data] +FROM [User22054] AS [u] +ORDER BY [u].[Id] DESC"); + } + } + + private class User22054 + { + public int Id { get; set; } + public Data22054 Data { get; set; } + public Contact22054 Contact { get; set; } + public byte[] RowVersion { get; set; } + } + + private class Data22054 + { + public string Data { get; set; } + } + + private class Contact22054 + { + public string MobileNumber { get; set; } + public string SharedProperty { get; set; } + public Address22054 Address { get; set; } + } + + private class Address22054 + { + public string City { get; set; } + public string SharedProperty { get; set; } + public string Zip { get; set; } + } + + private class MyContext22054 : DbContext + { + public MyContext22054(DbContextOptions options) + : base(options) + { + } + + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + modelBuilder.Entity(builder => + { + builder.HasKey(x => x.Id); + + builder.OwnsOne(x => x.Contact, contact => + { + contact.Property(e => e.SharedProperty).IsRequired().HasColumnName("SharedProperty"); + + contact.OwnsOne(c => c.Address, address => + { + address.Property("SharedProperty").IsRequired().HasColumnName("SharedProperty"); + }); + }); + + builder.OwnsOne(e => e.Data) + .Property("RowVersion") + .IsRowVersion() + .IsRequired() + .HasColumnType("TIMESTAMP") + .HasColumnName("RowVersion"); + + builder.Property(x => x.RowVersion) + .HasColumnType("TIMESTAMP") + .IsRowVersion() + .IsRequired() + .HasColumnName("RowVersion"); + }); + } + } + + private SqlServerTestStore CreateDatabase22054() + => CreateTestStore( + () => new MyContext22054(_options), + context => + { + context.AddRange( + new User22054 + { + Data = new Data22054 + { + Data = "Data1" + }, + Contact = new Contact22054 + { + MobileNumber = "123456", + SharedProperty = "Value1", + Address = new Address22054 + { + City = "Seattle", + Zip = "12345", + SharedProperty = "Value1" + } + } + }, + new User22054 + { + Data = new Data22054 + { + Data = "Data2" + }, + Contact = new Contact22054 + { + MobileNumber = "654321", + SharedProperty = "Value2", + Address = null + } + }, + new User22054 + { + Contact = null, + Data = null + }); + + context.SaveChanges(); + + ClearLog(); + }); + + #endregion + private DbContextOptions _options; private SqlServerTestStore CreateTestStore(