Skip to content

Add support for INCLUDE clause in indexes #699

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

Merged
roji merged 5 commits into from
Nov 16, 2018
Merged

Add support for INCLUDE clause in indexes #699

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e967fda
Add support for INCLUDE clause in indexes
khellang Nov 14, 2018
1da5630
Use pattern matching
khellang Nov 15, 2018
6eda95c
Add XML docs
khellang Nov 15, 2018
81ccc5c
Add Npgsql-specific method to get generic IndexBuilder
khellang Nov 15, 2018
53bdc8c
Address PR feedback
khellang Nov 15, 2018
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
2 changes: 2 additions & 0 deletions src/EFCore.PG/Design/Internal/NpgsqlAnnotationCodeGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,8 @@ public override MethodCallCodeFragment GenerateFluentApi(IIndex index, IAnnotati
return new MethodCallCodeFragment(nameof(NpgsqlIndexBuilderExtensions.ForNpgsqlHasMethod), annotation.Value);
if (annotation.Name == NpgsqlAnnotationNames.IndexOperators)
return new MethodCallCodeFragment(nameof(NpgsqlIndexBuilderExtensions.ForNpgsqlHasOperators), annotation.Value);
if (annotation.Name == NpgsqlAnnotationNames.IndexInclude)
Comment thread
austindrenski marked this conversation as resolved.
return new MethodCallCodeFragment(nameof(NpgsqlIndexBuilderExtensions.ForNpgsqlInclude), annotation.Value);

return null;
}
Expand Down
39 changes: 39 additions & 0 deletions src/EFCore.PG/Extensions/NpgsqlEntityTypeBuilderExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
using System;
using System.Collections.Generic;
using System.Linq.Expressions;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.Internal;
using Microsoft.EntityFrameworkCore.Metadata.Builders;
using Microsoft.EntityFrameworkCore.Metadata.Internal;
using Npgsql.EntityFrameworkCore.PostgreSQL.Utilities;

// ReSharper disable once CheckNamespace
Expand Down Expand Up @@ -136,5 +140,40 @@ public static EntityTypeBuilder<TEntity> ForCockroachDbInterleaveInParent<TEntit
=> (EntityTypeBuilder<TEntity>)ForCockroachDbInterleaveInParent((EntityTypeBuilder)entityTypeBuilder, parentTableType, interleavePrefix);

#endregion CockroachDB Interleave-in-parent

#region Generic Index

/// <summary>
/// Configures an index on the specified properties. If there is an existing index on the given
/// set of properties, then the existing index will be returned for configuration.
/// </summary>
/// <typeparam name="TEntity"> The entity type being configured. </typeparam>
/// <param name="entityTypeBuilder"> The builder for the entity type being configured. </param>
Comment thread
austindrenski marked this conversation as resolved.
/// <param name="indexExpression">
/// <para>
/// A lambda expression representing the property(s) to be included in the index
/// (<c>blog => blog.Url</c>).
/// </para>
/// <para>
/// If the index is made up of multiple properties then specify an anonymous type including the
/// properties (<c>post => new { post.Title, post.BlogId }</c>).
/// </para>
/// </param>
/// <returns> An object that can be used to configure the index. </returns>
Comment thread
austindrenski marked this conversation as resolved.
public static IndexBuilder<TEntity> ForNpgsqlHasIndex<TEntity>(
[NotNull] this EntityTypeBuilder<TEntity> entityTypeBuilder,
[NotNull] Expression<Func<TEntity, object>> indexExpression)
where TEntity : class
{
Check.NotNull(entityTypeBuilder, nameof(entityTypeBuilder));
Check.NotNull(indexExpression, nameof(indexExpression));

var builder = ((IInfrastructure<InternalEntityTypeBuilder>)entityTypeBuilder).GetInfrastructure();

return new IndexBuilder<TEntity>(
builder.HasIndex(indexExpression.GetPropertyAccessList(), ConfigurationSource.Explicit));
}

#endregion
}
}
68 changes: 65 additions & 3 deletions src/EFCore.PG/Extensions/NpgsqlIndexBuilderExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
using JetBrains.Annotations;
using System;
using System.Linq;
using System.Linq.Expressions;
using System.Runtime.CompilerServices;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Internal;
using Microsoft.EntityFrameworkCore.Metadata.Builders;
using Npgsql.EntityFrameworkCore.PostgreSQL.Utilities;

