Skip to content

Commit

Permalink
Map check constraints only to the tables for which they are declared
Browse files Browse the repository at this point in the history
Return null name for constraints on entity types not mapped to a table
Change calls from Array.Empty to Enumerable.Empty

Fixes #24219
  • Loading branch information
AndriySvyryd authored Mar 2, 2022
1 parent f7be8b3 commit 786cb40
Show file tree
Hide file tree
Showing 30 changed files with 300 additions and 79 deletions.
2 changes: 1 addition & 1 deletion src/EFCore.Cosmos/Query/Internal/RandomTranslator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public virtual SqlExpression Translate(
=> MethodInfo.Equals(method)
? _sqlExpressionFactory.Function(
"RAND",
Array.Empty<SqlExpression>(),
Enumerable.Empty<SqlExpression>(),
method.ReturnType)
: null;
}
Original file line number Diff line number Diff line change
Expand Up @@ -931,7 +931,8 @@ protected virtual void GenerateCheckConstraint(
.Append(", ")
.Append(Code.Literal(checkConstraint.Sql));

if (checkConstraint.Name != (checkConstraint.GetDefaultName() ?? checkConstraint.ModelName))
if (checkConstraint.Name != null
&& checkConstraint.Name != (checkConstraint.GetDefaultName() ?? checkConstraint.ModelName))
{
stringBuilder
.Append(", c => c.HasName(")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,7 @@ private void GenerateIndex(IIndex index)

var lines = new List<string>
{
$".{nameof(EntityTypeBuilder.HasIndex)}({_code.Lambda(index.Properties, "e")}, {_code.Literal(index.GetDatabaseName())})"
$".{nameof(EntityTypeBuilder.HasIndex)}({_code.Lambda(index.Properties, "e")}, {_code.Literal(index.GetDatabaseName()!)})"
};
annotations.Remove(RelationalAnnotationNames.Name);

Expand Down Expand Up @@ -845,7 +845,7 @@ private void GenerateManyToMany(ISkipNavigation skipNavigation)
_annotationCodeGenerator.RemoveAnnotationsHandledByConventions(index, indexAnnotations);

lines.Add(
$"j.{nameof(EntityTypeBuilder.HasIndex)}({_code.Literal(index.Properties.Select(e => e.Name).ToArray())}, {_code.Literal(index.GetDatabaseName())})");
$"j.{nameof(EntityTypeBuilder.HasIndex)}({_code.Literal(index.Properties.Select(e => e.Name).ToArray())}, {_code.Literal(index.GetDatabaseName()!)})");
indexAnnotations.Remove(RelationalAnnotationNames.Name);

if (index.IsUnique)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public static class RelationalEntityTypeExtensions
return entityType.GetRootType().GetTableName();
}

return (entityType as IConventionEntityType)?.GetViewNameConfigurationSource() == null
return ((entityType as IConventionEntityType)?.GetViewNameConfigurationSource() == null)
&& ((entityType as IConventionEntityType)?.GetFunctionNameConfigurationSource() == null)
#pragma warning disable CS0618 // Type or member is obsolete
&& ((entityType as IConventionEntityType)?.GetDefiningQueryConfigurationSource() == null)
Expand Down Expand Up @@ -254,7 +254,7 @@ public static void SetSchema(this IMutableEntityType entityType, string? value)
public static IEnumerable<ITableMappingBase> GetDefaultMappings(this IEntityType entityType)
=> (IEnumerable<ITableMappingBase>?)entityType.FindRuntimeAnnotationValue(
RelationalAnnotationNames.DefaultMappings)
?? Array.Empty<ITableMappingBase>();
?? Enumerable.Empty<ITableMappingBase>();

