Skip to content

Commit

Permalink
Temporal table migrations refactor. (#32239)
Browse files Browse the repository at this point in the history
Before we used to put temporal annotations on temporal tables and all their columns, so that it's easier to process. Problem was that this would generate very noisy migrations when converting from regular table to temporal and vice versa. Every column would have an AlterColumn operation (which we would ignore during processing, but they were nonetheless generated in migration files).
Also, we were using relatively simple logic to track state of our temporal tables. Some operations require temporary disabling of the versioning/period, and we need to keep track of that so that we don't try to disable period twice, or forget to enable it later.
The way we did it could lead to invalid SQL in some non-trivial scenarios (e.g. converting table to temporal and adding a new column at the same time)

The new approach is to only put temporal annotations on the table itself, and the period columns. Regular columns of the temporal table don't have any temporal annotations on them anymore and we reason about temporal information based on other table-based migration operation in the batch and, if need be, on the relational model.
We also keep track of the actual temporal information for every operation (rather than keeping global dictionaries of period/version), so that complex migrations, involving multiple operations are more robust.
To achieve that we compute the initial (temporal) state of all the tables involved in the migration. We scan all the table operations, and if some info is missing we get it from relational model. Then we do the proper processing of the migration operations - when we encounter table operation, we update the temporal information for that table (since table operations contain relevant temporal annotations). For all other operations we extract the current temporal state for the table involved, and reason based on that info.

Fixes #27459 - SQL Server Migrations: Review temporal table annotations
Fixes #29536 - EF Core IsTemporal() creates huge migration
Fixes #29799 - EF7 SqlServer Migration is trying to update columns on History table before creating the History table if any new columns are added in the same migration
  • Loading branch information
maumar authored Nov 13, 2023
1 parent a60c399 commit e3e7a5b
Show file tree
Hide file tree
Showing 6 changed files with 1,928 additions and 429 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,8 @@ public override void Generate(IColumn column, CSharpRuntimeAnnotationCodeGenerat
var annotations = parameters.Annotations;
annotations.Remove(SqlServerAnnotationNames.Identity);
annotations.Remove(SqlServerAnnotationNames.Sparse);
annotations.Remove(SqlServerAnnotationNames.IsTemporal);
annotations.Remove(SqlServerAnnotationNames.TemporalHistoryTableName);
annotations.Remove(SqlServerAnnotationNames.TemporalHistoryTableSchema);
annotations.Remove(SqlServerAnnotationNames.TemporalPeriodStartColumnName);
annotations.Remove(SqlServerAnnotationNames.TemporalPeriodEndColumnName);
annotations.Remove(SqlServerAnnotationNames.TemporalIsPeriodStartColumn);
annotations.Remove(SqlServerAnnotationNames.TemporalIsPeriodEndColumn);
}

base.Generate(column, parameters);
Expand Down
16 changes: 16 additions & 0 deletions src/EFCore.SqlServer/Metadata/Internal/SqlServerAnnotationNames.cs
Original file line number Diff line number Diff line change
Expand Up @@ -282,4 +282,20 @@ public static class SqlServerAnnotationNames
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public const string UseSqlOutputClause = Prefix + "UseSqlOutputClause";

/// <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>
public const string TemporalIsPeriodStartColumn = Prefix + "TemporalIsPeriodStartColumn";

/// <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>
public const string TemporalIsPeriodEndColumn = Prefix + "TemporalIsPeriodEndColumn";
}
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ public override IEnumerable<IAnnotation> For(ITable table, bool designTime)
yield return new Annotation(SqlServerAnnotationNames.MemoryOptimized, true);
}

if (entityType.IsTemporal() && designTime)
if (entityType.IsTemporal())
{
yield return new Annotation(SqlServerAnnotationNames.IsTemporal, true);
yield return new Annotation(SqlServerAnnotationNames.TemporalHistoryTableName, entityType.GetHistoryTableName());
Expand Down Expand Up @@ -253,7 +253,7 @@ public override IEnumerable<IAnnotation> For(IColumn column, bool designTime)
}

