Efcore: SQL Server: Support INCLUDE (columnA, columnB...) clause when creating indexes

[OlsonDev]

I'd like to be able to do something like:

``` c#
protected override void OnModelCreating(ModelBuilder modelBuilder) {
modelBuilder.Entity()
.HasIndex(e => new { e.ColumnA })
.Includes(e => new { e.ColumnB, e.ColumnC })
;
}

which would generate a migration that would generate something like:

``` sql
CREATE NONCLUSTERED INDEX IX_Foo_ColumnA_INCLUDE_ColumnB_ColumnC
ON dbo.Foo (ColumnA)
INCLUDE (ColumnB, ColumnC)
GO

[rowanmiller]

Dupe of https://github.com/aspnet/EntityFramework/issues/3748 which we closed... but we've now seen 3 distinct requests for this feature so we should re-discuss in triage.

[rabbal]

1+

[gdoron]

@rowanmiller
Is there any reason not implementing this or it's simply a matter of priority?
Index with included properties is a core feature of SQL-Server.

[DirtySmurf]

I was just trying to do something like this today, finally found this issue after a long search.

[gdoron]

@rowanmiller What's the status with this?
This issue has relatively high amount of thumbs up as well as the other linked issues.
And @jpadgeo even wrote it took him a long time to find this issue to upvote it.

Thanks!

[rowanmiller]

It's just a matter of priority. It comes after some of the other things on our Roadmap. You can workaround it at the moment by replacing the code to create indexes in your migrations with a Sql(stirng) call to create it with the INCLUDEs

[gdoron]

Thanks for the reply Rowan.
The problem with having raw DDL SQL in a migration is that you can't delete all your migrations and start fresh, which is a common pattern people do.

But I guess it's the only way currently available.

[JT-Bruch]

Just ran into this myself.. especially when attempting to create a code first database from an existing database. Would LOVE to see this added.

[arrkaye]

+1

[arrkaye]

Hi,

Is there any update on this?

I'm happy to contribute a PR if someone can point me in the right direction.

Thanks,

Ark

[ajcvickers]

@arrkaye Probably the API would be something like this:

C# modelBuilder.Entity<User>() .HasIndex(e => e.Prop1) .Include(e => e.Prop2);
That is, add an Include method to IndexBuilder.

The included properties would then be stored on the IIndex metadata, either as annotations or as a first-class part of the metadata. This would then flow into the MigrationsModelDiffer, and would end up in the Migrations model, which would also need updating with either annotations or first class metadata. That in turn would be used by Migrations to generate SQL.

@divega @bricelam @anpete @smitpatel @AndriySvyryd Any other thoughts on this? Should this be relational-only, or part of core indexes? If in core, annotations or first-class?

[bricelam]

These are SQL Server specific. As far as I can tell, SQLite, PostgreSQL, Oracle, and MySQL don't support them.

[ajcvickers]

@bricelam So in that case definitely not core, and maybe not relational either--just as SQL Server extension method?

[divega]

Didn't see @bricelam's response so I guess I did the same investigation :sweat_smile:

TL;DR: I can :+1: his findings.

For those really interested:

