Skip to content

Commit

Permalink
Fix to #35108 - Temporal table migration regression from EF Core 8 to…
Browse files Browse the repository at this point in the history 
… 9 (#35289)

In 9 we changed the way we process migration of temporal tables. One of the changes was drastically reducing the number of annotations for columns which are part of temporal tables. This however caused regressions for cases where migration code was created using EF8 (and containing those legacy annotations) but then executed using EF9 tooling. Specifically, extra annotations were generating a number of superfluous Alter Column operations (which were only modifying those annotations). In EF8 we had logic to weed out those operations, but it was removed in EF9.

Fix is to remove all the legacy annotations on column operations before we start processing them. We no longer rely on them, but rather use annotations on Table operations and/or relational model. The only exception is CreateColumnOperation, so for it we convert old annotations to TemporalIsPeriodStartColumn and TemporalIsPeriodEndColumn where appropriate. Also, we are bringing back logic from EF8 which removed unnecessary AlterColumnOperations if the old and new columns are the same after the legacy temporal annotations have been removed.
  • Loading branch information
@maumar
maumar authored Dec 10, 2024
1 parent 7937010 commit 4b3e12f
Show file tree
Hide file tree
Showing 2 changed files with 1,202 additions and 11 deletions.
141 changes: 130 additions & 11 deletions src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections;
using System.Globalization;
using System.Text;
using Microsoft.EntityFrameworkCore.SqlServer.Internal;
Expand Down Expand Up @@ -1599,17 +1600,6 @@ protected override void ColumnDefinition(
var isPeriodStartColumn = operation[SqlServerAnnotationNames.TemporalIsPeriodStartColumn] as bool? == true;
var isPeriodEndColumn = operation[SqlServerAnnotationNames.TemporalIsPeriodEndColumn] as bool? == true;

// falling back to legacy annotations, in case the migration was generated using pre-9.0 bits
if (!isPeriodStartColumn && !isPeriodEndColumn)
{
if (operation[SqlServerAnnotationNames.TemporalPeriodStartColumnName] is string periodStartColumnName
&& operation[SqlServerAnnotationNames.TemporalPeriodEndColumnName] is string periodEndColumnName)
{
isPeriodStartColumn = operation.Name == periodStartColumnName;
isPeriodEndColumn = operation.Name == periodEndColumnName;
}
}

if (isPeriodStartColumn || isPeriodEndColumn)
{
builder.Append(" GENERATED ALWAYS AS ROW ");
Expand Down Expand Up @@ -2363,11 +2353,140 @@ private string Uniquify(string variableName, bool increase = true)
return _variableCounter == 0 ? variableName : variableName + _variableCounter;
}

private IReadOnlyList<MigrationOperation> FixLegacyTemporalAnnotations(IReadOnlyList<MigrationOperation> migrationOperations)
{
// short-circuit for non-temporal migrations (which is the majority)
if (migrationOperations.All(o => o[SqlServerAnnotationNames.IsTemporal] as bool? != true))
{
return migrationOperations;
}

var resultOperations = new List<MigrationOperation>(migrationOperations.Count);
foreach (var migrationOperation in migrationOperations)
{
var isTemporal = migrationOperation[SqlServerAnnotationNames.IsTemporal] as bool? == true;
if (!isTemporal)
{
resultOperations.Add(migrationOperation);
continue;
}

switch (migrationOperation)
{
case CreateTableOperation createTableOperation:

foreach (var column in createTableOperation.Columns)
{
NormalizeTemporalAnnotationsForAddColumnOperation(column);
}

resultOperations.Add(migrationOperation);
break;

case AddColumnOperation addColumnOperation:
NormalizeTemporalAnnotationsForAddColumnOperation(addColumnOperation);
resultOperations.Add(addColumnOperation);
break;

case AlterColumnOperation alterColumnOperation:
RemoveLegacyTemporalColumnAnnotations(alterColumnOperation);
RemoveLegacyTemporalColumnAnnotations(alterColumnOperation.OldColumn);
if (!CanSkipAlterColumnOperation(alterColumnOperation, alterColumnOperation.OldColumn))
{
resultOperations.Add(alterColumnOperation);
}

break;

case DropColumnOperation dropColumnOperation:
RemoveLegacyTemporalColumnAnnotations(dropColumnOperation);
resultOperations.Add(dropColumnOperation);
break;

case RenameColumnOperation renameColumnOperation:
RemoveLegacyTemporalColumnAnnotations(renameColumnOperation);
resultOperations.Add(renameColumnOperation);
break;

default:
resultOperations.Add(migrationOperation);
break;
}
}

return resultOperations;

static void NormalizeTemporalAnnotationsForAddColumnOperation(AddColumnOperation addColumnOperation)
{
var periodStartColumnName = addColumnOperation[SqlServerAnnotationNames.TemporalPeriodStartColumnName] as string;
var periodEndColumnName = addColumnOperation[SqlServerAnnotationNames.TemporalPeriodEndColumnName] as string;
if (periodStartColumnName == addColumnOperation.Name)
{
addColumnOperation.AddAnnotation(SqlServerAnnotationNames.TemporalIsPeriodStartColumn, true);
}
else if (periodEndColumnName == addColumnOperation.Name)
{
addColumnOperation.AddAnnotation(SqlServerAnnotationNames.TemporalIsPeriodEndColumn, true);
}

RemoveLegacyTemporalColumnAnnotations(addColumnOperation);
}

static void RemoveLegacyTemporalColumnAnnotations(MigrationOperation operation)
{
operation.RemoveAnnotation(SqlServerAnnotationNames.IsTemporal);
operation.RemoveAnnotation(SqlServerAnnotationNames.TemporalHistoryTableName);
operation.RemoveAnnotation(SqlServerAnnotationNames.TemporalHistoryTableSchema);
operation.RemoveAnnotation(SqlServerAnnotationNames.TemporalPeriodStartColumnName);
operation.RemoveAnnotation(SqlServerAnnotationNames.TemporalPeriodEndColumnName);
}

static bool CanSkipAlterColumnOperation(ColumnOperation column, ColumnOperation oldColumn)
=> ColumnPropertiesAreTheSame(column, oldColumn) && AnnotationsAreTheSame(column, oldColumn);

// don't compare name, table or schema - they are not being set in the model differ (since they should always be the same)
static bool ColumnPropertiesAreTheSame(ColumnOperation column, ColumnOperation oldColumn)
=> column.ClrType == oldColumn.ClrType
&& column.Collation == oldColumn.Collation
&& column.ColumnType == oldColumn.ColumnType
&& column.Comment == oldColumn.Comment
&& column.ComputedColumnSql == oldColumn.ComputedColumnSql
&& Equals(column.DefaultValue, oldColumn.DefaultValue)
&& column.DefaultValueSql == oldColumn.DefaultValueSql
&& column.IsDestructiveChange == oldColumn.IsDestructiveChange
&& column.IsFixedLength == oldColumn.IsFixedLength
&& column.IsNullable == oldColumn.IsNullable
&& column.IsReadOnly == oldColumn.IsReadOnly
&& column.IsRowVersion == oldColumn.IsRowVersion
&& column.IsStored == oldColumn.IsStored
&& column.IsUnicode == oldColumn.IsUnicode
&& column.MaxLength == oldColumn.MaxLength
&& column.Precision == oldColumn.Precision
&& column.Scale == oldColumn.Scale;

static bool AnnotationsAreTheSame(ColumnOperation column, ColumnOperation oldColumn)
{
var columnAnnotations = column.GetAnnotations().ToList();
var oldColumnAnnotations = oldColumn.GetAnnotations().ToList();

if (columnAnnotations.Count != oldColumnAnnotations.Count)
{
return false;
}

return columnAnnotations.Zip(oldColumnAnnotations)
.All(x => x.First.Name == x.Second.Name
&& StructuralComparisons.StructuralEqualityComparer.Equals(x.First.Value, x.Second.Value));
}
}

private IReadOnlyList<MigrationOperation> RewriteOperations(
IReadOnlyList<MigrationOperation> migrationOperations,
IModel? model,
MigrationsSqlGenerationOptions options)
{
migrationOperations = FixLegacyTemporalAnnotations(migrationOperations);

var operations = new List<MigrationOperation>();
var availableSchemas = new List<string>();

Expand Down
Loading

0 comments on commit 4b3e12f

Please sign in to comment.