Skip to content

Commit

Permalink
Target FKs to the derived table in TPT if possible
Browse files Browse the repository at this point in the history
Fixes #21975
  • Loading branch information
AndriySvyryd committed Aug 12, 2020
1 parent b08b7bb commit c4c9c1f
Show file tree
Hide file tree
Showing 9 changed files with 165 additions and 25 deletions.
13 changes: 11 additions & 2 deletions src/EFCore.Relational/Extensions/RelationalForeignKeyExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,11 @@ public static class RelationalForeignKeyExtensions
/// <returns> The foreign key constraint name. </returns>
public static string GetConstraintName([NotNull] this IForeignKey foreignKey)
=> foreignKey.GetConstraintName(
StoreObjectIdentifier.Table(foreignKey.DeclaringEntityType.GetTableName(), foreignKey.DeclaringEntityType.GetSchema()),
StoreObjectIdentifier.Table(foreignKey.PrincipalEntityType.GetTableName(), foreignKey.PrincipalEntityType.GetSchema()));
StoreObjectIdentifier.Create(foreignKey.DeclaringEntityType, StoreObjectType.Table).Value,
StoreObjectIdentifier.Create(foreignKey.PrincipalKey.IsPrimaryKey()
? foreignKey.PrincipalEntityType
: foreignKey.PrincipalKey.DeclaringEntityType,
StoreObjectType.Table).Value);

