Skip to content

Commit a5e1dcd

Browse files
committed
Fix to #26676 - EF Core 6 - IsTemporal adds an unnecessary migration when DefaultSchema is configured at the DbContext Level
Problem was a discrepancy between RelationalModel build in runtime and the current model (from OnModelCreating) - relational model would set history table schema annotation using the table schema or default schema, however current model wouldn't set those annotations. Model differ would then pick up on those differences and create migration to set the schema for history table. When building actual migration sql we already compensated for the difference and not generate actual sql code, but the problem persists. Fix is to add missing default annotations in the model finalization step. This is a temporary measure until we implement default value conventions (#9329) Fixes #26676
1 parent e42407c commit a5e1dcd

File tree

7 files changed

+1197
-129
lines changed

7 files changed

+1197
-129
lines changed

src/EFCore.SqlServer/Extensions/SqlServerEntityTypeExtensions.cs

+1-2
Original file line numberDiff line numberDiff line change
@@ -252,8 +252,7 @@ public static void SetHistoryTableName(this IMutableEntityType entityType, strin
252252
public static string? GetHistoryTableSchema(this IReadOnlyEntityType entityType)
253253
=> (entityType is RuntimeEntityType)
254254
? throw new InvalidOperationException(CoreStrings.RuntimeModelMissingData)
255-
: entityType[SqlServerAnnotationNames.TemporalHistoryTableSchema] as string
256-
?? entityType[RelationalAnnotationNames.Schema] as string;
255+
: entityType[SqlServerAnnotationNames.TemporalHistoryTableSchema] as string ?? entityType.GetSchema();
257256

258257
/// <summary>
259258
/// Sets a value representing the schema of the history table associated with the entity mapped to a temporal table.

src/EFCore.SqlServer/Metadata/Conventions/SqlServerConventionSetBuilder.cs

+1
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ public override ConventionSet CreateConventionSet()
118118
conventionSet.ModelFinalizingConventions,
119119
(SharedTableConvention)new SqlServerSharedTableConvention(Dependencies, RelationalDependencies));
120120
conventionSet.ModelFinalizingConventions.Add(new SqlServerDbFunctionConvention(Dependencies, RelationalDependencies));
121+
conventionSet.ModelFinalizingConventions.Add(sqlServerTemporalConvention);
121122