Expand All @@ -19,7 +24,9 @@ public static class NpgsqlIndexBuilderExtensions
/// <param name="indexBuilder"> The builder for the index being configured. </param>
/// <param name="method"> The name of the index. </param>
/// <returns> A builder to further configure the index. </returns>
public static IndexBuilder ForNpgsqlHasMethod([NotNull] this IndexBuilder indexBuilder, [CanBeNull] string method)
public static IndexBuilder ForNpgsqlHasMethod(
[NotNull] this IndexBuilder indexBuilder,
[CanBeNull] string method)
{
Check.NotNull(indexBuilder, nameof(indexBuilder));
Check.NullButNotEmpty(method, nameof(method));
Expand All @@ -38,7 +45,9 @@ public static IndexBuilder ForNpgsqlHasMethod([NotNull] this IndexBuilder indexB
/// <param name="indexBuilder"> The builder for the index being configured. </param>
/// <param name="operators"> The operators to use for each column. </param>
/// <returns> A builder to further configure the index. </returns>
public static IndexBuilder ForNpgsqlHasOperators([NotNull] this IndexBuilder indexBuilder, [CanBeNull] params string[] operators)
public static IndexBuilder ForNpgsqlHasOperators(
[NotNull] this IndexBuilder indexBuilder,
[CanBeNull, ItemNotNull] params string[] operators)
{
Check.NotNull(indexBuilder, nameof(indexBuilder));
Check.NullButNotEmpty(operators, nameof(operators));
Expand All @@ -47,5 +56,58 @@ public static IndexBuilder ForNpgsqlHasOperators([NotNull] this IndexBuilder ind

return indexBuilder;
}

/// <summary>
/// Adds an INCLUDE clause to the index definition with the specified property names.
/// This clause specifies a list of columns which will be included as a non-key part in the index.
/// </summary>
/// <remarks>
/// https://www.postgresql.org/docs/current/sql-createindex.html
/// </remarks>
/// <param name="indexBuilder"> The builder for the index being configured. </param>
/// <param name="propertyNames"> An array of property names to be used in INCLUDE clause. </param>
/// <returns> A builder to further configure the index. </returns>
Comment thread
austindrenski marked this conversation as resolved.
public static IndexBuilder ForNpgsqlInclude(
[NotNull] this IndexBuilder indexBuilder,
[CanBeNull, ItemNotNull] params string[] propertyNames)
{
Check.NotNull(indexBuilder, nameof(indexBuilder));
Check.NullButNotEmpty(propertyNames, nameof(propertyNames));

indexBuilder.Metadata.Npgsql().IncludeProperties = propertyNames;

return indexBuilder;
}

/// <summary>
/// Adds an INCLUDE clause to the index definition with property names from the specified expression.
/// This clause specifies a list of columns which will be included as a non-key part in the index.
/// </summary>
/// <remarks>
/// https://www.postgresql.org/docs/current/sql-createindex.html
/// </remarks>
/// <param name="indexBuilder"> The builder for the index being configured. </param>
/// <param name="includeExpression">
/// <para>
/// A lambda expression representing the property(s) to be included in the INCLUDE clause
/// (<c>blog => blog.Url</c>).
/// </para>
/// <para>
/// If multiple properties are to be included then specify an anonymous type including the
/// properties (<c>post => new { post.Title, post.BlogId }</c>).
/// </para>
/// </param>
/// <returns> A builder to further configure the index. </returns>
public static IndexBuilder<TEntity> ForNpgsqlInclude<TEntity>(
[NotNull] this IndexBuilder<TEntity> indexBuilder,
[NotNull] Expression<Func<TEntity, object>> includeExpression)
{
Check.NotNull(indexBuilder, nameof(indexBuilder));
Check.NotNull(includeExpression, nameof(includeExpression));

indexBuilder.ForNpgsqlInclude(includeExpression.GetPropertyAccessList().Select(x => x.Name).ToArray());

return indexBuilder;
Comment thread
khellang marked this conversation as resolved.
}
}
}
10 changes: 9 additions & 1 deletion src/EFCore.PG/Metadata/INpgsqlIndexAnnotations.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,19 @@ public interface INpgsqlIndexAnnotations : IRelationalIndexAnnotations
string Method { get; }

/// <summary>
/// The PostgreSQL index operators to be used.
/// The PostgreSQL index operators to be used, or <c>null</c> if they have not been specified.
/// </summary>
/// <remarks>
/// https://www.postgresql.org/docs/current/static/indexes-opclass.html
/// </remarks>
IReadOnlyList<string> Operators { get; }

/// <summary>
/// The PostgreSQL included property names, or <c>null</c> if they have not been specified.
/// </summary>
/// <remarks>
/// https://www.postgresql.org/docs/current/sql-createindex.html
/// </remarks>
IReadOnlyList<string> IncludeProperties { get; }
Comment thread
khellang marked this conversation as resolved.
}
}
1 change: 1 addition & 0 deletions src/EFCore.PG/Metadata/Internal/NpgsqlAnnotationNames.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ public static class NpgsqlAnnotationNames
public const string HiLoSequenceSchema = Prefix + "HiLoSequenceSchema";
public const string IndexMethod = Prefix + "IndexMethod";
public const string IndexOperators = Prefix + "IndexOperators";
public const string IndexInclude = Prefix + "IndexInclude";
public const string PostgresExtensionPrefix = Prefix + "PostgresExtension:";
public const string EnumPrefix = Prefix + "Enum:";
public const string RangePrefix = Prefix + "Range:";
Expand Down
19 changes: 18 additions & 1 deletion src/EFCore.PG/Metadata/NpgsqlIndexAnnotations.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public string Method
}

