Skip to content

Commit

Permalink
Schrödinger's Cat (#22123)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@smitpatel
smitpatel committed Aug 19, 2020
1 parent 978370a commit cb004a3
Show file tree
Hide file tree
Showing 2 changed files with 208 additions and 22 deletions.
85 changes: 63 additions & 22 deletions src/EFCore.Relational/Query/RelationalEntityShaperExpression.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -169,5 +176,39 @@ public override EntityShaperExpression Update(Expression valueBufferExpression)
? new RelationalEntityShaperExpression(EntityType, valueBufferExpression, IsNullable, MaterializationCondition)
: this;
}

private IReadOnlyList<IProperty> GetNonSharedProperties(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);
}
}
}
}
145 changes: 145 additions & 0 deletions test/EFCore.SqlServer.FunctionalTests/Query/QueryBugsTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<User22054>().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<User22054>(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<string>("SharedProperty").IsRequired().HasColumnName("SharedProperty");
});
});

builder.OwnsOne(e => e.Data)
.Property<byte[]>("RowVersion")
.IsRowVersion()
.IsRequired()
.HasColumnType("TIMESTAMP")
.HasColumnName("RowVersion");

builder.Property<byte[]>(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<TContext>(
Expand Down

0 comments on commit cb004a3

Please sign in to comment.