/// <summary>
/// Returns the tables to which the entity type is mapped.
Expand All @@ -264,7 +264,7 @@ public static IEnumerable<ITableMappingBase> GetDefaultMappings(this IEntityType
public static IEnumerable<ITableMapping> GetTableMappings(this IEntityType entityType)
=> (IEnumerable<ITableMapping>?)entityType.FindRuntimeAnnotationValue(
RelationalAnnotationNames.TableMappings)
?? Array.Empty<ITableMapping>();
?? Enumerable.Empty<ITableMapping>();

/// <summary>
/// Returns the name of the view to which the entity type is mapped or <see langword="null" /> if not mapped to a view.
Expand All @@ -286,7 +286,7 @@ public static IEnumerable<ITableMapping> GetTableMappings(this IEntityType entit

return ((entityType as IConventionEntityType)?.GetFunctionNameConfigurationSource() == null)
#pragma warning disable CS0618 // Type or member is obsolete
&& (entityType as IConventionEntityType)?.GetDefiningQueryConfigurationSource() == null
&& ((entityType as IConventionEntityType)?.GetDefiningQueryConfigurationSource() == null)
#pragma warning restore CS0618 // Type or member is obsolete
&& ((entityType as IConventionEntityType)?.GetSqlQueryConfigurationSource() == null)
? GetDefaultViewName(entityType)
Expand Down Expand Up @@ -428,7 +428,7 @@ public static void SetViewSchema(this IMutableEntityType entityType, string? val
public static IEnumerable<IViewMapping> GetViewMappings(this IEntityType entityType)
=> (IEnumerable<IViewMapping>?)entityType.FindRuntimeAnnotationValue(
RelationalAnnotationNames.ViewMappings)
?? Array.Empty<IViewMapping>();
?? Enumerable.Empty<IViewMapping>();

/// <summary>
/// Gets the default SQL query name that would be used for this entity type when mapped using
Expand Down Expand Up @@ -493,7 +493,7 @@ public static void SetSqlQuery(this IMutableEntityType entityType, string? name)
public static IEnumerable<ISqlQueryMapping> GetSqlQueryMappings(this IEntityType entityType)
=> (IEnumerable<ISqlQueryMapping>?)entityType.FindRuntimeAnnotationValue(
RelationalAnnotationNames.SqlQueryMappings)
?? Array.Empty<ISqlQueryMapping>();
?? Enumerable.Empty<ISqlQueryMapping>();

/// <summary>
/// Returns the name of the function to which the entity type is mapped or <see langword="null" /> if not mapped to a function.
Expand Down Expand Up @@ -549,7 +549,7 @@ public static void SetFunctionName(this IMutableEntityType entityType, string? n
public static IEnumerable<IFunctionMapping> GetFunctionMappings(this IEntityType entityType)
=> (IEnumerable<IFunctionMapping>?)entityType.FindRuntimeAnnotationValue(
RelationalAnnotationNames.FunctionMappings)
?? Array.Empty<IFunctionMapping>();
?? Enumerable.Empty<IFunctionMapping>();

/// <summary>
/// Finds an <see cref="IReadOnlyCheckConstraint" /> with the given name.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@ public static class RelationalForeignKeyExtensions
/// <returns>The foreign key constraint name.</returns>
public static string? GetConstraintName(this IReadOnlyForeignKey foreignKey)
{
var tableName = foreignKey.DeclaringEntityType.GetTableName();
if (tableName == null)
{
return null;
}

var annotation = foreignKey.FindAnnotation(RelationalAnnotationNames.Name);
return annotation != null
? (string?)annotation.Value
Expand All @@ -39,6 +45,12 @@ public static class RelationalForeignKeyExtensions
in StoreObjectIdentifier storeObject,
in StoreObjectIdentifier principalStoreObject)
{
if (storeObject.StoreObjectType != StoreObjectType.Table
|| principalStoreObject.StoreObjectType != StoreObjectType.Table)
{
return null;
}

var annotation = foreignKey.FindAnnotation(RelationalAnnotationNames.Name);
return annotation != null
? (string?)annotation.Value
Expand All @@ -50,10 +62,15 @@ public static class RelationalForeignKeyExtensions
/// </summary>
/// <param name="foreignKey">The foreign key.</param>
/// <returns>The default constraint name that would be used for this foreign key.</returns>
public static string GetDefaultName(this IReadOnlyForeignKey foreignKey)
public static string? GetDefaultName(this IReadOnlyForeignKey foreignKey)
{
var tableName = foreignKey.DeclaringEntityType.GetTableName();
var principalTableName = foreignKey.PrincipalEntityType.GetTableName();
if (tableName == null
|| principalTableName == null)
{
return null;
}

var name = new StringBuilder()
.Append("FK_")
Expand All @@ -79,6 +96,12 @@ public static string GetDefaultName(this IReadOnlyForeignKey foreignKey)
in StoreObjectIdentifier storeObject,
in StoreObjectIdentifier principalStoreObject)
{
if (storeObject.StoreObjectType != StoreObjectType.Table
|| principalStoreObject.StoreObjectType != StoreObjectType.Table)
{
return null;
}

var propertyNames = foreignKey.Properties.GetColumnNames(storeObject);
var principalPropertyNames = foreignKey.PrincipalKey.Properties.GetColumnNames(principalStoreObject);
if (propertyNames == null
Expand Down
22 changes: 18 additions & 4 deletions src/EFCore.Relational/Extensions/RelationalIndexExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@ public static class RelationalIndexExtensions
/// </summary>
/// <param name="index">The index.</param>
/// <returns>The name of the index in the database.</returns>
public static string GetDatabaseName(this IReadOnlyIndex index)
=> (string?)index[RelationalAnnotationNames.Name]
public static string? GetDatabaseName(this IReadOnlyIndex index)
=> index.DeclaringEntityType.GetTableName() == null
? null
: (string?)index[RelationalAnnotationNames.Name]
?? index.Name
?? index.GetDefaultDatabaseName();

Expand All @@ -31,7 +33,9 @@ public static string GetDatabaseName(this IReadOnlyIndex index)
/// <param name="storeObject">The identifier of the store object.</param>
/// <returns>The name of the index in the database.</returns>
public static string? GetDatabaseName(this IReadOnlyIndex index, in StoreObjectIdentifier storeObject)
=> (string?)index[RelationalAnnotationNames.Name]
=> storeObject.StoreObjectType != StoreObjectType.Table
? null
: (string?)index[RelationalAnnotationNames.Name]
?? index.Name
?? index.GetDefaultDatabaseName(storeObject);

Expand All @@ -40,9 +44,14 @@ public static string GetDatabaseName(this IReadOnlyIndex index)
/// </summary>
/// <param name="index">The index.</param>
/// <returns>The default name that would be used for this index.</returns>
public static string GetDefaultDatabaseName(this IReadOnlyIndex index)
public static string? GetDefaultDatabaseName(this IReadOnlyIndex index)
{
var tableName = index.DeclaringEntityType.GetTableName();
if (tableName == null)
{
return null;
}

var baseName = new StringBuilder()
.Append("IX_")
.Append(tableName)
Expand All @@ -61,6 +70,11 @@ public static string GetDefaultDatabaseName(this IReadOnlyIndex index)
/// <returns>The default name that would be used for this index.</returns>
public static string? GetDefaultDatabaseName(this IReadOnlyIndex index, in StoreObjectIdentifier storeObject)
{
if (storeObject.StoreObjectType != StoreObjectType.Table)
{
return null;
}

var columnNames = index.Properties.GetColumnNames(storeObject);
if (columnNames == null)
{
Expand Down
35 changes: 31 additions & 4 deletions src/EFCore.Relational/Extensions/RelationalKeyExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@ public static class RelationalKeyExtensions
/// <param name="key">The key.</param>
/// <returns>The key constraint name for this key.</returns>
public static string? GetName(this IReadOnlyKey key)
=> key.GetName(StoreObjectIdentifier.Table(key.DeclaringEntityType.GetTableName()!, key.DeclaringEntityType.GetSchema()));
{
var table = StoreObjectIdentifier.Create(key.DeclaringEntityType, StoreObjectType.Table);
return !table.HasValue ? null : key.GetName(table.Value);
}

/// <summary>
/// Returns the key constraint name for this key for a particular table.
Expand All @@ -29,17 +32,36 @@ public static class RelationalKeyExtensions
/// <param name="storeObject">The identifier of the containing store object.</param>
/// <returns>The key constraint name for this key.</returns>
public static string? GetName(this IReadOnlyKey key, in StoreObjectIdentifier storeObject)
=> (string?)key[RelationalAnnotationNames.Name]
?? key.GetDefaultName(storeObject);
{
if (storeObject.StoreObjectType != StoreObjectType.Table)
{
return null;
}

foreach (var containingType in key.DeclaringEntityType.GetDerivedTypesInclusive())
{
if (StoreObjectIdentifier.Create(containingType, storeObject.StoreObjectType) == storeObject)
{
return (string?)key[RelationalAnnotationNames.Name] ?? key.GetDefaultName(storeObject);
}
}

return null;
}

/// <summary>
/// Returns the default key constraint name that would be used for this key.
/// </summary>
/// <param name="key">The key.</param>
/// <returns>The default key constraint name that would be used for this key.</returns>
public static string GetDefaultName(this IReadOnlyKey key)
public static string? GetDefaultName(this IReadOnlyKey key)
{
var tableName = key.DeclaringEntityType.GetTableName();
if (tableName == null)
{
return null;
}

var name = key.IsPrimaryKey()
? "PK_" + tableName
: new StringBuilder()
Expand All @@ -60,6 +82,11 @@ public static string GetDefaultName(this IReadOnlyKey key)
/// <returns>The default key constraint name that would be used for this key.</returns>
public static string? GetDefaultName(this IReadOnlyKey key, in StoreObjectIdentifier storeObject)
{
if (storeObject.StoreObjectType != StoreObjectType.Table)
{
return null;
}

string? name;
if (key.IsPrimaryKey())
{
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore.Relational/Metadata/IMutableCheckConstraint.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,5 @@ public interface IMutableCheckConstraint : IReadOnlyCheckConstraint, IMutableAnn
/// <summary>
/// Gets or sets the name of the check constraint in the database.
/// </summary>
new string Name { get; set; }
new string? Name { get; set; }
}
8 changes: 5 additions & 3 deletions src/EFCore.Relational/Metadata/IReadOnlyCheckConstraint.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public interface IReadOnlyCheckConstraint : IReadOnlyAnnotatable
/// <summary>
/// Gets the database name of the check constraint.
/// </summary>
string Name { get; }
string? Name { get; }

/// <summary>
/// Returns the default database name that would be used for this check constraint.
Expand All @@ -43,8 +43,10 @@ public interface IReadOnlyCheckConstraint : IReadOnlyAnnotatable
/// </summary>
/// <param name="storeObject">The identifier of the store object.</param>
/// <returns>The default name that would be used for this check constraint.</returns>
string GetDefaultName(in StoreObjectIdentifier storeObject)
=> Uniquifier.Truncate(ModelName, EntityType.Model.GetMaxIdentifierLength());
string? GetDefaultName(in StoreObjectIdentifier storeObject)
=> storeObject.StoreObjectType == StoreObjectType.Table
? Uniquifier.Truncate(ModelName, EntityType.Model.GetMaxIdentifierLength())
: null;

/// <summary>
/// Gets the entity type on which this check constraint is defined.
Expand Down
4 changes: 1 addition & 3 deletions src/EFCore.Relational/Metadata/ITable.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,7 @@ public interface ITable : ITableBase
/// <summary>
/// Gets the check constraints for this table.
/// </summary>
IEnumerable<ICheckConstraint> CheckConstraints
=> EntityTypeMappings.SelectMany(m => m.EntityType.GetDeclaredCheckConstraints())
.Distinct((x, y) => x!.Name == y!.Name);
IEnumerable<ICheckConstraint> CheckConstraints { get; }

/// <summary>
/// Gets the comment for this table.
Expand Down
23 changes: 20 additions & 3 deletions src/EFCore.Relational/Metadata/Internal/CheckConstraint.cs
Original file line number Diff line number Diff line change
Expand Up @@ -280,15 +280,32 @@ public override bool IsReadOnly
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public virtual string Name
public virtual string? Name
{
get => _name ?? ((IReadOnlyCheckConstraint)this).GetDefaultName() ?? ModelName;
get => EntityType.GetTableName() == null
? null
: _name ?? ((IReadOnlyCheckConstraint)this).GetDefaultName();
set => SetName(value, ConfigurationSource.Explicit);
}

/// <inheritdoc />
public virtual string? GetName(in StoreObjectIdentifier storeObject)
=> _name ?? ((IReadOnlyCheckConstraint)this).GetDefaultName(storeObject) ?? ModelName;
{
if (storeObject.StoreObjectType != StoreObjectType.Table)
{
return null;
}

foreach (var containingType in EntityType.GetDerivedTypesInclusive())
{
if (StoreObjectIdentifier.Create(containingType, storeObject.StoreObjectType) == storeObject)
{
return _name ?? ((IReadOnlyCheckConstraint)this).GetDefaultName(storeObject);
}
}

return null;
}

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down
Loading

0 comments on commit 786cb40

Please sign in to comment.