Efcore: Scaffold error when two indexes exist on the same column set

[roji]

In https://github.com/npgsql/Npgsql.EntityFrameworkCore.PostgreSQL/issues/228, @berets76 describes an issue with the way indices are managed during scaffolding.

PostgreSQL supports multiple index "methods", and the Npgsql provider represents these via a simple string annotation on the index - all indices in a scaffolded DatabaseModel contain this annotation (note that if the default method is detected, the annotation is later eliminated by convention in NpgsqlAnnotationCodeGenerator).

Now, consider the following database schema:

CREATE TABLE data
(
  id integer PRIMARY KEY,
  name character(3)
);

CREATE INDEX index1 ON data (name);
CREATE INDEX index2 ON data (name DESC);

Two indices exist on the same column, which should be fine. However, trying to scaffold this throws the following:

System.InvalidOperationException: The annotation 'Npgsql:IndexMethod' cannot be added because an annotation with the same name already exists.
   at Microsoft.EntityFrameworkCore.Infrastructure.Annotatable.AddAnnotation(String name, Annotation annotation)
   at Microsoft.EntityFrameworkCore.MutableAnnotatableExtensions.AddAnnotations(IMutableAnnotatable annotatable, IEnumerable`1 annotations)
   at Microsoft.EntityFrameworkCore.Scaffolding.Internal.RelationalScaffoldingModelFactory.VisitIndex(EntityTypeBuilder builder, DatabaseIndex index) in C:\b\w\33bdfc1cae7b2a38\modules\EntityFrameworkCore\src\EFCore.Design\Scaffolding\Internal\RelationalScaffoldingModelFactory.cs:line 607
   at Microsoft.EntityFrameworkCore.Scaffolding.Internal.RelationalScaffoldingModelFactory.VisitIndexes(EntityTypeBuilder builder, ICollection`1 indexes) in C:\b\w\33bdfc1cae7b2a38\modules\EntityFrameworkCore\src\EFCore.Design\Scaffolding\Internal\RelationalScaffoldingModelFactory.cs:line 560
   at Microsoft.EntityFrameworkCore.Scaffolding.Internal.RelationalScaffoldingModelFactory.VisitTable(ModelBuilder modelBuilder, DatabaseTable table) in C:\b\w\33bdfc1cae7b2a38\modules\EntityFrameworkCore\src\EFCore.Design\Scaffolding\Internal\RelationalScaffoldingModelFactory.cs:line 323
   at Microsoft.EntityFrameworkCore.Scaffolding.Internal.RelationalScaffoldingModelFactory.VisitTables(ModelBuilder modelBuilder, ICollection`1 tables) in C:\b\w\33bdfc1cae7b2a38\modules\EntityFrameworkCore\src\EFCore.Design\Scaffolding\Internal\RelationalScaffoldingModelFactory.cs:line 279
   at Microsoft.EntityFrameworkCore.Scaffolding.Internal.RelationalScaffoldingModelFactory.VisitDatabaseModel(ModelBuilder modelBuilder, DatabaseModel databaseModel) in C:\b\w\33bdfc1cae7b2a38\modules\EntityFrameworkCore\src\EFCore.Design\Scaffolding\Internal\RelationalScaffoldingModelFactory.cs:line 182
   at Microsoft.EntityFrameworkCore.Scaffolding.Internal.RelationalScaffoldingModelFactory.Create(DatabaseModel databaseModel, Boolean useDatabaseNames) in C:\b\w\33bdfc1cae7b2a38\modules\EntityFrameworkCore\src\EFCore.Design\Scaffolding\Internal\RelationalScaffoldingModelFactory.cs:line 99
   at Microsoft.EntityFrameworkCore.Scaffolding.Internal.ReverseEngineerScaffolder.ScaffoldModel(String connectionString, IEnumerable`1 tables, IEnumerable`1 schemas, String namespace, String language, String contextDir, String contextName, ModelReverseEngineerOptions modelOptions, ModelCodeGenerationOptions codeOptions) in C:\b\w\33bdfc1cae7b2a38\modules\EntityFrameworkCore\src\EFCore.Design\Scaffolding\Internal\ReverseEngineerScaffolder.cs:line 97
   at Microsoft.EntityFrameworkCore.Design.Internal.DatabaseOperations.ScaffoldContext(String provider, String connectionString, String outputDir, String outputContextDir, String dbContextClassName, IEnumerable`1 schemas, IEnumerable`1 tables, Boolean useDataAnnotations, Boolean overwriteFiles, Boolean useDatabaseNames) in C:\b\w\33bdfc1cae7b2a38\modules\EntityFrameworkCore\src\EFCore.Design\Design\Internal\DatabaseOperations.cs:line 94
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.ScaffoldContextImpl(String provider, String connectionString, String outputDir, String outputDbContextDir, String dbContextClassName, IEnumerable`1 schemaFilters, IEnumerable`1 tableFilters, Boolean useDataAnnotations, Boolean overwriteFiles, Boolean useDatabaseNames) in C:\b\w\33bdfc1cae7b2a38\modules\EntityFrameworkCore\src\EFCore.Design\Design\OperationExecutor.cs:line 463
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.ScaffoldContext.<>c__DisplayClass0_1.<.ctor>b__0() in C:\b\w\33bdfc1cae7b2a38\modules\EntityFrameworkCore\src\EFCore.Design\Design\OperationExecutor.cs:line 445
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.OperationBase.<>c__DisplayClass3_0`1.<Execute>b__0() in C:\b\w\33bdfc1cae7b2a38\modules\EntityFrameworkCore\src\EFCore.Design\Design\OperationExecutor.cs:line 555
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.OperationBase.Execute(Action action) in C:\b\w\33bdfc1cae7b2a38\modules\EntityFrameworkCore\src\EFCore.Design\Design\OperationExecutor.cs:line 538

It seems that at some point during the scaffolding process, the two indices provided by the DatabaseModel get "collapsed" to one, and we get the error since the annotation is already set. This is probably because at some point EF Core looks up the index, keying only by the column list, and omits other index characteristics (e.g ascending/descending).

Thanks again to @berets76 for reproducing and analyzing, I only provided the writeup.

[smitpatel]

4150 Index column sort order is not supported in metadata at all. Hence, the scaffolded model did not expect same index to appear multiple times. Perhaps, it can avoid throwing error but that would still ignore other index and keep only 1 (without ordering specs).

[TheBaradhur]

@ajcvickers I'm actually really sad you removed that tag. I can't scaffold my model automatically using EFCore because of that and I need a long term solution to that index issue.

Any chance you could implement quickly @smitpatel suggestion for it to at least not throw and expection and just not create those problematic indexes?

Thanks!

[divega]

@TheBaradhur the “consider-for-next-release” label is assigned temporarily for release planning. This is now assigned to the 2.2.0 milestone (at least for now), which has more significance.

[TheBaradhur]

@divega thank you for the update. But considering that 2.1 is not even officially released, I'm looking at months, if not a year, before this issue can be fixed. I will have to find other ways to scaffold my db until then.

[ErikEJ]

It could be fixed in the Npgsql provider

[roji]

@ErikEJ is right, I'll make a change in the Npgsql provider to make sure it does not throw in this case, at least as a workaround until the core issue is resolved.

[roji]

@divega, @smitpatel, I've pushed a workaround which makes this issue go away for the majority of users, making this less urgent (I now include the IndexMethod annotation in the scaffolded database model only if it's the PostgreSQL non-default one). The issue is still there but shouldn't bother most people anymore.

