Fix index creation when both collation and operators are specified (#3028)

Fixes #3027

Author: roji

src/EFCore.PG/Migrations/NpgsqlMigrationsSqlGenerator.cs

@@ -2051,20 +2051,24 @@ private string DelimitIdentifier(string name, string? schema)
 
     private string IndexColumnList(IndexColumn[] columns, string? method)
     {
-        var isFirst = true;
         var builder = new StringBuilder();
 
         for (var i = 0; i < columns.Length; i++)
         {
-            if (!isFirst)
+            var column = columns[i];
+
+            if (i > 0)
             {
                 builder.Append(", ");
             }
 
-            var column = columns[i];
-
             builder.Append(DelimitIdentifier(column.Name));
 
+            if (!string.IsNullOrEmpty(column.Collation))
+            {
+                builder.Append(" COLLATE ").Append(DelimitIdentifier(column.Collation));
+            }
+
             if (!string.IsNullOrEmpty(column.Operator))
             {
                 var delimitedOperator = TryParseSchema(column.Operator, out var name, out var schema)
@@ -2074,11 +2078,6 @@ private string IndexColumnList(IndexColumn[] columns, string? method)
                 builder.Append(" ").Append(delimitedOperator);
             }
 
-            if (!string.IsNullOrEmpty(column.Collation))
-            {
-                builder.Append(" COLLATE ").Append(DelimitIdentifier(column.Collation));
-            }
-
             // Of the built-in access methods, only btree (the default) supports
             // sorting, thus we only want to emit sort options for btree indexes.
             if (method is null or "btree")
@@ -2105,8 +2104,6 @@ private string IndexColumnList(IndexColumn[] columns, string? method)
                     }
                 }
             }
-
-            isFirst = false;
         }
 
         return builder.ToString();

test/EFCore.PG.FunctionalTests/Migrations/MigrationsNpgsqlTest.cs

@@ -2235,9 +2235,56 @@ await Test(
             @"CREATE INDEX ""IX_People_FirstName_LastName"" ON ""People"" (""FirstName"" text_pattern_ops, ""LastName"");");
     }
 
-    // Index collation: which collations are available on a given PostgreSQL varies (e.g. Linux vs. Windows),
-    // so we test support for this on the generated SQL only, in NpgsqlMigrationSqlGeneratorTest, and not against
-    // the database here.
+    [Fact]
+    public virtual async Task Create_index_with_collation()
+    {
+        await Test(
+            builder =>
+            {
+                builder.Entity("People", e => e.Property<string>("Name"));
+                builder.HasCollation("some_collation", locale: "POSIX", provider: "libc");
+            },
+            _ => { },
+            builder => builder.Entity("People").HasIndex("Name").UseCollation("some_collation"),
+            model =>
+            {
+                var table = Assert.Single(model.Tables);
+                var index = Assert.Single(table.Indexes);
+                Assert.Equal("some_collation", Assert.Single((IReadOnlyList<string>)index[RelationalAnnotationNames.Collation]!));
+            });
+
+        AssertSql(
+            """
+CREATE INDEX "IX_People_Name" ON "People" ("Name" COLLATE some_collation);
+""");
+    }
+
+    [Fact] // #3027
+    public virtual async Task Create_index_with_collation_and_operators()
+    {
+        await Test(
+            builder =>
+            {
+                builder.Entity("People", e => e.Property<string>("Name"));
+                builder.HasCollation("some_collation", locale: "POSIX", provider: "libc");
+            },
+            _ => { },
+            builder => builder.Entity("People").HasIndex("Name")
+                .UseCollation("some_collation")
+                .HasOperators("text_pattern_ops"),
+            model =>
+            {
+                var table = Assert.Single(model.Tables);
+                var index = Assert.Single(table.Indexes);
+                Assert.Equal("text_pattern_ops", Assert.Single((IReadOnlyList<string>)index[NpgsqlAnnotationNames.IndexOperators]!));
+                Assert.Equal("some_collation", Assert.Single((IReadOnlyList<string>)index[RelationalAnnotationNames.Collation]!));
+            });
+
+        AssertSql(
+            """
+CREATE INDEX "IX_People_Name" ON "People" ("Name" COLLATE some_collation text_pattern_ops);
+""");
+    }
 
     [Fact]
     public virtual async Task Create_index_with_null_sort_order()

test/EFCore.PG.FunctionalTests/Migrations/NpgsqlMigrationsSqlGeneratorTest.cs

@@ -473,26 +473,6 @@ public override void Sequence_restart_operation(long? startsAt)
                 : """ALTER SEQUENCE dbo."TestRestartSequenceOperation" RESTART;""");
     }
 
-    // Which index collations are available on a given PostgreSQL varies (e.g. Linux vs. Windows)
-    // so we test support for this on the generated SQL only, and not against the database in MigrationsNpgsqlTest.
-    [Fact]
-    public void CreateIndexOperation_collation()
-    {
-        Generate(
-            new CreateIndexOperation
-            {
-                Name = "IX_People_Name",
-                Table = "People",
-                Schema = "dbo",
-                Columns = new[] { "FirstName", "LastName" },
-                [RelationalAnnotationNames.Collation] = new[] { null, "de_DE" }
-            });
-
-        AssertSql(
-            @"CREATE INDEX ""IX_People_Name"" ON dbo.""People"" (""FirstName"", ""LastName"" COLLATE ""de_DE"");
-");
-    }
-
     [Theory]
     [InlineData(MigrationsSqlGenerationOptions.Default)]
     [InlineData(MigrationsSqlGenerationOptions.Idempotent)]