/// <summary>
/// The PostgreSQL index operators to be used.
/// The PostgreSQL index operators to be used, or <c>null</c> if they have not been specified.
/// </summary>
/// <remarks>
/// https://www.postgresql.org/docs/current/static/indexes-opclass.html
Expand All @@ -43,10 +43,27 @@ public string[] Operators

IReadOnlyList<string> INpgsqlIndexAnnotations.Operators => Operators;

/// <summary>
/// The PostgreSQL included property names, or <c>null</c> if they have not been specified.
/// </summary>
/// <remarks>
/// https://www.postgresql.org/docs/current/sql-createindex.html
/// </remarks>
public string[] IncludeProperties
{
get => (string[]) Annotations.Metadata[NpgsqlAnnotationNames.IndexInclude];
set => SetIncludeProperties(value);
}

IReadOnlyList<string> INpgsqlIndexAnnotations.IncludeProperties => IncludeProperties;

protected virtual bool SetMethod(string value)
=> Annotations.SetAnnotation(NpgsqlAnnotationNames.IndexMethod, value);

protected virtual bool SetOperators(string[] value)
=> Annotations.SetAnnotation(NpgsqlAnnotationNames.IndexOperators, value);

protected virtual bool SetIncludeProperties(string[] value)
=> Annotations.SetAnnotation(NpgsqlAnnotationNames.IndexInclude, value);
}
}
2 changes: 2 additions & 0 deletions src/EFCore.PG/Migrations/Internal/NpgsqlMigrationsAnnotationProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ public override IEnumerable<IAnnotation> For(IIndex index)
yield return new Annotation(NpgsqlAnnotationNames.IndexMethod, index.Npgsql().Method);
if (index.Npgsql().Operators != null)
yield return new Annotation(NpgsqlAnnotationNames.IndexOperators, index.Npgsql().Operators);
if (index.Npgsql().IncludeProperties != null)
yield return new Annotation(NpgsqlAnnotationNames.IndexInclude, index.Npgsql().IncludeProperties);
}

public override IEnumerable<IAnnotation> For(IModel model)
Expand Down
33 changes: 19 additions & 14 deletions src/EFCore.PG/Migrations/NpgsqlMigrationsSqlGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,7 @@ protected override void Generate(
}