/// <summary>
/// Returns the foreign key constraint name.
Expand Down Expand Up @@ -80,6 +83,12 @@ public static string GetDefaultName(
{
var propertyNames = foreignKey.Properties.GetColumnNames(storeObject);
var principalPropertyNames = foreignKey.PrincipalKey.Properties.GetColumnNames(principalStoreObject);
if (propertyNames == null
|| principalPropertyNames == null)
{
return null;
}

var rootForeignKey = foreignKey;

// Limit traversal to avoid getting stuck in a cycle (validation will throw for these later)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,14 +85,19 @@ public static string GetDefaultName([NotNull] this IIndex index)
public static string GetDefaultDatabaseName([NotNull] this IIndex index, in StoreObjectIdentifier storeObject)
{
var propertyNames = index.Properties.GetColumnNames(storeObject);
if (propertyNames == null)
{
return null;
}

var rootIndex = index;

// Limit traversal to avoid getting stuck in a cycle (validation will throw for these later)
// Using a hashset is detrimental to the perf when there are no cycles
for (var i = 0; i < Metadata.Internal.RelationalEntityTypeExtensions.MaxEntityTypesSharingTable; i++)
{
IIndex linkedIndex = null;
foreach(var otherIndex in rootIndex.DeclaringEntityType
foreach (var otherIndex in rootIndex.DeclaringEntityType
.FindRowInternalForeignKeys(storeObject)
.SelectMany(fk => fk.PrincipalEntityType.GetIndexes()))
{
Expand Down
5 changes: 5 additions & 0 deletions src/EFCore.Relational/Extensions/RelationalKeyExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,11 @@ public static string GetDefaultName([NotNull] this IKey key, in StoreObjectIdent
else
{
var propertyNames = key.Properties.GetColumnNames(storeObject);
if (propertyNames == null)
{
return null;
}

var rootKey = key;

// Limit traversal to avoid getting stuck in a cycle (validation will throw for these later)
Expand Down
24 changes: 18 additions & 6 deletions src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -901,17 +901,20 @@ protected virtual void ValidateSharedForeignKeysCompatibility(

foreach (var foreignKey in mappedTypes.SelectMany(et => et.GetDeclaredForeignKeys()))
{
var principalTable = foreignKey.PrincipalEntityType.GetTableName();
var principalSchema = foreignKey.PrincipalEntityType.GetSchema();

var principalTable = foreignKey.PrincipalKey.IsPrimaryKey()
? StoreObjectIdentifier.Create(foreignKey.PrincipalEntityType, StoreObjectType.Table)
: StoreObjectIdentifier.Create(foreignKey.PrincipalKey.DeclaringEntityType, StoreObjectType.Table);
if (principalTable == null)
{
continue;
}

var foreignKeyName = foreignKey.GetConstraintName(
storeObject,
StoreObjectIdentifier.Table(principalTable, principalSchema));
var foreignKeyName = foreignKey.GetConstraintName(storeObject, principalTable.Value);
if (foreignKeyName == null)
{
continue;
}

if (!foreignKeyMappings.TryGetValue(foreignKeyName, out var duplicateForeignKey))
{
foreignKeyMappings[foreignKeyName] = foreignKey;
Expand Down Expand Up @@ -953,6 +956,11 @@ protected virtual void ValidateSharedIndexesCompatibility(
foreach (var index in mappedTypes.SelectMany(et => et.GetDeclaredIndexes()))
{
var indexName = index.GetDatabaseName(storeObject);
if (indexName == null)
{
continue;
}

if (!indexMappings.TryGetValue(indexName, out var duplicateIndex))
{
indexMappings[indexName] = index;
Expand Down Expand Up @@ -994,6 +1002,10 @@ protected virtual void ValidateSharedKeysCompatibility(
foreach (var key in mappedTypes.SelectMany(et => et.GetDeclaredKeys()))
{
var keyName = key.GetName(storeObject);
if (keyName == null)
{
continue;
}

if (!keyMappings.TryGetValue(keyName, out var duplicateKey))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,12 @@ public static IReadOnlyList<string> GetColumnNames(
var propertyNames = new List<string>();
foreach (var property in properties)
{
propertyNames.Add(property.GetColumnName(storeObject));
var columnName = property.GetColumnName(storeObject);
if (columnName == null)
{
return null;
}
propertyNames.Add(columnName);
}
return propertyNames;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,11 @@ private void TryUniquifyKeyNames(
foreach (var key in entityType.GetDeclaredKeys())
{
var keyName = key.GetName(storeObject);
if (keyName == null)
{
continue;
}

if (!keys.TryGetValue(keyName, out var otherKey))
{
keys[keyName] = key;
Expand Down Expand Up @@ -304,6 +309,11 @@ private void TryUniquifyIndexNames(
foreach (var index in entityType.GetDeclaredIndexes())
{
var indexName = index.GetDatabaseName(storeObject);
if (indexName == null)
{
continue;
}

if (!indexes.TryGetValue(indexName, out var otherIndex))
{
indexes[indexName] = index;
Expand Down Expand Up @@ -365,17 +375,16 @@ private void TryUniquifyForeignKeyNames(
{
foreach (var foreignKey in entityType.GetDeclaredForeignKeys())
{
var principalTable = foreignKey.PrincipalEntityType.GetTableName();
var principalSchema = foreignKey.PrincipalEntityType.GetSchema();
var principalTable = foreignKey.PrincipalKey.IsPrimaryKey()
? StoreObjectIdentifier.Create(foreignKey.PrincipalEntityType, StoreObjectType.Table)
: StoreObjectIdentifier.Create(foreignKey.PrincipalKey.DeclaringEntityType, StoreObjectType.Table);
if (principalTable == null
|| (foreignKey.DeclaringEntityType.GetTableName() == principalTable
&& foreignKey.DeclaringEntityType.GetSchema() == principalSchema))
|| storeObject == principalTable.Value)
{
continue;
}

var foreignKeyName = foreignKey.GetConstraintName(storeObject,
StoreObjectIdentifier.Table(principalTable, principalSchema));
var foreignKeyName = foreignKey.GetConstraintName(storeObject, principalTable.Value);
if (!foreignKeys.TryGetValue(foreignKeyName, out var otherForeignKey))
{
foreignKeys[foreignKeyName] = foreignKey;
Expand Down
12 changes: 11 additions & 1 deletion src/EFCore.Relational/Metadata/Internal/RelationalModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -753,7 +753,7 @@ private static void PopulateConstraints(Table table)
var entityType = (IConventionEntityType)entityTypeMapping.EntityType;
foreach (var foreignKey in entityType.GetForeignKeys())
{
foreach (var principalMapping in foreignKey.PrincipalEntityType.GetTableMappings())
foreach (var principalMapping in foreignKey.PrincipalEntityType.GetTableMappings().Reverse())
{
if (!principalMapping.IncludesDerivedTypes
&& foreignKey.PrincipalEntityType.GetDirectlyDerivedTypes().Any())
Expand Down Expand Up @@ -842,6 +842,11 @@ private static void PopulateConstraints(Table table)
foreach (var key in entityType.GetKeys())
{
var name = key.GetName(storeObject);
if (name == null)
{
continue;
}

var constraint = table.FindUniqueConstraint(name);
if (constraint == null)
{
Expand Down Expand Up @@ -883,6 +888,11 @@ private static void PopulateConstraints(Table table)
foreach (var index in entityType.GetIndexes())
{
var name = index.GetDatabaseName(storeObject);
if (name == null)
{
continue;
}

if (!table.Indexes.TryGetValue(name, out var tableIndex))
{
var columns = new Column[index.Properties.Count];
Expand Down
37 changes: 29 additions & 8 deletions test/EFCore.Relational.Tests/Metadata/RelationalModelTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ private static void AssertDefaultMappings(IRelationalModel model)
Assert.Equal(2, specialCustomerTable.EntityTypeMappings.Count());
Assert.True(specialCustomerTable.EntityTypeMappings.First().IsSharedTablePrincipal);

Assert.Equal(specialCustomerType.GetDiscriminatorProperty() == null ? 7 : 8, specialCustomerTable.Columns.Count());
Assert.Equal(specialCustomerType.GetDiscriminatorProperty() == null ? 8 : 9, specialCustomerTable.Columns.Count());

var specialityColumn = specialCustomerTable.Columns.Single(c => c.Name == nameof(SpecialCustomer.Speciality));
Assert.Equal(specialCustomerType.GetDiscriminatorProperty() != null, specialityColumn.IsNullable);
Expand Down Expand Up @@ -209,7 +209,7 @@ private static void AssertViews(IRelationalModel model, Mapping mapping)

var specialCustomerView = specialCustomerType.GetViewMappings().Select(t => t.Table).First(t => t.Name == "SpecialCustomerView");
Assert.Null(specialCustomerView.Schema);
Assert.Equal(3, specialCustomerView.Columns.Count());
Assert.Equal(4, specialCustomerView.Columns.Count());

Assert.True(specialCustomerView.EntityTypeMappings.Single().IsSharedTablePrincipal);

Expand Down Expand Up @@ -404,7 +404,7 @@ private static void AssertTables(IRelationalModel model, Mapping mapping)

var specialCustomerTable = specialCustomerType.GetTableMappings().Select(t => t.Table).First(t => t.Name == "SpecialCustomer");
Assert.Equal("SpecialSchema", specialCustomerTable.Schema);
Assert.Equal(3, specialCustomerTable.Columns.Count());
Assert.Equal(4, specialCustomerTable.Columns.Count());

Assert.True(specialCustomerTable.EntityTypeMappings.Single().IsSharedTablePrincipal);

Expand All @@ -425,15 +425,24 @@ private static void AssertTables(IRelationalModel model, Mapping mapping)
Assert.Equal("AK_Customer_SpecialityAk", specialCustomerUniqueConstraint.Name);
Assert.NotNull(specialCustomerUniqueConstraint.MappedKeys.Single());

var specialCustomerFkConstraint = specialCustomerTable.ForeignKeyConstraints.Single();
var specialCustomerFkConstraint = specialCustomerTable.ForeignKeyConstraints.First();
Assert.Equal("FK_SpecialCustomer_Customer_RelatedCustomerSpeciality", specialCustomerFkConstraint.Name);
Assert.Same(customerTable, specialCustomerFkConstraint.PrincipalTable);
Assert.NotNull(specialCustomerFkConstraint.MappedForeignKeys.Single());
Assert.Same(customerTable, specialCustomerFkConstraint.PrincipalTable);

var specialCustomerDbIndex = specialCustomerTable.Indexes.Single();
var anotherSpecialCustomerFkConstraint = specialCustomerTable.ForeignKeyConstraints.Last();
Assert.Equal("FK_SpecialCustomer_SpecialCustomer_AnotherRelatedCustomerId", anotherSpecialCustomerFkConstraint.Name);
Assert.NotNull(anotherSpecialCustomerFkConstraint.MappedForeignKeys.Single());
Assert.Same(specialCustomerTable, anotherSpecialCustomerFkConstraint.PrincipalTable);

var specialCustomerDbIndex = specialCustomerTable.Indexes.Last();
Assert.Equal("IX_SpecialCustomer_RelatedCustomerSpeciality", specialCustomerDbIndex.Name);
Assert.NotNull(specialCustomerDbIndex.MappedIndexes.Single());

var anotherSpecialCustomerDbIndex = specialCustomerTable.Indexes.First();
Assert.Equal("IX_SpecialCustomer_AnotherRelatedCustomerId", anotherSpecialCustomerDbIndex.Name);
Assert.NotNull(anotherSpecialCustomerDbIndex.MappedIndexes.Single());

Assert.Null(customerType.GetDiscriminatorProperty());
Assert.Null(customerType.GetDiscriminatorValue());
Assert.Null(specialCustomerType.GetDiscriminatorProperty());
Expand Down Expand Up @@ -466,13 +475,21 @@ private static void AssertTables(IRelationalModel model, Mapping mapping)
Assert.Equal("AK_Customer_SpecialityAk", specialCustomerUniqueConstraint.Name);
Assert.NotNull(specialCustomerUniqueConstraint.MappedKeys.Single());

var specialCustomerFkConstraint = specialCustomerTable.ForeignKeyConstraints.Single();
var specialCustomerFkConstraint = specialCustomerTable.ForeignKeyConstraints.Last();
Assert.Equal("FK_Customer_Customer_RelatedCustomerSpeciality", specialCustomerFkConstraint.Name);
Assert.NotNull(specialCustomerFkConstraint.MappedForeignKeys.Single());

var specialCustomerDbIndex = specialCustomerTable.Indexes.Single();
var anotherSpecialCustomerFkConstraint = specialCustomerTable.ForeignKeyConstraints.First();
Assert.Equal("FK_Customer_Customer_AnotherRelatedCustomerId", anotherSpecialCustomerFkConstraint.Name);
Assert.NotNull(anotherSpecialCustomerFkConstraint.MappedForeignKeys.Single());

var specialCustomerDbIndex = specialCustomerTable.Indexes.Last();
Assert.Equal("IX_Customer_RelatedCustomerSpeciality", specialCustomerDbIndex.Name);
Assert.NotNull(specialCustomerDbIndex.MappedIndexes.Single());

var anotherSpecialCustomerDbIndex = specialCustomerTable.Indexes.First();
Assert.Equal("IX_Customer_AnotherRelatedCustomerId", anotherSpecialCustomerDbIndex.Name);
Assert.NotNull(specialCustomerDbIndex.MappedIndexes.Single());
}
}

Expand Down Expand Up @@ -513,6 +530,9 @@ private IRelationalModel CreateTestModel(bool mapToTables = false, bool mapToVie
cb.HasOne(c => c.RelatedCustomer).WithOne()
.HasForeignKey<SpecialCustomer>(c => c.RelatedCustomerSpeciality)
.HasPrincipalKey<SpecialCustomer>("SpecialityAk"); // TODO: Use the derived one, #2611
cb.HasOne<SpecialCustomer>().WithOne()
.HasForeignKey<SpecialCustomer>("AnotherRelatedCustomerId");
});

modelBuilder.Entity<Order>(ob =>
Expand All @@ -535,6 +555,7 @@ private IRelationalModel CreateTestModel(bool mapToTables = false, bool mapToVie
var alternateId = odb.Property(o => o.AlternateId).HasColumnName("AlternateId").Metadata;
odb.OwnedEntityType.AddKey(new[] { alternateId });
// Issue #20948
//odb.HasAlternateKey(o => o.AlternateId);
odb.HasOne(od => od.DateDetails).WithOne()
.HasForeignKey<OrderDetails>(o => o.OrderDate).HasPrincipalKey<DateDetails>(o => o.Date);
Expand Down
Loading

0 comments on commit c4c9c1f

Please sign in to comment.