Skip to content

Quote collation names by default #37463

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
roji merged 1 commit into from
Jan 8, 2026
Merged

Quote collation names by default #37463

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/EFCore.Relational/Migrations/MigrationsSqlGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1367,7 +1367,7 @@ protected virtual void ColumnDefinition(
{
builder
.Append(" COLLATE ")
.Append(operation.Collation);
.Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Collation));
}

builder.Append(operation.IsNullable ? " NULL" : " NOT NULL");
Expand Down
6 changes: 5 additions & 1 deletion src/EFCore.Relational/Query/QuerySqlGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -733,9 +733,13 @@ protected override Expression VisitCollate(CollateExpression collateExpression)
{
Visit(collateExpression.Operand);

// In some databases (e.g. SQL Server), collations aren't regular identifiers (like column or table names), and so shouldn't be
// quoted. However, we still default to quoting to prevent cases where arbitrary collation name inputs get integrated into the SQL.
// Providers which don't support quoting of collation names should override this method to provide sanitization logic,
// e.g. throwing if the name contains any but a restricted set of characters.
_relationalCommandBuilder
.Append(" COLLATE ")
.Append(collateExpression.Collation);
.Append(_sqlGenerationHelper.DelimitIdentifier(collateExpression.Collation));

return collateExpression;
}
Expand Down
12 changes: 12 additions & 0 deletions src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1646,6 +1646,18 @@ protected override void ColumnDefinition(

if (operation.Collation != null)
{
// SQL Server collation docs: https://learn.microsoft.com/sql/relational-databases/collations/collation-and-unicode-support

// The default behavior in MigrationsSqlGenerator is to quote collation names, but SQL Server does not support that.
// Instead, make sure the collation name only contains a restricted set of characters.
foreach (var c in operation.Collation)
{
if (!char.IsLetterOrDigit(c) && c != '_')
{
throw new InvalidOperationException(SqlServerStrings.InvalidCollationName(operation.Collation));
}
}

builder
.Append(" COLLATE ")
.Append(operation.Collation);
Expand Down
8 changes: 8 additions & 0 deletions src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/EFCore.SqlServer/Properties/SqlServerStrings.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,9 @@
<data name="IndexTableRequired" xml:space="preserve">
<value>SQL Server requires the table name to be specified for index operations. Specify table name in calls to 'MigrationBuilder.RenameIndex' and 'DropIndex'.</value>
</data>
<data name="InvalidCollationName" xml:space="preserve">
<value>Collation name '{collation}' is invalid; collation names may only contain alphanumeric characters and underscores.</value>
</data>
<data name="InvalidColumnNameForFreeText" xml:space="preserve">
<value>The expression passed to the 'propertyReference' parameter of the 'FreeText' method is not a valid reference to a property. The expression must represent a reference to a full-text indexed property on the object referenced in the from clause: 'from e in context.Entities where EF.Functions.FreeText(e.SomeProperty, textToSearchFor) select e'</value>
</data>
Expand Down
29 changes: 29 additions & 0 deletions src/EFCore.SqlServer/Query/Internal/SqlServerQuerySqlGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,35 @@ protected override bool TryGenerateWithoutWrappingSelect(SelectExpression select
=> selectExpression.Tables is not [ValuesExpression]
&& base.TryGenerateWithoutWrappingSelect(selectExpression);

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
protected override Expression VisitCollate(CollateExpression collateExpression)
{
Visit(collateExpression.Operand);

// SQL Server collation docs: https://learn.microsoft.com/sql/relational-databases/collations/collation-and-unicode-support

// The default behavior in QuerySqlGenerator is to quote collation names, but SQL Server does not support that.
// Instead, make sure the collation name only contains a restricted set of characters.
foreach (var c in collateExpression.Collation)
{
if (!char.IsLetterOrDigit(c) && c != '_')
{
throw new InvalidOperationException(SqlServerStrings.InvalidCollationName(collateExpression.Collation));
}
}

Sql
.Append(" COLLATE ")
.Append(collateExpression.Collation);

return collateExpression;
}

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
Expand Down
6 changes: 3 additions & 3 deletions src/EFCore.Sqlite.Core/Migrations/SqliteMigrationsSqlGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,13 +377,13 @@ private IReadOnlyList<MigrationOperation> RewriteOperations(
// Skip autoincrement primary key columns that are being added in this migration
var isAutoincrement = column.FindAnnotation(SqliteAnnotationNames.Autoincrement)?.Value as bool? == true;
var isPrimaryKey = column.Table.PrimaryKey?.Columns.Contains(column) == true;

if (isAutoincrement && isPrimaryKey)
{
// Check if this column is being added in the current migration
var isNewColumn = migrationOperations.OfType<AddColumnOperation>()
.Any(op => op.Table == key.Table && op.Schema == key.Schema && op.Name == column.Name);

if (isNewColumn)
{
continue; // Skip newly added autoincrement columns
Expand Down Expand Up @@ -976,7 +976,7 @@ protected override void ComputedColumnDefinition(
{
builder
.Append(" COLLATE ")
.Append(operation.Collation);
.Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Collation));
}
}

Expand Down
12 changes: 12 additions & 0 deletions test/EFCore.SqlServer.FunctionalTests/Query/NorthwindDbFunctionsQuerySqlServerTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using Microsoft.Data.SqlClient;
using Microsoft.EntityFrameworkCore.SqlServer.Internal;
using Microsoft.EntityFrameworkCore.TestModels.Northwind;

// ReSharper disable InconsistentNaming
Expand Down Expand Up @@ -133,6 +134,17 @@ WHERE [c].[Region] IS NULL
""");
}

[ConditionalTheory, MemberData(nameof(IsAsyncData))]
public virtual async Task Collate_with_invalid_chars_throws(bool async)
{
using var context = CreateContext();

var exception = await Assert.ThrowsAsync<InvalidOperationException>(
() => context.Customers.CountAsync(c => EF.Functions.Collate(c.ContactName, "Invalid]Collation") == "test"));

Assert.Equal(SqlServerStrings.InvalidCollationName("Invalid]Collation"), exception.Message);
}

public override Task Least(bool async)
=> AssertTranslationFailed(() => base.Least(async));

Expand Down
6 changes: 3 additions & 3 deletions test/EFCore.Sqlite.FunctionalTests/BuiltInDataTypesSqliteTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1476,7 +1476,7 @@ public virtual void Can_query_OrderBy_of_converted_types()
SELECT "b"."Id"
FROM "BuiltInDataTypes" AS "b"
WHERE "b"."PartitionId" = 207
ORDER BY "b"."TestDecimal" COLLATE EF_DECIMAL
ORDER BY "b"."TestDecimal" COLLATE "EF_DECIMAL"
LIMIT 1
""");

Expand Down Expand Up @@ -1532,7 +1532,7 @@ public virtual void Can_query_ThenBy_of_converted_types()
SELECT "b"."Id"
FROM "BuiltInDataTypes" AS "b"
WHERE "b"."PartitionId" = 208
ORDER BY "b"."PartitionId", "b"."TestDecimal" COLLATE EF_DECIMAL
ORDER BY "b"."PartitionId", "b"."TestDecimal" COLLATE "EF_DECIMAL"
LIMIT 1
""");

Expand Down Expand Up @@ -1591,7 +1591,7 @@ public virtual void Can_query_OrderBy_decimal_with_Turkish_culture()
"""
SELECT "b"."Id", "b"."TestDecimal"
FROM "BuiltInDataTypes" AS "b"
ORDER BY "b"."TestDecimal" COLLATE EF_DECIMAL
ORDER BY "b"."TestDecimal" COLLATE "EF_DECIMAL"
""",
//
"""
Expand Down
20 changes: 10 additions & 10 deletions test/EFCore.Sqlite.FunctionalTests/Migrations/MigrationsSqliteTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public override async Task Create_table_all_settings()
-- Employer ID comment
"EmployerId" INTEGER NOT NULL,

"SSN" TEXT COLLATE NOCASE NOT NULL,
"SSN" TEXT COLLATE "NOCASE" NOT NULL,
CONSTRAINT "AK_People_SSN" UNIQUE ("SSN"),
CONSTRAINT "CK_People_EmployerId" CHECK ("EmployerId" > 0),
CONSTRAINT "FK_People_Employers_EmployerId" FOREIGN KEY ("EmployerId") REFERENCES "Employers" ("Id") ON DELETE CASCADE
Expand Down Expand Up @@ -498,7 +498,7 @@ public override async Task Add_column_with_collation()

AssertSql(
"""
ALTER TABLE "People" ADD "Name" TEXT COLLATE NOCASE NULL;
ALTER TABLE "People" ADD "Name" TEXT COLLATE "NOCASE" NULL;
""");
}

Expand All @@ -508,8 +508,8 @@ public override async Task Add_column_computed_with_collation(bool stored)

AssertSql(
stored
? """ALTER TABLE "People" ADD "Name" AS ('hello') STORED COLLATE NOCASE;"""
: """ALTER TABLE "People" ADD "Name" AS ('hello') COLLATE NOCASE;""");
? """ALTER TABLE "People" ADD "Name" AS ('hello') STORED COLLATE "NOCASE";"""
: """ALTER TABLE "People" ADD "Name" AS ('hello') COLLATE "NOCASE";""");
}

