Skip to content

Commit

Permalink
Uniquify all variables used in SQL Server migration scripts
Browse files Browse the repository at this point in the history 
Fixes #35132
  • Loading branch information
@AndriySvyryd
AndriySvyryd committed Nov 22, 2024
1 parent 3f5dcef commit fe4a364
Show file tree
Hide file tree
Showing 9 changed files with 552 additions and 480 deletions.
12 changes: 12 additions & 0 deletions src/EFCore.Relational/Migrations/MigrationCommandListBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,18 @@ public virtual MigrationCommandListBuilder AppendLine(string value)
return this;
}

/// <summary>
/// Appends the given string to the command being built, and then starts a new line.
/// </summary>
/// <param name="value">The string to append.</param>
/// <returns>This builder so that additional calls can be chained.</returns>
public virtual MigrationCommandListBuilder AppendLine(FormattableString value)
{
_commandBuilder.AppendLine(value);

return this;
}

/// <summary>
/// Appends the given object to the command being built as multiple lines of text. That is,
/// each line in the passed string is added as a line to the command being built.
Expand Down
7 changes: 7 additions & 0 deletions src/EFCore.Relational/Storage/IRelationalCommandBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,13 @@ public interface IRelationalCommandBuilder
/// <returns>The same builder instance so that multiple calls can be chained.</returns>
IRelationalCommandBuilder Append(string value);

/// <summary>
/// Appends an object to the command text.
/// </summary>
/// <param name="value">The object to be written.</param>
/// <returns>The same builder instance so that multiple calls can be chained.</returns>
IRelationalCommandBuilder Append(FormattableString value);

/// <summary>
/// Appends a blank line to the command text.
/// </summary>
Expand Down
10 changes: 10 additions & 0 deletions src/EFCore.Relational/Storage/RelationalCommandBuilder.cs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Text;

namespace Microsoft.EntityFrameworkCore.Storage;

/// <inheritdoc />
Expand Down Expand Up @@ -71,6 +73,14 @@ public virtual IRelationalCommandBuilder Append(string value)
return this;
}

/// <inheritdoc />
public virtual IRelationalCommandBuilder Append(FormattableString value)
{
_commandTextBuilder.Append(value);

return this;
}

