Skip to content

Commit

Permalink
Target FKs to the derived table in TPT if possible (#22039)
Browse files Browse the repository at this point in the history
Fixes #21975
  • Loading branch information
AndriySvyryd committed Aug 13, 2020
1 parent 33ad238 commit acd5a1f
Show file tree
Hide file tree
Showing 14 changed files with 327 additions and 89 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
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,42 @@ public static bool AreCompatible(
in StoreObjectIdentifier storeObject,
bool shouldThrow)
{
var principalType = foreignKey.PrincipalEntityType;
var principalType = foreignKey.PrincipalKey.IsPrimaryKey()
? foreignKey.PrincipalEntityType
: foreignKey.PrincipalKey.DeclaringEntityType;
var principalTable = StoreObjectIdentifier.Create(principalType, StoreObjectType.Table);
var duplicatePrincipalType = duplicateForeignKey.PrincipalEntityType;

var duplicatePrincipalType = duplicateForeignKey.PrincipalKey.IsPrimaryKey()
? duplicateForeignKey.PrincipalEntityType
: duplicateForeignKey.PrincipalKey.DeclaringEntityType;
var duplicatePrincipalTable = StoreObjectIdentifier.Create(duplicatePrincipalType, StoreObjectType.Table);
if (principalTable != duplicatePrincipalTable)

var columnNames = foreignKey.Properties.GetColumnNames(storeObject);
var duplicateColumnNames = duplicateForeignKey.Properties.GetColumnNames(storeObject);
if (columnNames == null
|| duplicateColumnNames == null)
{
if (shouldThrow)
{
throw new InvalidOperationException(
RelationalStrings.DuplicateForeignKeyTableMismatch(
foreignKey.Properties.Format(),
foreignKey.DeclaringEntityType.DisplayName(),
duplicateForeignKey.Properties.Format(),
duplicateForeignKey.DeclaringEntityType.DisplayName(),
foreignKey.GetConstraintName(storeObject, principalTable.Value),
foreignKey.DeclaringEntityType.GetSchemaQualifiedTableName(),
duplicateForeignKey.DeclaringEntityType.GetSchemaQualifiedTableName()));
}

return false;
}

var principalColumns = foreignKey.PrincipalKey.Properties.GetColumnNames(principalTable.Value);
var duplicatePrincipalColumns = duplicateForeignKey.PrincipalKey.Properties.GetColumnNames(principalTable.Value);
if (principalTable != duplicatePrincipalTable
|| principalColumns == null
|| duplicatePrincipalColumns == null)
{
if (shouldThrow)
{
Expand All @@ -53,7 +84,7 @@ public static bool AreCompatible(
return false;
}

if (!SameColumnNames(foreignKey.Properties, duplicateForeignKey.Properties, storeObject))
if (!columnNames.SequenceEqual(duplicateColumnNames))
{
if (shouldThrow)
{
Expand All @@ -72,7 +103,7 @@ public static bool AreCompatible(
return false;
}

if (!SameColumnNames(foreignKey.PrincipalKey.Properties, duplicateForeignKey.PrincipalKey.Properties, principalTable.Value))
if (!principalColumns.SequenceEqual(duplicatePrincipalColumns))
{
if (shouldThrow)
{
Expand Down Expand Up @@ -128,12 +159,6 @@ public static bool AreCompatible(
}

return true;

static bool SameColumnNames(
IReadOnlyList<IProperty> properties,
IReadOnlyList<IProperty> duplicateProperties,
in StoreObjectIdentifier storeObject)
=> properties.GetColumnNames(storeObject).SequenceEqual(duplicateProperties.GetColumnNames(storeObject));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,28 @@ public static bool AreCompatible(
in StoreObjectIdentifier storeObject,
bool shouldThrow)
{
if (!index.Properties.GetColumnNames(storeObject)
.SequenceEqual(duplicateIndex.Properties.GetColumnNames(storeObject)))
var columnNames = index.Properties.GetColumnNames(storeObject);
var duplicateColumnNames = duplicateIndex.Properties.GetColumnNames(storeObject);
if (columnNames == null
|| duplicateColumnNames == null)
{
if (shouldThrow)
{
throw new InvalidOperationException(
RelationalStrings.DuplicateIndexTableMismatch(
index.Properties.Format(),
index.DeclaringEntityType.DisplayName(),
duplicateIndex.Properties.Format(),
duplicateIndex.DeclaringEntityType.DisplayName(),
index.GetDatabaseName(storeObject),
index.DeclaringEntityType.GetSchemaQualifiedTableName(),
duplicateIndex.DeclaringEntityType.GetSchemaQualifiedTableName()));
}

return false;
}

if (!columnNames.SequenceEqual(duplicateColumnNames))
{
if (shouldThrow)
{
Expand Down
42 changes: 31 additions & 11 deletions src/EFCore.Relational/Metadata/Internal/RelationalKeyExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,21 +29,41 @@ public static bool AreCompatible(
in StoreObjectIdentifier storeObject,
bool shouldThrow)
{
if (!key.Properties.GetColumnNames(storeObject)
.SequenceEqual(duplicateKey.Properties.GetColumnNames(storeObject)))
var columnNames = key.Properties.GetColumnNames(storeObject);
var duplicateColumnNames = duplicateKey.Properties.GetColumnNames(storeObject);
if (columnNames == null
|| duplicateColumnNames == null)
{
if (shouldThrow)
{
throw new InvalidOperationException(
RelationalStrings.DuplicateKeyColumnMismatch(
key.Properties.Format(),
key.DeclaringEntityType.DisplayName(),
duplicateKey.Properties.Format(),
duplicateKey.DeclaringEntityType.DisplayName(),
key.DeclaringEntityType.GetSchemaQualifiedTableName(),
key.GetName(storeObject),
key.Properties.FormatColumns(storeObject),
duplicateKey.Properties.FormatColumns(storeObject)));
RelationalStrings.DuplicateKeyTableMismatch(
key.Properties.Format(),
key.DeclaringEntityType.DisplayName(),
duplicateKey.Properties.Format(),
duplicateKey.DeclaringEntityType.DisplayName(),
key.GetName(storeObject),
key.DeclaringEntityType.GetSchemaQualifiedTableName(),
duplicateKey.DeclaringEntityType.GetSchemaQualifiedTableName()));
}

return false;
}

if (!columnNames.SequenceEqual(duplicateColumnNames))
{
if (shouldThrow)
{
throw new InvalidOperationException(
RelationalStrings.DuplicateKeyColumnMismatch(
key.Properties.Format(),
key.DeclaringEntityType.DisplayName(),
duplicateKey.Properties.Format(),
duplicateKey.DeclaringEntityType.DisplayName(),
key.DeclaringEntityType.GetSchemaQualifiedTableName(),
key.GetName(storeObject),
key.Properties.FormatColumns(storeObject),
duplicateKey.Properties.FormatColumns(storeObject)));
}

return false;
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
Loading

0 comments on commit acd5a1f

Please sign in to comment.