122123
ReplaceConvention(
123124
conventionSet.ModelFinalizedConventions,

src/EFCore.SqlServer/Metadata/Conventions/SqlServerTemporalConvention.cs

+19-5
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,10 @@ namespace Microsoft.EntityFrameworkCore.Metadata.Conventions;
1313
/// <see href="https://aka.ms/efcore-docs-sqlserver">Accessing SQL Server and SQL Azure databases with EF Core</see>
1414
/// for more information and examples.
1515
/// </remarks>
16-
public class SqlServerTemporalConvention : IEntityTypeAnnotationChangedConvention, ISkipNavigationForeignKeyChangedConvention
16+
public class SqlServerTemporalConvention : IEntityTypeAnnotationChangedConvention, ISkipNavigationForeignKeyChangedConvention, IModelFinalizingConvention
1717
{
18-
private const string PeriodStartDefaultName = "PeriodStart";
19-
private const string PeriodEndDefaultName = "PeriodEnd";
18+
private const string DefaultPeriodStartName = "PeriodStart";
19+
private const string DefaultPeriodEndName = "PeriodEnd";
2020

2121
/// <summary>
2222
/// Creates a new instance of <see cref="SqlServerTemporalConvention" />.
@@ -56,12 +56,12 @@ public virtual void ProcessEntityTypeAnnotationChanged(
5656
{
5757
if (entityTypeBuilder.Metadata.GetPeriodStartPropertyName() == null)
5858
{
59-
entityTypeBuilder.HasPeriodStart(PeriodStartDefaultName);
59+
entityTypeBuilder.HasPeriodStart(DefaultPeriodStartName);
6060
}
6161

6262
if (entityTypeBuilder.Metadata.GetPeriodEndPropertyName() == null)
6363
{
64-
entityTypeBuilder.HasPeriodEnd(PeriodEndDefaultName);
64+
entityTypeBuilder.HasPeriodEnd(DefaultPeriodEndName);
6565
}
6666

6767
foreach (var skipLevelNavigation in entityTypeBuilder.Metadata.GetSkipNavigations())
@@ -138,4 +138,18 @@ public virtual void ProcessSkipNavigationForeignKeyChanged(
138138
joinEntityType.SetIsTemporal(true);
139139
}
140140
}
141+
142+
/// <inheritdoc />
143+
public virtual void ProcessModelFinalizing(
144+
IConventionModelBuilder modelBuilder,
145+
IConventionContext<IConventionModelBuilder> context)
146+
{
147+
foreach (var entityType in modelBuilder.Metadata.GetEntityTypes().Where(e => e.IsTemporal()))
148+
{
149+
// Needed for the annotation to show up in the model snapshot - issue #9329
150+
// history table name will always be non-null for temporal table case
151+
entityType.Builder.UseHistoryTableName(entityType.GetHistoryTableName()!);
152+
entityType.Builder.UseHistoryTableSchema(entityType.GetHistoryTableSchema());
153+
}
154+
}
141155
}

src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs

+3-2
Original file line numberDiff line numberDiff line change
@@ -534,6 +534,7 @@ protected override void Generate(
534534
{
535535
var historyTableSchema = operation[SqlServerAnnotationNames.TemporalHistoryTableSchema] as string
536536
?? model?.GetDefaultSchema();
537+
537538
var needsExec = historyTableSchema == null;
538539
var subBuilder = needsExec
539540
? new MigrationCommandListBuilder(Dependencies)
@@ -1261,7 +1262,7 @@ protected override void Generate(
12611262
{
12621263
var historyTableName = operation[SqlServerAnnotationNames.TemporalHistoryTableName] as string;
12631264
var historyTableSchema = operation[SqlServerAnnotationNames.TemporalHistoryTableSchema] as string
1264-
?? model?.GetDefaultSchema();
1265+
?? operation.Schema ?? model?.GetDefaultSchema();
12651266
var periodStartColumnName = operation[SqlServerAnnotationNames.TemporalPeriodStartColumnName] as string;
12661267
var periodEndColumnName = operation[SqlServerAnnotationNames.TemporalPeriodEndColumnName] as string;
12671268

@@ -2261,7 +2262,7 @@ private IReadOnlyList<MigrationOperation> RewriteOperations(
22612262
schema ??= model?.GetDefaultSchema();
22622263
var historyTableName = operation[SqlServerAnnotationNames.TemporalHistoryTableName] as string;
22632264
var historyTableSchema = operation[SqlServerAnnotationNames.TemporalHistoryTableSchema] as string
2264-
?? model?.GetDefaultSchema();
2265+
?? schema;
22652266
var periodStartColumnName = operation[SqlServerAnnotationNames.TemporalPeriodStartColumnName] as string;
22662267
var periodEndColumnName = operation[SqlServerAnnotationNames.TemporalPeriodEndColumnName] as string;
22672268

test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs

+2-1
Original file line numberDiff line numberDiff line change
@@ -2123,6 +2123,7 @@ public virtual void Temporal_table_information_is_stored_in_snapshot_minimal_set
21232123
21242124
b.ToTable(tb => tb.IsTemporal(ttb =>
21252125
{
2126+
ttb.UseHistoryTable(""EntityWithStringPropertyHistory"");
21262127
ttb
21272128
.HasPeriodStart(""PeriodStart"")
21282129
.HasColumnName(""PeriodStart"");
@@ -2138,7 +2139,7 @@ public virtual void Temporal_table_information_is_stored_in_snapshot_minimal_set
21382139
"Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+EntityWithStringProperty");
21392140
var annotations = temporalEntity.GetAnnotations().ToList();
21402141

2141-
Assert.Equal(5, annotations.Count);
2142+
Assert.Equal(6, annotations.Count);
21422143
Assert.Contains(annotations, a => a.Name == SqlServerAnnotationNames.IsTemporal && a.Value as bool? == true);
21432144
Assert.Contains(
21442145
annotations,

test/EFCore.Design.Tests/Scaffolding/Internal/CSharpDbContextGeneratorTest.cs

+1
Original file line numberDiff line numberDiff line change
@@ -1062,6 +1062,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
10621062
{
10631063
entity.ToTable(tb => tb.IsTemporal(ttb =>
10641064
{
1065+
ttb.UseHistoryTable(""CustomerHistory"");
10651066
ttb
10661067
.HasPeriodStart(""PeriodStart"")
10671068
.HasColumnName(""PeriodStart"");

0 commit comments

Comments
 (0)