/// <inheritdoc />
public virtual IRelationalCommandBuilder AppendLine()
{
Expand Down
15 changes: 15 additions & 0 deletions src/EFCore.Relational/Storage/RelationalCommandBuilderExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,21 @@ public static IRelationalCommandBuilder AppendLine(
return commandBuilder;
}

/// <summary>
/// Appends an object to the command text on a new line.
/// </summary>
/// <param name="commandBuilder">The command builder.</param>
/// <param name="value">The object to be written.</param>
/// <returns>The same builder instance so that multiple calls can be chained.</returns>
public static IRelationalCommandBuilder AppendLine(
this IRelationalCommandBuilder commandBuilder,
FormattableString value)
{
commandBuilder.Append(value).AppendLine();

return commandBuilder;
}

/// <summary>
/// Appends an object, that contains multiple lines of text, to the command text.
/// Each line read from the object is appended on a new line.
Expand Down
98 changes: 56 additions & 42 deletions src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ namespace Microsoft.EntityFrameworkCore.Migrations;
public class SqlServerMigrationsSqlGenerator : MigrationsSqlGenerator
{
private IReadOnlyList<MigrationOperation> _operations = null!;
private int _variableCounter;
private int _variableCounter = -1;

private readonly ICommandBatchPreparer _commandBatchPreparer;

Expand Down Expand Up @@ -643,25 +643,23 @@ protected override void Generate(

subBuilder.Append(")");

var historyTableName = operation[SqlServerAnnotationNames.TemporalHistoryTableName] as string;
string historyTable;
if (needsExec)
{
subBuilder
.EndCommand();

var execBody = subBuilder.GetCommandList().Single().CommandText.Replace("'", "''");

var schemaVariable = Uniquify("@historyTableSchema");
builder
.AppendLine("DECLARE @historyTableSchema sysname = SCHEMA_NAME()")
.AppendLine($"DECLARE {schemaVariable} sysname = SCHEMA_NAME()")
.Append("EXEC(N'")
.Append(execBody);
}

var historyTableName = operation[SqlServerAnnotationNames.TemporalHistoryTableName] as string;
string historyTable;
if (needsExec)
{
historyTable = Dependencies.SqlGenerationHelper.DelimitIdentifier(historyTableName!);
tableCreationOptions.Add($"SYSTEM_VERSIONING = ON (HISTORY_TABLE = [' + @historyTableSchema + N'].{historyTable})");
tableCreationOptions.Add($"SYSTEM_VERSIONING = ON (HISTORY_TABLE = [' + {schemaVariable} + N'].{historyTable})");
}
else
{
Expand Down Expand Up @@ -1116,10 +1114,11 @@ protected override void Generate(
{
if (operation[SqlServerAnnotationNames.EditionOptions] is string editionOptions)
{
var dbVariable = Uniquify("@db_name");
builder
.AppendLine("BEGIN")
.AppendLine("DECLARE @db_name nvarchar(max) = DB_NAME();")
.AppendLine("EXEC(N'ALTER DATABASE [' + @db_name + '] MODIFY ( ")
.AppendLine($"DECLARE {dbVariable} nvarchar(max) = DB_NAME();")
.AppendLine($"EXEC(N'ALTER DATABASE [' + {dbVariable} + '] MODIFY ( ")
.Append(editionOptions.Replace("'", "''"))
.AppendLine(" );');")
.AppendLine("END")
Expand All @@ -1128,19 +1127,21 @@ protected override void Generate(

if (operation.Collation != operation.OldDatabase.Collation)
{
var dbVariable = Uniquify("@db_name");
builder
.AppendLine("BEGIN")
.AppendLine("DECLARE @db_name nvarchar(max) = DB_NAME();");
.AppendLine($"DECLARE {dbVariable} nvarchar(max) = DB_NAME();");

var collation = operation.Collation;
if (operation.Collation == null)
{
builder.AppendLine("DECLARE @defaultCollation nvarchar(max) = CAST(SERVERPROPERTY('Collation') AS nvarchar(max));");
var collationVariable = Uniquify("@defaultCollation");
builder.AppendLine($"DECLARE {collationVariable} nvarchar(max) = CAST(SERVERPROPERTY('Collation') AS nvarchar(max));");
collation = "' + " + collationVariable + " + N'";
}

builder
.Append("EXEC(N'ALTER DATABASE [' + @db_name + '] COLLATE ")
.Append(operation.Collation ?? "' + @defaultCollation + N'")
.AppendLine(";');")
.AppendLine($"EXEC(N'ALTER DATABASE [' + {dbVariable} + '] COLLATE {collation};');")
.AppendLine("END")
.AppendLine();
}
Expand All @@ -1167,10 +1168,11 @@ protected override void Generate(

using (builder.Indent())
{
var dbVariable = Uniquify("@db_name");
builder
.AppendLine("BEGIN")
.AppendLine("ALTER DATABASE CURRENT SET AUTO_CLOSE OFF;")
.AppendLine("DECLARE @db_name nvarchar(max) = DB_NAME();")
.AppendLine($"DECLARE {dbVariable} nvarchar(max) = DB_NAME();")
.AppendLine("DECLARE @fg_name nvarchar(max);")
.AppendLine("SELECT TOP(1) @fg_name = [name] FROM [sys].[filegroups] WHERE [type] = N'FX';")
.AppendLine()
Expand All @@ -1180,20 +1182,21 @@ protected override void Generate(
{
builder
.AppendLine("BEGIN")
.AppendLine("SET @fg_name = @db_name + N'_MODFG';")
.AppendLine($"SET @fg_name = {dbVariable} + N'_MODFG';")
.AppendLine("EXEC(N'ALTER DATABASE CURRENT ADD FILEGROUP [' + @fg_name + '] CONTAINS MEMORY_OPTIMIZED_DATA;');")
.AppendLine("END");
}

var pathVariable = Uniquify("@path");
builder
.AppendLine()
.AppendLine("DECLARE @path nvarchar(max);")
.Append("SELECT TOP(1) @path = [physical_name] FROM [sys].[database_files] ")
.AppendLine($"DECLARE {pathVariable} nvarchar(max);")
.Append($"SELECT TOP(1) {pathVariable} = [physical_name] FROM [sys].[database_files] ")
.AppendLine("WHERE charindex('\\', [physical_name]) > 0 ORDER BY [file_id];")
.AppendLine("IF (@path IS NULL)")
.IncrementIndent().AppendLine("SET @path = '\\' + @db_name;").DecrementIndent()
.AppendLine($"IF ({pathVariable} IS NULL)")
.IncrementIndent().AppendLine($"SET {pathVariable} = '\\' + {dbVariable};").DecrementIndent()
.AppendLine()
.AppendLine("DECLARE @filename nvarchar(max) = right(@path, charindex('\\', reverse(@path)) - 1);")
.AppendLine($"DECLARE @filename nvarchar(max) = right({pathVariable}, charindex('\\', reverse({pathVariable})) - 1);")
.AppendLine(
"SET @filename = REPLACE(left(@filename, len(@filename) - charindex('.', reverse(@filename))), '''', '''''') + N'_MOD';")
.AppendLine(
Expand Down Expand Up @@ -1767,10 +1770,11 @@ protected virtual void Transfer(
{
var stringTypeMapping = Dependencies.TypeMappingSource.GetMapping(typeof(string));

var schemaVariable = Uniquify("@defaultSchema");
builder
.AppendLine("DECLARE @defaultSchema sysname = SCHEMA_NAME();")
.AppendLine($"DECLARE {schemaVariable} sysname = SCHEMA_NAME();")
.Append("EXEC(")
.Append("N'ALTER SCHEMA [' + @defaultSchema + ")
.Append($"N'ALTER SCHEMA [' + {schemaVariable} + ")
.Append(
stringTypeMapping.GenerateSqlLiteral(
"] TRANSFER " + Dependencies.SqlGenerationHelper.DelimitIdentifier(name, schema) + ";"))
Expand Down Expand Up @@ -1936,7 +1940,7 @@ protected virtual void DropDefaultConstraint(
{
var stringTypeMapping = Dependencies.TypeMappingSource.GetMapping(typeof(string));

var variable = "@var" + _variableCounter++;
var variable = Uniquify("@var");

builder
.Append("DECLARE ")
Expand Down Expand Up @@ -2067,18 +2071,18 @@ protected virtual void AddDescription(
string? column = null,
bool omitVariableDeclarations = false)
{
string schemaLiteral;
var schemaLiteral = Uniquify("@defaultSchema", increase: !omitVariableDeclarations);
var descriptionVariable = Uniquify("@description", increase: false);

if (schema == null)
{
if (!omitVariableDeclarations)
{
builder.Append("DECLARE @defaultSchema AS sysname")
builder.Append($"DECLARE {schemaLiteral} AS sysname")
.AppendLine(Dependencies.SqlGenerationHelper.StatementTerminator);
builder.Append("SET @defaultSchema = SCHEMA_NAME()")
builder.Append($"SET {schemaLiteral} = SCHEMA_NAME()")
.AppendLine(Dependencies.SqlGenerationHelper.StatementTerminator);
}

schemaLiteral = "@defaultSchema";
}
else
{
Expand All @@ -2087,16 +2091,15 @@ protected virtual void AddDescription(

if (!omitVariableDeclarations)
{
builder.Append("DECLARE @description AS sql_variant")
builder.Append($"DECLARE {descriptionVariable} AS sql_variant")
.AppendLine(Dependencies.SqlGenerationHelper.StatementTerminator);
}

builder.Append("SET @description = ")
.Append(Literal(description))
builder.Append($"SET {descriptionVariable} = {Literal(description)}")
.AppendLine(Dependencies.SqlGenerationHelper.StatementTerminator);
builder
.Append("EXEC sp_addextendedproperty 'MS_Description', ")
.Append("@description")
.Append(descriptionVariable)
.Append(", 'SCHEMA', ")
.Append(schemaLiteral)
.Append(", 'TABLE', ")
Expand Down Expand Up @@ -2249,18 +2252,17 @@ protected virtual void DropDescription(
{
var stringTypeMapping = Dependencies.TypeMappingSource.GetMapping(typeof(string));

string schemaLiteral;
var schemaLiteral = Uniquify("@defaultSchema", increase: !omitVariableDeclarations);
var descriptionVariable = Uniquify("@description", increase: false);
if (schema == null)
{
if (!omitVariableDeclarations)
{
builder.Append("DECLARE @defaultSchema AS sysname")
builder.Append($"DECLARE {schemaLiteral} AS sysname")
.AppendLine(Dependencies.SqlGenerationHelper.StatementTerminator);
builder.Append("SET @defaultSchema = SCHEMA_NAME()")
builder.Append($"SET {schemaLiteral} = SCHEMA_NAME()")
.AppendLine(Dependencies.SqlGenerationHelper.StatementTerminator);
}

schemaLiteral = "@defaultSchema";
}
else
{
Expand All @@ -2269,7 +2271,7 @@ protected virtual void DropDescription(

if (!omitVariableDeclarations)
{
builder.Append("DECLARE @description AS sql_variant")
builder.Append($"DECLARE {descriptionVariable} AS sql_variant")
.AppendLine(Dependencies.SqlGenerationHelper.StatementTerminator);
}

Expand Down Expand Up @@ -2379,6 +2381,16 @@ private static bool HasDifferences(IEnumerable<IAnnotation> source, IEnumerable<
return count != targetAnnotations.Count;
}

private string Uniquify(string variableName, bool increase = true)
{
if (increase)
{
_variableCounter++;
}

return _variableCounter == 0 ? variableName : variableName + _variableCounter;
}

private IReadOnlyList<MigrationOperation> RewriteOperations(
IReadOnlyList<MigrationOperation> migrationOperations,
IModel? model,
Expand Down Expand Up @@ -3071,10 +3083,12 @@ void EnableVersioning(string table, string? schema, string historyTableName, str
{
var stringBuilder = new StringBuilder();

string? schemaVariable = null;
if (historyTableSchema == null)
{
schemaVariable = Uniquify("@historyTableSchema");
// need to run command using EXEC to inject default schema
stringBuilder.AppendLine("DECLARE @historyTableSchema sysname = SCHEMA_NAME()");
stringBuilder.AppendLine($"DECLARE {schemaVariable} sysname = SCHEMA_NAME()");
stringBuilder.Append("EXEC(N'");
}

Expand All @@ -3093,7 +3107,7 @@ void EnableVersioning(string table, string? schema, string historyTableName, str
else
{
stringBuilder.AppendLine(
$" SET (SYSTEM_VERSIONING = ON (HISTORY_TABLE = [' + @historyTableSchema + '].{historyTable}))')");
$" SET (SYSTEM_VERSIONING = ON (HISTORY_TABLE = [' + {schemaVariable} + '].{historyTable}))')");
}

operations.Add(
Expand Down
Loading

0 comments on commit fe4a364

Please sign in to comment.