Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix for 21089. Allow multiple indexes on the same properties. #21115

Merged
merged 6 commits into from
Jun 4, 2020
Merged

Fix for 21089. Allow multiple indexes on the same properties. #21115

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2d99ee5
Fix for 21089. Allow multiple indexes on the same properties.
lajones Jun 1, 2020
fb9f9da
Updated per review comments and design meeting.
lajones Jun 3, 2020
1b22662
Missed small docs change.
lajones Jun 4, 2020
87b31a1
More updates noticed during review.
lajones Jun 4, 2020
cb4d8f0
More updates from review.
lajones Jun 4, 2020
9f1c1e2
Update to make looking up index quicker.
lajones Jun 4, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 19 additions & 12 deletions src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -749,21 +749,26 @@ protected virtual void GenerateIndex(
stringBuilder
.AppendLine()
.Append(builderName)
.Append(".HasIndex(")
.Append(string.Join(", ", index.Properties.Select(p => Code.Literal(p.Name))))
.Append(")");
.Append(".HasIndex(");

using (stringBuilder.Indent())
if (index.Name == null)
{
if (index.Name != null)
{
stringBuilder
.AppendLine()
.Append(".HasName(")
.Append(Code.Literal(index.Name))
.Append(")");
}
stringBuilder
.Append(string.Join(", ", index.Properties.Select(p => Code.Literal(p.Name))));
}
else
{
stringBuilder
.Append("new[] { ")
.Append(string.Join(", ", index.Properties.Select(p => Code.Literal(p.Name))))
.Append(" }, ")
.Append(Code.Literal(index.Name));
}

stringBuilder.Append(")");

using (stringBuilder.Indent())
{
if (index.IsUnique)
{
stringBuilder
Expand Down Expand Up @@ -791,6 +796,8 @@ protected virtual void GenerateIndexAnnotations(
annotations,
CSharpModelGenerator.IgnoredIndexAnnotations);

GenerateFluentApiForAnnotation(
ref annotations, RelationalAnnotationNames.Name, nameof(RelationalIndexBuilderExtensions.HasDatabaseName), stringBuilder);
GenerateFluentApiForAnnotation(
ref annotations, RelationalAnnotationNames.Filter, nameof(RelationalIndexBuilderExtensions.HasFilter), stringBuilder);

Expand Down
15 changes: 6 additions & 9 deletions src/EFCore.Design/Scaffolding/Internal/CSharpDbContextGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -586,22 +586,19 @@ private void GenerateTableName(IEntityType entityType)

private void GenerateIndex(IIndex index)
{
var lines = new List<string> { $".{nameof(EntityTypeBuilder.HasIndex)}({_code.Lambda(index.Properties)})" };

var annotations = index.GetAnnotations().ToList();

var lines = new List<string> {
$".{nameof(EntityTypeBuilder.HasIndex)}" +
$"({_code.Lambda(index.Properties)}, " +
$"{_code.Literal(index.GetDatabaseName())})" };
lajones marked this conversation as resolved.
Show resolved Hide resolved
RemoveAnnotation(ref annotations, RelationalAnnotationNames.Name);

foreach (var annotation in CSharpModelGenerator.IgnoredIndexAnnotations)
{
RemoveAnnotation(ref annotations, annotation);
}

if (index.Name != null)
{
lines.Add(
$".{nameof(IndexBuilder.HasName)}" +
$"({_code.Literal(index.GetDatabaseName())})");
}

if (index.IsUnique)
{
lines.Add($".{nameof(IndexBuilder.IsUnique)}()");
Expand Down
28 changes: 9 additions & 19 deletions src/EFCore.Design/Scaffolding/Internal/RelationalScaffoldingModelFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -617,14 +617,7 @@ protected virtual IndexBuilder VisitUniqueConstraint(
}

var propertyNames = uniqueConstraint.Columns.Select(GetPropertyName).ToArray();
var indexBuilder = builder.HasIndex(propertyNames).IsUnique();

if (!string.IsNullOrEmpty(uniqueConstraint.Name)
&& uniqueConstraint.Name != indexBuilder.Metadata.GetDefaultDatabaseName())
{
indexBuilder.HasName(uniqueConstraint.Name);
}

var indexBuilder = builder.HasIndex(propertyNames, uniqueConstraint.Name).IsUnique();
indexBuilder.Metadata.AddAnnotations(uniqueConstraint.GetAnnotations());

return indexBuilder;
Expand Down Expand Up @@ -674,20 +667,14 @@ protected virtual IndexBuilder VisitIndex([NotNull] EntityTypeBuilder builder, [
}

var propertyNames = index.Columns.Select(GetPropertyName).ToArray();
var indexBuilder = builder.HasIndex(propertyNames)
var indexBuilder = builder.HasIndex(propertyNames, index.Name)
.IsUnique(index.IsUnique);

if (index.Filter != null)
{
indexBuilder.HasFilter(index.Filter);
}

if (!string.IsNullOrEmpty(index.Name)
&& index.Name != indexBuilder.Metadata.GetDefaultDatabaseName())
{
indexBuilder.HasName(index.Name);
}

indexBuilder.Metadata.AddAnnotations(index.GetAnnotations());

return indexBuilder;
Expand Down Expand Up @@ -803,8 +790,10 @@ protected virtual IMutableForeignKey VisitForeignKey([NotNull] ModelBuilder mode
var principalKey = principalEntityType.FindKey(principalProperties);
if (principalKey == null)
{
var index = principalEntityType.FindIndex(principalProperties.AsReadOnly());
if (index?.IsUnique == true)
var index = principalEntityType.GetIndexes()
.Where(i => i.Properties.SequenceEqual(principalProperties) && i.IsUnique)
.FirstOrDefault();
if (index != null)
{
// ensure all principal properties are non-nullable even if the columns
// are nullable on the database. EF's concept of a key requires this.
Expand Down Expand Up @@ -844,9 +833,10 @@ protected virtual IMutableForeignKey VisitForeignKey([NotNull] ModelBuilder mode
dependentProperties, principalKey, principalEntityType);

var dependentKey = dependentEntityType.FindKey(dependentProperties);
var dependentIndex = dependentEntityType.FindIndex(dependentProperties);
var dependentIndexes = dependentEntityType.GetIndexes()
.Where(i => i.Properties.SequenceEqual(dependentProperties));
newForeignKey.IsUnique = dependentKey != null
|| dependentIndex?.IsUnique == true;
|| dependentIndexes.Any(i => i.IsUnique);

if (!string.IsNullOrEmpty(foreignKey.Name)
&& foreignKey.Name != newForeignKey.GetDefaultName())
Expand Down
74 changes: 67 additions & 7 deletions src/EFCore.Relational/Extensions/RelationalIndexBuilderExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,36 @@ public static class RelationalIndexBuilderExtensions
/// <param name="indexBuilder"> The builder for the index being configured. </param>
/// <param name="name"> The name of the index. </param>
/// <returns> A builder to further configure the index. </returns>
[Obsolete("Use IndexBuilder.HasName() instead.")]
public static IndexBuilder HasDatabaseName([NotNull] this IndexBuilder indexBuilder, [CanBeNull] string name)
{
indexBuilder.Metadata.SetDatabaseName(name);

return indexBuilder;
}

/// <summary>
/// Configures the name of the index in the database when targeting a relational database.
/// </summary>
/// <param name="indexBuilder"> The builder for the index being configured. </param>
/// <param name="name"> The name of the index. </param>
/// <returns> A builder to further configure the index. </returns>
[Obsolete("Use HasDatabaseName() instead.")]
public static IndexBuilder HasName([NotNull] this IndexBuilder indexBuilder, [CanBeNull] string name)
=> indexBuilder.HasName(name);
=> HasDatabaseName(indexBuilder, name);

/// <summary>
/// Configures the name of the index in the database when targeting a relational database.
/// </summary>
/// <typeparam name="TEntity"> The entity type being configured. </typeparam>
/// <param name="indexBuilder"> The builder for the index being configured. </param>
/// <param name="name"> The name of the index. </param>
/// <returns> A builder to further configure the index. </returns>
public static IndexBuilder<TEntity> HasDatabaseName<TEntity>([NotNull] this IndexBuilder<TEntity> indexBuilder, [CanBeNull] string name)
{
indexBuilder.Metadata.SetDatabaseName(name);

return indexBuilder;
}

/// <summary>
/// Configures the name of the index in the database when targeting a relational database.
Expand All @@ -32,7 +59,7 @@ public static IndexBuilder HasName([NotNull] this IndexBuilder indexBuilder, [Ca
/// <param name="indexBuilder"> The builder for the index being configured. </param>
/// <param name="name"> The name of the index. </param>
/// <returns> A builder to further configure the index. </returns>
[Obsolete("Use IndexBuilder<TEntity>.HasName() instead.")]
[Obsolete("Use HasDatabaseName() instead.")]
public static IndexBuilder<TEntity> HasName<TEntity>([NotNull] this IndexBuilder<TEntity> indexBuilder, [CanBeNull] string name)
=> indexBuilder.HasName(name);

Expand All @@ -46,10 +73,43 @@ public static IndexBuilder<TEntity> HasName<TEntity>([NotNull] this IndexBuilder
/// The same builder instance if the configuration was applied,
/// <see langword="null" /> otherwise.
/// </returns>
[Obsolete("Use IConventionIndexBuilder.HasName() instead.")]
public static IConventionIndexBuilder HasDatabaseName(
[NotNull] this IConventionIndexBuilder indexBuilder, [CanBeNull] string name, bool fromDataAnnotation = false)
{
if (indexBuilder.CanSetDatabaseName(name, fromDataAnnotation))
{
indexBuilder.Metadata.SetDatabaseName(name, fromDataAnnotation);
return indexBuilder;
}

return null;
}

/// <summary>
/// Configures the name of the index in the database when targeting a relational database.
/// </summary>
/// <param name="indexBuilder"> The builder for the index being configured. </param>
/// <param name="name"> The name of the index. </param>
/// <param name="fromDataAnnotation"> Indicates whether the configuration was specified using a data annotation. </param>
/// <returns>
/// The same builder instance if the configuration was applied,
/// <see langword="null" /> otherwise.
/// </returns>
[Obsolete("Use HasDatabaseName() instead.")]
public static IConventionIndexBuilder HasName(
[NotNull] this IConventionIndexBuilder indexBuilder, [CanBeNull] string name, bool fromDataAnnotation = false)
=> indexBuilder.HasName(name, fromDataAnnotation);
=> indexBuilder.HasDatabaseName(name, fromDataAnnotation);

/// <summary>
/// Returns a value indicating whether the given name can be set for the index.
/// </summary>
/// <param name="indexBuilder"> The builder for the index being configured. </param>
/// <param name="name"> The name of the index. </param>
/// <param name="fromDataAnnotation"> Indicates whether the configuration was specified using a data annotation. </param>
/// <returns> <see langword="true" /> if the given name can be set for the index. </returns>
public static bool CanSetDatabaseName(
[NotNull] this IConventionIndexBuilder indexBuilder, [CanBeNull] string name, bool fromDataAnnotation = false)
=> indexBuilder.CanSetAnnotation(RelationalAnnotationNames.Name, name, fromDataAnnotation);

/// <summary>
/// Returns a value indicating whether the given name can be set for the index.
Expand All @@ -58,10 +118,10 @@ public static IConventionIndexBuilder HasName(
/// <param name="name"> The name of the index. </param>
/// <param name="fromDataAnnotation"> Indicates whether the configuration was specified using a data annotation. </param>
/// <returns> <see langword="true" /> if the given name can be set for the index. </returns>
[Obsolete("Use IConventionIndexBuilder.CanSetName() instead.")]
[Obsolete("Use CanSetDatabaseName() instead.")]
public static bool CanSetName(
[NotNull] this IConventionIndexBuilder indexBuilder, [CanBeNull] string name, bool fromDataAnnotation = false)
=> indexBuilder.CanSetName(name, fromDataAnnotation);
=> CanSetDatabaseName(indexBuilder, name, fromDataAnnotation);

/// <summary>
/// Configures the filter expression for the index.
Expand Down
77 changes: 59 additions & 18 deletions src/EFCore.Relational/Extensions/RelationalIndexExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,34 +19,38 @@ namespace Microsoft.EntityFrameworkCore
public static class RelationalIndexExtensions
{
/// <summary>
/// Returns the name for this index.
/// Returns the name of the index on the database.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in the database or on the database?

/// </summary>
/// <param name="index"> The index. </param>
/// <returns> The name for this index. </returns>
/// <returns> The name of the index on the database. </returns>
public static string GetDatabaseName([NotNull] this IIndex index)
=> index.Name ?? index.GetDefaultDatabaseName();
=> (string)index[RelationalAnnotationNames.Name]
?? index.Name
?? index.GetDefaultDatabaseName();

/// <summary>
/// Returns the name for this index.
/// Returns the name of the index on the database.
/// </summary>
/// <param name="index"> The index. </param>
/// <returns> The name for this index. </returns>
/// <returns> The name of the index on the database. </returns>
[Obsolete("Use GetDatabaseName() instead")]
public static string GetName([NotNull] this IIndex index)
=> GetDatabaseName(index);

/// <summary>
/// Returns the name for this index.
/// Returns the name of the index on the database.
/// </summary>
/// <param name="index"> The index. </param>
/// <param name="tableName"> The table name. </param>
/// <param name="schema"> The schema. </param>
/// <returns> The name for this index. </returns>
/// <returns> The name of the index on the database. </returns>
public static string GetDatabaseName(
[NotNull] this IIndex index,
[NotNull] string tableName,
[CanBeNull] string schema)
=> index.Name ?? index.GetDefaultDatabaseName(tableName, schema);
=> (string)index[RelationalAnnotationNames.Name]
?? index.Name
?? index.GetDefaultDatabaseName(tableName, schema);

/// <summary>
/// Returns the default name that would be used for this index.
Expand Down Expand Up @@ -123,33 +127,70 @@ public static string GetDefaultDatabaseName(
}

/// <summary>
/// Sets the index name.
/// Sets the name of the index on the database.
/// </summary>
/// <param name="index"> The index. </param>
/// <param name="name"> The value to set. </param>
[Obsolete("Use IMutableIndex.Name instead.")]
public static void SetDatabaseName([NotNull] this IMutableIndex index, [CanBeNull] string name)
{
index.SetOrRemoveAnnotation(
RelationalAnnotationNames.Name,
Check.NullButNotEmpty(name, nameof(name)));
}

/// <summary>
/// Sets the name of the index on the database.
/// </summary>
/// <param name="index"> The index. </param>
/// <param name="name"> The value to set. </param>
[Obsolete("Use SetDatabaseName() instead.")]
public static void SetName([NotNull] this IMutableIndex index, [CanBeNull] string name)
=> index.Name = Check.NullButNotEmpty(name, nameof(name));
=> SetDatabaseName(index, name);

/// <summary>
/// Sets the name of the index on the database.
/// </summary>
/// <param name="index"> The index. </param>
/// <param name="name"> The value to set. </param>
/// <param name="fromDataAnnotation"> Indicates whether the configuration was specified using a data annotation. </param>
/// <returns> The configured value. </returns>
public static string SetDatabaseName([NotNull] this IConventionIndex index, [CanBeNull] string name, bool fromDataAnnotation = false)
{
index.SetOrRemoveAnnotation(
RelationalAnnotationNames.Name,
Check.NullButNotEmpty(name, nameof(name)),
fromDataAnnotation);

return name;
}

/// <summary>
/// Sets the index name.
/// Sets the name of the index on the database.
/// </summary>
/// <param name="index"> The index. </param>
/// <param name="name"> The value to set. </param>
/// <param name="fromDataAnnotation"> Indicates whether the configuration was specified using a data annotation. </param>
/// <returns> The configured value. </returns>
[Obsolete("Use IConventionIndex.SetName() instead.")]
[Obsolete("Use SetDatabaseName() instead.")]
public static string SetName([NotNull] this IConventionIndex index, [CanBeNull] string name, bool fromDataAnnotation = false)
=> index.SetName(Check.NullButNotEmpty(name, nameof(name)), fromDataAnnotation);
=> SetDatabaseName(index, name, fromDataAnnotation);

/// <summary>
/// Gets the <see cref="ConfigurationSource" /> for the name of the index on the database.
/// </summary>
/// <param name="index"> The index. </param>
/// <returns> The <see cref="ConfigurationSource" /> for the name of the index on the database. </returns>
public static ConfigurationSource? GetDatabaseNameConfigurationSource([NotNull] this IConventionIndex index)
=> index.FindAnnotation(RelationalAnnotationNames.Name)?.GetConfigurationSource();

/// <summary>
/// Gets the <see cref="ConfigurationSource" /> for the index name.
/// Gets the <see cref="ConfigurationSource" /> for the name of the index on the database.
/// </summary>
/// <param name="index"> The index. </param>
/// <returns> The <see cref="ConfigurationSource" /> for the index name. </returns>
[Obsolete("Use IConventionIndex.GetNameConfigurationSource() instead.")]
/// <returns> The <see cref="ConfigurationSource" /> for the name of the index on the database. </returns>
[Obsolete("Use GetDatabaseNameConfigurationSource() instead.")]
public static ConfigurationSource? GetNameConfigurationSource([NotNull] this IConventionIndex index)
=> index.GetNameConfigurationSource();
=> GetDatabaseNameConfigurationSource(index);

/// <summary>
/// Returns the index filter expression.
Expand Down
4 changes: 2 additions & 2 deletions src/EFCore.Relational/Metadata/Conventions/SharedTableConvention.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,10 +349,10 @@ protected virtual bool AreCompatible(
private static string TryUniquify<T>(
IConventionIndex index, string indexName, Dictionary<string, T> indexes, int maxLength)
{
if (index.Builder.CanSetName(null))
if (index.Builder.CanSetDatabaseName(null))
{
indexName = Uniquifier.Uniquify(indexName, indexes, maxLength);
index.Builder.HasName(indexName);
index.Builder.HasDatabaseName(indexName);
return indexName;
}

Expand Down
3 changes: 2 additions & 1 deletion src/EFCore/Extensions/ConventionEntityTypeExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,8 @@ public static IConventionProperty AddIndexerProperty(
}

/// <summary>
/// Gets the index defined on the given property. Returns null if no index is defined.
/// Gets the unnamed index defined on the given property. Returns null if no such index is defined.
lajones marked this conversation as resolved.
Show resolved Hide resolved
/// Note: named indexes will not be returned even if the list of properties matches.
/// </summary>
/// <param name="entityType"> The entity type. </param>
/// <param name="property"> The property to find the index on. </param>
Expand Down
Loading