• Among the database I investigated Only SQL Server and DB2 seem to support this, but DB2 only supports INCLUDE if UNIQUE is specified. The idea is that with INCLUDE you can add columns to create covering indexes without changing the set of columns on which the uniqueness is checked.
• Relational databases other than SQL Server tend to support wider index keys, which means you can generally create covering indexes in more cases without needing something like INCLUDE.
• MySQL has "prefix indexing", which means you can truncate how many of the initial characters of a column get included in the index to make more columns fit. This can be done explicitly or automatically (e.g. if the total size of the columns doesn't fit in the limits). It only really helps with creating indexes containing lots of columns. It doesn't really help with covering indexes because now the full data of the column isn't in the index and you need to always go to the table to obtain it.

[vinniep79]

Please add this feature! Here's my situation:

We're using SQL Azure, which gives you automatic indexing recommendations.
We're also using EF code-first.

There's a persistent index recommendation from Azure SQL that involves included columns. I don't want to allow Azure to do it automatically because that'll disconnect my code-first models from what's in the DB. The next time I try to run a migration, it'll likely break because EF's picture of reality is disconnected from SQL's. I'd much rather take SQL's recommendation and apply it to my EF model.

Even when I tell Azure SQL to disregard that particular recommendation it comes back whenever there's an EF model change, so it's quite obnoxious. I don't want to disable Azure SQL's across the board though, because it often has some very good stuff!

[mivano]

See here for an implementation: https://mindbyte.nl/2017/12/26/Create-indexes-with-included-columns-with-Entity-Framework-Core-using-code-first.html

[Kukkimonsuta]

I'd like to attempt to implement this. Could you give me some feedback on proposed public API? Since this is realistically only useful for sql server I believe it should be ForSqlServerInclude to avoid throwing on most other providers. I've though of two ways to go about this:

1. simply accept string params only

namespace Microsoft.EntityFrameworkCore
{
    public static class SqlServerIndexBuilderExtensions
    {
+        public static IndexBuilder ForSqlServerInclude(this IndexBuilder indexBuilder, params string[] propertyNames);
    }
}

1. accept also an expression like HasIndex

Now there an issue where HasIndex with expression param returns non-generic IndexBuilder meaning the type information about entity is lost to next fluent calls. This can be fixed by introducing a IndexBuilder<TEntity> and modifying HasIndex, but that would likely be a breaking change.

namespace Microsoft.EntityFrameworkCore.Metadata.Builders
{
    public class EntityTypeBuilder<TEntity> : EntityTypeBuilder where TEntity : class
    {
-        public virtual IndexBuilder HasIndex(Expression<Func<TEntity, object>> indexExpression);
+        public virtual IndexBuilder<TEntity> HasIndex(Expression<Func<TEntity, object>> indexExpression);
    }

+   public class IndexBuilder<TEntity> : IndexBuilder
+   {
+   }
}


namespace Microsoft.EntityFrameworkCore
{
    public static class SqlServerIndexBuilderExtensions
    {
+        public static IndexBuilder ForSqlServerInclude(this IndexBuilder indexBuilder, params string[] propertyNames);
+        public static IndexBuilder<TEntity> ForSqlServerInclude<TEntity>(this IndexBuilder<TEntity> indexBuilder, Expression<Func<TEntity, object>> includeExpression);
    }
}

[ajcvickers]

@Kukkimonsuta Thanks for your interest in this. Adding @AndriySvyryd to comment on API. @AndriySvyryd is making a generic IndexBuilder something that will cause issues in the API? (We don't have other generics at this level.) One idea we had in triage was to create a new overload of HasIndex (or a new extension method called ForSqlServerHasIndex) as a SQL Server extension method that take two parameters--the lambda for properties and a new lambda for included properties. However, that works less well with the string overloads. Thoughts?

[AndriySvyryd]

I don't see any issues with adding a generic IndexBuilder that derives from the non-generic one. This wouldn't be a compilation breaking change.

[ajcvickers]

@Kukkimonsuta We discussed again with @AndriySvyryd and came to the following conclusions:

• To avoid the breaking change, there should be new ForSqlServerHasIndex extension methods returning a generic IndexBuilder.
• The generic IndexBuilder itself should be in the main EFCore assembly, not in the SQL Server provider
• The extension method ForSqlServerInclude that takes a lambda will be on this new IndexBuilder
• For 3.0, we should consider breaking HasIndex to return the generic and obsoleting ForSqlServerHasIndex.

[eriksendc]

Does milestone 2.2.preview2 mean this is in (or hopefully in) 2.2?

[ajcvickers]

@eriksendc Yep.

[clement911]

Was this feature released?

[ajcvickers]

@clement911 Yes. For closed issues, the milestone indicates the release in which it was fixed.

[clement911]

Got it. thanks