public override async Task Add_column_with_check_constraint()
Expand Down Expand Up @@ -953,7 +953,7 @@ public override async Task Alter_column_set_collation()
AssertSql(
"""
CREATE TABLE "ef_temp_People" (
"Name" TEXT COLLATE NOCASE NULL
"Name" TEXT COLLATE "NOCASE" NULL
);
""",
//
Expand Down Expand Up @@ -2123,7 +2123,7 @@ await Test(
var table = Assert.Single(model.Tables);
Assert.Equal("Person", table.Name);
Assert.Equal(2, table.Columns.Count());

var idColumn = Assert.Single(table.Columns, c => c.Name == "Id");
Assert.False(idColumn.IsNullable);
});
Expand Down Expand Up @@ -2187,7 +2187,7 @@ await Test(
var table = Assert.Single(model.Tables);
Assert.Equal("ProductWithStrongId", table.Name);
Assert.Equal(2, table.Columns.Count());

var idColumn = Assert.Single(table.Columns, c => c.Name == "Id");
Assert.False(idColumn.IsNullable);
});
Expand Down Expand Up @@ -2220,7 +2220,7 @@ await Test(
var table = Assert.Single(model.Tables);
Assert.Equal("ProductWithStrongId", table.Name);
Assert.Equal(2, table.Columns.Count());

var idColumn = Assert.Single(table.Columns, c => c.Name == "Id");
Assert.False(idColumn.IsNullable);
});
Expand Down Expand Up @@ -2252,7 +2252,7 @@ await Test(
var table = Assert.Single(model.Tables);
Assert.Equal("CompositeEntity", table.Name);
Assert.Equal(2, table.Columns.Count());

var id1Column = Assert.Single(table.Columns, c => c.Name == "Id1");
Assert.False(id1Column.IsNullable);
var id2Column = Assert.Single(table.Columns, c => c.Name == "Id2");
Expand Down Expand Up @@ -2294,7 +2294,7 @@ await Test(
var table = Assert.Single(model.Tables);
Assert.Equal("Product", table.Name);
Assert.Equal(2, table.Columns.Count());

var idColumn = Assert.Single(table.Columns, c => c.Name == "Id");
Assert.False(idColumn.IsNullable);
});
Expand Down