Skip to content

Fix to #35108 - Temporal table migration regression from EF Core 8 to 9 #35283

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
merged 1 commit into from
Dec 7, 2024
Merged

Fix to #35108 - Temporal table migration regression from EF Core 8 to 9 #35283

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
141 changes: 130 additions & 11 deletions src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs
View file
Open in desktop
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 @@ -1627,17 +1628,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 @@ -2391,11 +2381,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))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be safe only call this if any annotations were removed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's already the case - we short-circuit all operations that don't contain SqlServerAnnotationNames.IsTemporal set to true. For AlterColumnOperation (actually all column operations) IsTemporal is only set for legacy scenario

{
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