// Comment on the table
var comment = operation[NpgsqlAnnotationNames.Comment] as string;
if (comment != null)
if (operation[NpgsqlAnnotationNames.Comment] is string comment && comment.Length > 0)
{
builder.AppendLine(';');

Expand Down Expand Up @@ -245,8 +244,7 @@ protected override void Generate(

base.Generate(operation, model, builder, terminate: false);

var comment = operation[NpgsqlAnnotationNames.Comment] as string;
if (comment != null)
if (operation[NpgsqlAnnotationNames.Comment] is string comment && comment.Length > 0)
{
builder.AppendLine(';');

Expand Down Expand Up @@ -535,9 +533,6 @@ protected override void Generate(
Check.NotNull(operation, nameof(operation));
Check.NotNull(builder, nameof(builder));

var method = (string)operation[NpgsqlAnnotationNames.IndexMethod];
var operators = (string[])operation[NpgsqlAnnotationNames.IndexOperators];

builder.Append("CREATE ");

if (operation.IsUnique)
Expand All @@ -551,30 +546,40 @@ protected override void Generate(
.Append(" ON ")
.Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Table, operation.Schema));

if (method != null)
if (operation[NpgsqlAnnotationNames.IndexMethod] is string method && method.Length > 0)
{
builder
.Append(" USING ")
.Append(method);
}

var operators = operation[NpgsqlAnnotationNames.IndexOperators] as string[];

builder
.Append(" (")
.Append(IndexColumnList(operation.Columns, operators))
.Append(")");

if (!string.IsNullOrEmpty(operation.Filter))
Copy link
Copy Markdown

@suadev suadev Sep 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@roji hi,

I don't know why this 'Where' is deleted but i can't add a filter for my all indexes. I need to add this index filter; 'is_deleted is FALSE' just because i am using soft delete with unique indexes. Any idea?

Copy link
Copy Markdown

@suadev suadev Sep 23, 2020

Choose a reason for hiding this comment

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

/CC: @khellang

Copy link
Copy Markdown
Contributor

@YohDeadfall YohDeadfall Sep 23, 2020

Choose a reason for hiding this comment

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

INCLUDE isn't the same as the WHERE option. The feature which @khellang added supports the first one to improve read performance (the specified columns are included into the index, but not participate in ordering; this allows to take values only from one table called an index table without joining to the original table).

Copy link
Copy Markdown
Contributor Author

@khellang khellang Sep 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@suadev It's deleted because it's using the overriden IndexOptions method, with a call to base.IndexOptions below.

If you take a look at the base class (in EF Core), you can see that it's still adding the WHERE clause if there's a filter specified:

https://github.com/dotnet/efcore/blob/c13f8b4a645c6b7cf141c9c62833ea4269622912/src/EFCore.Relational/Migrations/MigrationsSqlGenerator.cs#L1832-L1843

Copy link
Copy Markdown
Member

@roji roji Sep 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

To add to what @YohDeadfall wrote, the WHERE support wasn't deleted - it was simply moved into EF Core itself (see MigrationsSqlGenerator).

Are you seeing an actual issue with it not working? If so, please open a new issue with a code sample so we can investigate.

Copy link
Copy Markdown
Member

@roji roji Sep 23, 2020

Choose a reason for hiding this comment

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

You may need to look at this: https://docs.microsoft.com/en-us/ef/core/miscellaneous/configuring-dbcontext#design-time-dbcontext-configuration

Copy link
Copy Markdown

@suadev suadev Sep 23, 2020

Choose a reason for hiding this comment

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

@roji thanks, but i didn't get it. I am still not sure what is my mistake. Overriding IndexOptions method is not a correct way?

Copy link
Copy Markdown
Contributor Author

@khellang khellang Sep 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

You could also try to just modify the model itself, instead of swapping components? I.e.

protected override void OnModelCreating(ModelBuilder modelBuilder) {
	foreach (var entity in modelBuilder.Model.GetEntityTypes()) {
		foreach (var index in entity.GetDeclaredIndexes()) {
			if (index.IsUnique) {
				index.SetFilter("is_deleted is FALSE");
			}
		}
	}
}

Copy link
Copy Markdown

@suadev suadev Sep 23, 2020

Choose a reason for hiding this comment

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

@khellang thank you! i am not even aware of GetDeclaredIndexes() method !

Copy link
Copy Markdown
Member

@roji roji Sep 23, 2020

Choose a reason for hiding this comment

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

Changing the model is definitely a better approach than replacing services, wherever it's possible.

{
builder
.Append(" WHERE ")
.Append(operation.Filter);
}
IndexOptions(operation, model, builder);

builder.AppendLine(';');

EndStatement(builder);
}

protected override void IndexOptions(CreateIndexOperation operation, IModel model, MigrationCommandListBuilder builder)
{
if (operation[NpgsqlAnnotationNames.IndexInclude] is string[] includeProperties && includeProperties.Length > 0)
{
builder
.Append(" INCLUDE (")
.Append(ColumnList(includeProperties))
.Append(")");
}

base.IndexOptions(operation, model, builder);
Comment thread
roji marked this conversation as resolved.
}

protected override void Generate(EnsureSchemaOperation operation, [CanBeNull] IModel model, MigrationCommandListBuilder builder)
{
Check.NotNull(operation, nameof(operation));
Expand Down
17 changes: 17 additions & 0 deletions test/EFCore.PG.FunctionalTests/NpgsqlMigrationSqlGeneratorTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -633,6 +633,23 @@ public void CreateIndexOperation_operations()
Sql);
}

[Fact]
public void CreateIndexOperation_includes()
{
Generate(
new CreateIndexOperation
{
Name = "IX_People_Name",
Table = "People",
Columns = new[] { "Name" },
[NpgsqlAnnotationNames.IndexInclude] = new[] { "FirstName", "LastName" }
});

Assert.Equal(
"CREATE INDEX \"IX_People_Name\" ON \"People\" (\"Name\") INCLUDE (\"FirstName\", \"LastName\");" + EOL,
Sql);
}

[Fact]
public void CreateIndexOperation_schema_qualified_operations()
{
Expand Down