[roji]

However, EF Core is still incapable of scaffolding two indices on the same column (ascending and descending) which does seem to be a serious issue (and has nothing to do with PostgreSQL).

[ErikEJ]

The value of DESC columns in indexes is debateable

[roji]

@ErikEJ I think that depends on specific databases... I admit I'm not sure exactly what the impact is in PostgreSQL, but it seems like if it's supported in DDL EF Core should be able to render it...

[TheBaradhur]

@ErikEJ We use double indexes on same column with a tolower conditions in our PostgreSQL db, a few with DESC, in a lot of different tables. Especially the ones where we are dealing with 10th of thousands of rows in the table.

Having researched the internet about that double index issues in EFCore, I can tell you that it is not uncommon.

[divega]

DESC on a column in an index is useful when queries have ORDER BY clauses on multiple columns with mixed order.

In SQL Server, very often tables are clustered (i.e. organized) on the PK, hence you can think of indexes on those tables as always containing an implicit reference to the PK column or columns. For example if on SQL Server you want to execute:

SELECT *
FROM Customers
ORDER BY CutomerID, Name DESC

Then an index on Name DESC can help.

I don’t know much about PostgreSQL, but the information I was able to find says it does not support index organized tables. That means that for DESC to be useful on an index on PostgreSQL, you need to have an index on more than one column, and the same or opposite mixed order on both the index and the queries.

