From 38d2176f27d31db602fc2ee9540aaa4fa6411ce9 Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Sat, 26 May 2018 07:05:49 +0100 Subject: [PATCH] Don't scaffold default index method annotation When scaffolding, we have an Npgsql-specific IndexMethod annotation to represent PostgreSQL's index methods. Previously, we would always output this annotation on indices, even when the method was the default (btree); NpgsqlAnnotationCodeGenerator would elide it as by- convention. Although this is cleaner, it caused issues whenever two indices where defined on the same column: https://github.com/aspnet/EntityFrameworkCore/issues/11846# We now scaffold the annotation only for non-default index methods as a workaround; the issue should now affect much less people. Fixes #228 --- .../Scaffolding/Internal/NpgsqlDatabaseModelFactory.cs | 9 ++++++++- .../Scaffolding/NpgsqlDatabaseModelFactoryTest.cs | 8 +++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/EFCore.PG/Scaffolding/Internal/NpgsqlDatabaseModelFactory.cs b/src/EFCore.PG/Scaffolding/Internal/NpgsqlDatabaseModelFactory.cs index fe0316332..68c9c8919 100644 --- a/src/EFCore.PG/Scaffolding/Internal/NpgsqlDatabaseModelFactory.cs +++ b/src/EFCore.PG/Scaffolding/Internal/NpgsqlDatabaseModelFactory.cs @@ -513,7 +513,14 @@ NOT indisprimary if (predicate != null) index.Filter = predicate; - index[NpgsqlAnnotationNames.IndexMethod] = record.GetValueOrDefault("amname"); + // It's cleaner to always output the index method on the database model, + // even when it's btree (the default); + // NpgsqlAnnotationCodeGenerator can then omit it as by-convention. + // However, because of https://github.com/aspnet/EntityFrameworkCore/issues/11846 we omit + // the annotation from the model entirely. + var indexMethod = record.GetValueOrDefault("amname"); + if (indexMethod != "btree") + index[NpgsqlAnnotationNames.IndexMethod] = indexMethod; table.Indexes.Add(index); } diff --git a/test/EFCore.PG.FunctionalTests/Scaffolding/NpgsqlDatabaseModelFactoryTest.cs b/test/EFCore.PG.FunctionalTests/Scaffolding/NpgsqlDatabaseModelFactoryTest.cs index d1b01ad95..3598ad4d0 100644 --- a/test/EFCore.PG.FunctionalTests/Scaffolding/NpgsqlDatabaseModelFactoryTest.cs +++ b/test/EFCore.PG.FunctionalTests/Scaffolding/NpgsqlDatabaseModelFactoryTest.cs @@ -1505,8 +1505,14 @@ public void Index_method() var methodIndex = table.Indexes.Single(i => i.Name == "ix_a"); Assert.Equal("hash", methodIndex.FindAnnotation(NpgsqlAnnotationNames.IndexMethod).Value); + // It's cleaner to always output the index method on the database model, + // even when it's btree (the default); + // NpgsqlAnnotationCodeGenerator can then omit it as by-convention. + // However, because of https://github.com/aspnet/EntityFrameworkCore/issues/11846 we omit + // the annotation from the model entirely. var noMethodIndex = table.Indexes.Single(i => i.Name == "ix_b"); - Assert.Equal("btree", noMethodIndex.FindAnnotation(NpgsqlAnnotationNames.IndexMethod).Value); + Assert.Null(noMethodIndex.FindAnnotation(NpgsqlAnnotationNames.IndexMethod)); + //Assert.Equal("btree", noMethodIndex.FindAnnotation(NpgsqlAnnotationNames.IndexMethod).Value); }, @"DROP TABLE ""IndexMethod"""); }