Skip to content

Commit

Permalink
Fix index creation when both collation and operators are specified (#…
Browse files Browse the repository at this point in the history
…3028)

Fixes #3027
  • Loading branch information
roji authored Dec 17, 2023
1 parent f9e3428 commit 6731019
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 34 deletions.
19 changes: 8 additions & 11 deletions src/EFCore.PG/Migrations/NpgsqlMigrationsSqlGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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")
Expand All @@ -2105,8 +2104,6 @@ private string IndexColumnList(IndexColumn[] columns, string? method)
}
}
}

isFirst = false;
}

return builder.ToString();
Expand Down
53 changes: 50 additions & 3 deletions test/EFCore.PG.FunctionalTests/Migrations/MigrationsNpgsqlTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down

0 comments on commit 6731019

Please sign in to comment.