Skip to content

Commit

Permalink
Fix SQL Server memory-optimized + temporal tables
Browse files Browse the repository at this point in the history 
Fixes dotnet#27592
  • Loading branch information
@roji
roji committed Mar 10, 2022
1 parent ac143e6 commit 452b788
Show file tree
Hide file tree
Showing 2 changed files with 222 additions and 44 deletions.
132 changes: 88 additions & 44 deletions src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -530,12 +530,16 @@ protected override void Generate(
throw new ArgumentException(SqlServerStrings.CannotProduceUnterminatedSQLWithComments(nameof(CreateTableOperation)));
}

var needsExec = false;

var tableCreationOptions = new List<string>();

if (operation[SqlServerAnnotationNames.IsTemporal] as bool? == true)
{
var historyTableSchema = operation[SqlServerAnnotationNames.TemporalHistoryTableSchema] as string
?? model?.GetDefaultSchema();

var needsExec = historyTableSchema == null;
needsExec = historyTableSchema == null;
var subBuilder = needsExec
? new MigrationCommandListBuilder(Dependencies)
: builder;
Expand All @@ -557,6 +561,8 @@ protected override void Generate(
subBuilder.AppendLine($"PERIOD FOR SYSTEM_TIME({start}, {end})");
}

subBuilder.Append(")");

if (needsExec)
{
subBuilder
Expand All @@ -575,12 +581,12 @@ protected override void Generate(
if (needsExec)
{
historyTable = Dependencies.SqlGenerationHelper.DelimitIdentifier(historyTableName!);
builder.Append($") WITH (SYSTEM_VERSIONING = ON (HISTORY_TABLE = [' + @historyTableSchema + N'].{historyTable}))')");
tableCreationOptions.Add($"SYSTEM_VERSIONING = ON (HISTORY_TABLE = [' + @historyTableSchema + N'].{historyTable})");
}
else
{
historyTable = Dependencies.SqlGenerationHelper.DelimitIdentifier(historyTableName!, historyTableSchema);
builder.Append($") WITH (SYSTEM_VERSIONING = ON (HISTORY_TABLE = {historyTable}))");
tableCreationOptions.Add($"SYSTEM_VERSIONING = ON (HISTORY_TABLE = {historyTable})");
}
}
else
Expand All @@ -591,17 +597,46 @@ protected override void Generate(
var memoryOptimized = IsMemoryOptimized(operation);
if (memoryOptimized)
{
builder.AppendLine();
using (builder.Indent())
tableCreationOptions.Add("MEMORY_OPTIMIZED = ON");
}

if (tableCreationOptions.Count > 0)
{
builder.Append(" WITH (");
if (tableCreationOptions.Count == 1)
{
builder.AppendLine("WITH");
builder
.Append(tableCreationOptions[0])
.Append(")");
}
else
{
builder.AppendLine();

using (builder.Indent())
{
builder.Append("(MEMORY_OPTIMIZED = ON)");
for (var i = 0; i < tableCreationOptions.Count; i++)
{
builder.Append(tableCreationOptions[i]);

if (i < tableCreationOptions.Count - 1)
{
builder.Append(",");
}

builder.AppendLine();
}
}

builder.Append(")");
}
}

if (needsExec)
{
builder.Append("')");
}

if (hasComments)
{
Check.DebugAssert(terminate, "terminate is false but there are comments");
Expand Down Expand Up @@ -2232,8 +2267,8 @@ private IReadOnlyList<MigrationOperation> RewriteOperations(
{
var operations = new List<MigrationOperation>();

var versioningMap = new Dictionary<(string?, string?), (string, string?)>();
var periodMap = new Dictionary<(string?, string?), (string, string)>();
var versioningMap = new Dictionary<(string?, string?), (string, string?, bool)>();
var periodMap = new Dictionary<(string?, string?), (string, string, bool)>();
var availableSchemas = new List<string>();

foreach (var operation in migrationOperations)
Expand All @@ -2255,6 +2290,8 @@ private IReadOnlyList<MigrationOperation> RewriteOperations(
schema = tableMigrationOperation.Schema;
}

var suppressTransaction = table is not null && IsMemoryOptimized(operation, model, schema, table);

schema ??= model?.GetDefaultSchema();
var historyTableName = operation[SqlServerAnnotationNames.TemporalHistoryTableName] as string;
var historyTableSchema = operation[SqlServerAnnotationNames.TemporalHistoryTableSchema] as string
Expand All @@ -2277,22 +2314,22 @@ private IReadOnlyList<MigrationOperation> RewriteOperations(
break;

case DropTableOperation:
DisableVersioning(table!, schema, historyTableName!, historyTableSchema);
DisableVersioning(table!, schema, historyTableName!, historyTableSchema, suppressTransaction);
operations.Add(operation);

versioningMap.Remove((table, schema));
periodMap.Remove((table, schema));
break;

case RenameTableOperation renameTableOperation:
DisableVersioning(table!, schema, historyTableName!, historyTableSchema);
DisableVersioning(table!, schema, historyTableName!, historyTableSchema, suppressTransaction);
operations.Add(operation);

// since table was renamed, remove old entry and add new entry
// marked as versioning disabled, so we enable it in the end for the new table
versioningMap.Remove((table, schema));
versioningMap[(renameTableOperation.NewName, renameTableOperation.NewSchema)] =
(historyTableName!, historyTableSchema);
(historyTableName!, historyTableSchema, suppressTransaction);

// same thing for disabled system period - remove one associated with old table and add one for the new table
if (periodMap.TryGetValue((table, schema), out var result))
Expand All @@ -2308,9 +2345,9 @@ private IReadOnlyList<MigrationOperation> RewriteOperations(
if (!oldIsTemporal)
{
periodMap[(alterTableOperation.Name, alterTableOperation.Schema)] =
(periodStartColumnName!, periodEndColumnName!);
(periodStartColumnName!, periodEndColumnName!, suppressTransaction);
versioningMap[(alterTableOperation.Name, alterTableOperation.Schema)] =
(historyTableName!, historyTableSchema);
(historyTableName!, historyTableSchema, suppressTransaction);
}
else
{
Expand Down Expand Up @@ -2343,7 +2380,7 @@ alterTableOperation.OldTable[SqlServerAnnotationNames.TemporalHistoryTableSchema
if (versioningMap.ContainsKey((alterTableOperation.Name, alterTableOperation.Schema)))
{
versioningMap[(alterTableOperation.Name, alterTableOperation.Schema)] =
(historyTableName!, historyTableSchema);
(historyTableName!, historyTableSchema, suppressTransaction);
}
}
}
Expand Down Expand Up @@ -2376,19 +2413,19 @@ alterTableOperation.OldTable[SqlServerAnnotationNames.TemporalHistoryTableSchema

case DropPrimaryKeyOperation:
case AddPrimaryKeyOperation:
DisableVersioning(table!, schema, historyTableName!, historyTableSchema);
DisableVersioning(table!, schema, historyTableName!, historyTableSchema, suppressTransaction);
operations.Add(operation);
break;

case DropColumnOperation dropColumnOperation:
DisableVersioning(table!, schema, historyTableName!, historyTableSchema);
DisableVersioning(table!, schema, historyTableName!, historyTableSchema, suppressTransaction);
if (dropColumnOperation.Name == periodStartColumnName
|| dropColumnOperation.Name == periodEndColumnName)
{
// period columns can be null here - it doesn't really matter since we are never enabling the period back
// if we remove the period columns, it means we will be dropping the table also or at least convert it back to regular
// which will clear the entry in the periodMap for this table
DisablePeriod(table!, schema, periodStartColumnName!, periodEndColumnName!);
// if we remove the period columns, it means we will be dropping the table also or at least convert it back to
// regular which will clear the entry in the periodMap for this table
DisablePeriod(table!, schema, periodStartColumnName!, periodEndColumnName!, suppressTransaction);
}

operations.Add(operation);
Expand All @@ -2397,16 +2434,17 @@ alterTableOperation.OldTable[SqlServerAnnotationNames.TemporalHistoryTableSchema

case AddColumnOperation addColumnOperation:
// when adding a period column, we need to add it as a normal column first, and only later enable period
// removing the period information now, so that when we generate SQL that adds the column we won't be making them auto generated as period
// it won't work, unless period is enabled
// but we can't enable period without adding the columns first - chicken and egg
// removing the period information now, so that when we generate SQL that adds the column we won't be making them
// auto generated as period it won't work, unless period is enabled but we can't enable period without adding the
// columns first - chicken and egg
if (addColumnOperation[SqlServerAnnotationNames.IsTemporal] as bool? == true)
{
addColumnOperation.RemoveAnnotation(SqlServerAnnotationNames.IsTemporal);
addColumnOperation.RemoveAnnotation(SqlServerAnnotationNames.TemporalPeriodStartColumnName);
addColumnOperation.RemoveAnnotation(SqlServerAnnotationNames.TemporalPeriodEndColumnName);

// model differ adds default value, but for period end we need to replace it with the correct one - DateTime.MaxValue
// model differ adds default value, but for period end we need to replace it with the correct one -
// DateTime.MaxValue
if (addColumnOperation.Name == periodEndColumnName)
{
addColumnOperation.DefaultValue = DateTime.MaxValue;
Expand Down Expand Up @@ -2437,9 +2475,13 @@ alterTableOperation.OldTable[SqlServerAnnotationNames.TemporalHistoryTableSchema
alterTableOperation.OldTable[SqlServerAnnotationNames.TemporalPeriodStartColumnName] as string;
var periodEndColumnName =
alterTableOperation.OldTable[SqlServerAnnotationNames.TemporalPeriodEndColumnName] as string;
var suppressTransaction = IsMemoryOptimized(operation, model, alterTableOperation.Schema, alterTableOperation.Name);

DisableVersioning(alterTableOperation.Name, alterTableOperation.Schema, historyTableName!, historyTableSchema);
DisablePeriod(alterTableOperation.Name, alterTableOperation.Schema, periodStartColumnName!, periodEndColumnName!);
DisableVersioning(
alterTableOperation.Name, alterTableOperation.Schema, historyTableName!, historyTableSchema, suppressTransaction);
DisablePeriod(
alterTableOperation.Name, alterTableOperation.Schema, periodStartColumnName!, periodEndColumnName!,
suppressTransaction);

if (historyTableName != null)
{
Expand Down Expand Up @@ -2470,25 +2512,23 @@ alterTableOperation.OldTable[SqlServerAnnotationNames.TemporalHistoryTableSchema
}
}

foreach (var (key, value) in periodMap)
foreach (var ((table, schema), (periodStartColumnName, periodEndColumnName, suppressTransaction)) in periodMap)
{
EnablePeriod(key.Item1!, key.Item2, value.Item1, value.Item2);
EnablePeriod(table!, schema, periodStartColumnName, periodEndColumnName, suppressTransaction);
}

foreach (var (key, value) in versioningMap)
foreach (var ((table, schema), (historyTableName, historyTableSchema, suppressTransaction)) in versioningMap)
{
EnableVersioning(
key.Item1!, key.Item2, value.Item1,
value.Item2);
EnableVersioning(table!, schema, historyTableName, historyTableSchema, suppressTransaction);
}

return operations;

void DisableVersioning(string table, string? schema, string historyTableName, string? historyTableSchema)
void DisableVersioning(string table, string? schema, string historyTableName, string? historyTableSchema, bool suppressTransaction)
{
if (!versioningMap.TryGetValue((table, schema), out _))
{
versioningMap[(table, schema)] = (historyTableName, historyTableSchema);
versioningMap[(table, schema)] = (historyTableName, historyTableSchema, suppressTransaction);

operations.Add(
new SqlOperation
Expand All @@ -2497,12 +2537,13 @@ void DisableVersioning(string table, string? schema, string historyTableName, st
.Append("ALTER TABLE ")
.Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(table, schema))
.AppendLine(" SET (SYSTEM_VERSIONING = OFF)")
.ToString()
.ToString(),
SuppressTransaction = suppressTransaction
});
}
}

void EnableVersioning(string table, string? schema, string historyTableName, string? historyTableSchema)
void EnableVersioning(string table, string? schema, string historyTableName, string? historyTableSchema, bool suppressTransaction)
{
var stringBuilder = new StringBuilder();

Expand Down Expand Up @@ -2532,14 +2573,14 @@ void EnableVersioning(string table, string? schema, string historyTableName, str
}

operations.Add(
new SqlOperation { Sql = stringBuilder.ToString() });
new SqlOperation { Sql = stringBuilder.ToString(), SuppressTransaction = suppressTransaction });
}

void DisablePeriod(string table, string? schema, string periodStartColumnName, string periodEndColumnName)
void DisablePeriod(string table, string? schema, string periodStartColumnName, string periodEndColumnName, bool suppressTransaction)
{
if (!periodMap.TryGetValue((table, schema), out _))
{
periodMap[(table, schema)] = (periodStartColumnName, periodEndColumnName);
periodMap[(table, schema)] = (periodStartColumnName, periodEndColumnName, suppressTransaction);

operations.Add(
new SqlOperation
Expand All @@ -2548,12 +2589,13 @@ void DisablePeriod(string table, string? schema, string periodStartColumnName, s
.Append("ALTER TABLE ")
.Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(table, schema))
.AppendLine(" DROP PERIOD FOR SYSTEM_TIME")
.ToString()
.ToString(),
SuppressTransaction = suppressTransaction
});
}
}

void EnablePeriod(string table, string? schema, string periodStartColumnName, string periodEndColumnName)
void EnablePeriod(string table, string? schema, string periodStartColumnName, string periodEndColumnName, bool suppressTransaction)
{
var addPeriodSql = new StringBuilder()
.Append("ALTER TABLE ")
Expand All @@ -2575,7 +2617,7 @@ void EnablePeriod(string table, string? schema, string periodStartColumnName, st
}

operations.Add(
new SqlOperation { Sql = addPeriodSql });
new SqlOperation { Sql = addPeriodSql, SuppressTransaction = suppressTransaction });

operations.Add(
new SqlOperation
Expand All @@ -2586,7 +2628,8 @@ void EnablePeriod(string table, string? schema, string periodStartColumnName, st
.Append(" ALTER COLUMN ")
.Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(periodStartColumnName))
.Append(" ADD HIDDEN")
.ToString()
.ToString(),
SuppressTransaction = suppressTransaction
});

operations.Add(
Expand All @@ -2598,7 +2641,8 @@ void EnablePeriod(string table, string? schema, string periodStartColumnName, st
.Append(" ALTER COLUMN ")
.Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(periodEndColumnName))
.Append(" ADD HIDDEN")
.ToString()
.ToString(),
SuppressTransaction = suppressTransaction
});
}

Expand Down
Loading

0 comments on commit 452b788

Please sign in to comment.