var entityType = (IEntityType)column.Table.EntityTypeMappings.First().TypeBase;
if (entityType.IsTemporal() && designTime)
if (entityType.IsTemporal())
{
var periodStartPropertyName = entityType.GetPeriodStartPropertyName();
var periodEndPropertyName = entityType.GetPeriodEndPropertyName();
Expand All @@ -275,12 +275,14 @@ public override IEnumerable<IAnnotation> For(IColumn column, bool designTime)
? periodEndProperty.GetColumnName(storeObjectIdentifier)
: periodEndPropertyName;

// TODO: issue #27459 - we want to avoid having those annotations on every column
yield return new Annotation(SqlServerAnnotationNames.IsTemporal, true);
yield return new Annotation(SqlServerAnnotationNames.TemporalHistoryTableName, entityType.GetHistoryTableName());
yield return new Annotation(SqlServerAnnotationNames.TemporalHistoryTableSchema, entityType.GetHistoryTableSchema());
yield return new Annotation(SqlServerAnnotationNames.TemporalPeriodStartColumnName, periodStartColumnName);
yield return new Annotation(SqlServerAnnotationNames.TemporalPeriodEndColumnName, periodEndColumnName);
if (column.Name == periodStartColumnName)
{
yield return new Annotation(SqlServerAnnotationNames.TemporalIsPeriodStartColumn, true);
}
else if (column.Name == periodEndColumnName)
{
yield return new Annotation(SqlServerAnnotationNames.TemporalIsPeriodEndColumn, true);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,58 +32,19 @@ public override IEnumerable<IAnnotation> ForRemove(IRelationalModel model)
public override IEnumerable<IAnnotation> ForRemove(ITable table)
=> table.GetAnnotations();

/// <inheritdoc />
public override IEnumerable<IAnnotation> ForRemove(IUniqueConstraint constraint)
{
if (constraint.Table[SqlServerAnnotationNames.IsTemporal] as bool? == true)
{
yield return new Annotation(SqlServerAnnotationNames.IsTemporal, true);

yield return new Annotation(
SqlServerAnnotationNames.TemporalPeriodStartColumnName,
constraint.Table[SqlServerAnnotationNames.TemporalPeriodStartColumnName]);

yield return new Annotation(
SqlServerAnnotationNames.TemporalPeriodEndColumnName,
constraint.Table[SqlServerAnnotationNames.TemporalPeriodEndColumnName]);

yield return new Annotation(
SqlServerAnnotationNames.TemporalHistoryTableName,
constraint.Table[SqlServerAnnotationNames.TemporalHistoryTableName]);

yield return new Annotation(
SqlServerAnnotationNames.TemporalHistoryTableSchema,
constraint.Table[SqlServerAnnotationNames.TemporalHistoryTableSchema]);
}
}

/// <inheritdoc />
public override IEnumerable<IAnnotation> ForRemove(IColumn column)
{
if (column.Table[SqlServerAnnotationNames.IsTemporal] as bool? == true)
{
yield return new Annotation(SqlServerAnnotationNames.IsTemporal, true);

yield return new Annotation(
SqlServerAnnotationNames.TemporalHistoryTableName,
column.Table[SqlServerAnnotationNames.TemporalHistoryTableName]);

yield return new Annotation(
SqlServerAnnotationNames.TemporalHistoryTableSchema,
column.Table[SqlServerAnnotationNames.TemporalHistoryTableSchema]);

if (column[SqlServerAnnotationNames.TemporalPeriodStartColumnName] is string periodStartColumnName)
if (column[SqlServerAnnotationNames.TemporalIsPeriodStartColumn] as bool? == true)
{
yield return new Annotation(
SqlServerAnnotationNames.TemporalPeriodStartColumnName,
periodStartColumnName);
yield return new Annotation(SqlServerAnnotationNames.TemporalIsPeriodStartColumn, true);
}

if (column[SqlServerAnnotationNames.TemporalPeriodEndColumnName] is string periodEndColumnName)
if (column[SqlServerAnnotationNames.TemporalIsPeriodEndColumn] as bool? == true)
{
yield return new Annotation(
SqlServerAnnotationNames.TemporalPeriodEndColumnName,
periodEndColumnName);
yield return new Annotation(SqlServerAnnotationNames.TemporalIsPeriodEndColumn, true);
}
}
}
Expand All @@ -102,37 +63,28 @@ public override IEnumerable<IAnnotation> ForRename(ITable table)
yield return new Annotation(
SqlServerAnnotationNames.TemporalHistoryTableSchema,
table[SqlServerAnnotationNames.TemporalHistoryTableSchema]);

yield return new Annotation(
SqlServerAnnotationNames.TemporalPeriodStartColumnName,
table[SqlServerAnnotationNames.TemporalPeriodStartColumnName]);

yield return new Annotation(
SqlServerAnnotationNames.TemporalPeriodEndColumnName,
table[SqlServerAnnotationNames.TemporalPeriodEndColumnName]);
}
}

/// <inheritdoc />
public override IEnumerable<IAnnotation> ForRename(IColumn column)
{
if (column.Table[SqlServerAnnotationNames.IsTemporal] as bool? == true)
if (column[SqlServerAnnotationNames.TemporalIsPeriodStartColumn] as bool? == true)
{
yield return new Annotation(SqlServerAnnotationNames.IsTemporal, true);

yield return new Annotation(
SqlServerAnnotationNames.TemporalHistoryTableName,
column.Table[SqlServerAnnotationNames.TemporalHistoryTableName]);

yield return new Annotation(
SqlServerAnnotationNames.TemporalHistoryTableSchema,
column.Table[SqlServerAnnotationNames.TemporalHistoryTableSchema]);

if (column[SqlServerAnnotationNames.TemporalPeriodStartColumnName] is string periodStartColumnName)
{
yield return new Annotation(
SqlServerAnnotationNames.TemporalPeriodStartColumnName,
periodStartColumnName);
}
yield return new Annotation(SqlServerAnnotationNames.TemporalIsPeriodStartColumn, true);
}

if (column[SqlServerAnnotationNames.TemporalPeriodEndColumnName] is string periodEndColumnName)
{
yield return new Annotation(
SqlServerAnnotationNames.TemporalPeriodEndColumnName,
periodEndColumnName);
}
if (column[SqlServerAnnotationNames.TemporalIsPeriodEndColumn] as bool? == true)
{
yield return new Annotation(SqlServerAnnotationNames.TemporalIsPeriodEndColumn, true);
}
}
}
Loading

0 comments on commit e3e7a5b

Please sign in to comment.