AFAIK, having DESC on an index on a single column should never be necessary on PostgreSQL, as indexes can be used to return results ordered in the opposite direction.

Re tolower, are those using a function or computed column, or is tolower an attribute of the index?

Some interesting reads:

https://stackoverflow.com/questions/743858/sql-server-indexes-ascending-or-descending-what-difference-does-it-make

https://use-the-index-luke.com/sql/sorting-grouping/order-by-asc-desc-nulls-last

[roji]

@divega an index on tolower() in PostgreSQL would be an expression index. There is no specific support for those in the Npgsql provider (see https://github.com/npgsql/Npgsql.EntityFrameworkCore.PostgreSQL/issues/119) but they can be created with raw SQL.

To summarize, I think there are several legitimate reasons why someone would have multiple indices on the same column set; between expression indices, ASC/DESC, index methods (a concept specific to PostgreSQL) I think there's enough material. At the moment, when scaffolding EF Core will retain the last index present on the database model, and silently discard the others.

[divega]

an index on tolower() in PostgreSQL would be an expression index

Makes sense. This is what I was expecting to hear :smile:

there are several legitimate reasons why someone would have multiple indices on the same column set

Agreed. However I wonder if it would be better to change EF Core to separate the common case (index over a sequence of columns with implicit ASC on each of them, which we already support) from all other variations. EF Core would still reason about the column set for the simple scenario, but the index definition would become "custom"/provider-specific (and opaque to EF Core) for all other cases. Then we would never try to match a simple index against custom indexes based on the columns.

[ajcvickers]

Closing this in favor of #4150

[ajcvickers]

Leaving this in 2.2 for better reverse engineering experience.

[yyjdelete]

It would be helpful after #17083 is fixed. And at least ignore the second index in VisitIndex instead of simply crash .

For something like

CREATE NONCLUSTERED INDEX [Z_Common] ON [Table]
([A],[B])
INCLUDE([C],[D])
//WHERE ([Z] = 1)



CREATE NONCLUSTERED INDEX [Z_0] ON [Table]
([A],[B])
INCLUDE([C],[D],[E],[F]) 
WHERE ([Z] = 0)

[AndriySvyryd]

Named indexes are now supported, so this can be fixed properly

[lauxjpn]

Got about the same issue reported for Pomelo:
https://github.com/PomeloFoundation/Pomelo.EntityFrameworkCore.MySql/issues/1189#issuecomment-707354546

This issues appears, when some facet of the index (in this case the prefix) is being represented internally by an annotation. The moment an annotation of the same type (in this case MySql:IndexPrefixLength) is being set for the second time for an object (here: index), the exception is being thrown.

Could it be related to this: dotnet/efcore#11846 ?

I can confirm, that the moment two indices are used for the same column, and both are using a prefix length, I can reproduce the exception:

CREATE TABLE `Test` (
  `TestId` int NOT NULL AUTO_INCREMENT,
  `LongString` varchar(12000) DEFAULT NULL,
  PRIMARY KEY (`TestId`),
  KEY `IX_LongString` (`LongString`(120)),
  KEY `IX_LongString2` (`LongString`(100))
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci;

Basically, the `LongString`(120) and `LongString`(100) parts get translated into a MySql:IndexPrefixLength annotation respectively, that should be set only once per index. The indices are individually named, but get collapsed/keyed into one single index builder, that throws once the annotation gets set the second time (and also overwrites the first index name with the second one).