Addressing "Temporal Table" feature feedback

[maumar]

Placeholder for Temporal Tables feedback from customers as well as EF Team addressing the feedback / improvements.

[jzabroski]

@smitpatel So as to not derail dotnet/efcore#25365 , here is my reply to your comment:

LINQ is infinite problem space, we have laid out what are current restrictions with the system, so anything outside of it should be considered valid and should work so best thing to do is write different query and see what SQL/result they generate and it is expected.

Got it. I think the hard part for some people, like me, is there is somewhat opaque terms like "set operation", "navigation expansion", "manual joins", "consistency of the result graph", etc. that the audience is presumed to be familiar with. I don't know if there is a glossary of terms, but I wasn't trying to derail this question on purpose. I just didn't know what this issue was fully about. In particular, I didn't grok what "consistency of the result graph" meant until you brought up referential integrity and it clicked for me that you need to be able to round trip these objects with change tracking. (I'm actually not 100% sure I fully grok it, but I do understand that a temporal dimension makes typical third normal form expressed via navigation properties insufficient.) Related, I don't know things like "Is Include operator part of 'navigation expansion'?"

Separately, I personally don't run preview builds of EFCore - I find that installing the latest preview SDK causes unpredictable local environment dependencies that might hamper my abilities to do my day job on stable environments.

Here is my rough shot at a small slang glossary:

Set Operation: Also known as DbSet Operation. Operations that are only supported on a DbSet rather than any IQueryable type. These Operations are: Union, Intersection, Except, Concat.

Navigation Expansion: Also known as query expression syntax for navigating relationships. Unclear whether this also refers to eager loading via Include(). Navigation expansion allows the end user to avoid needing to explicitly type the join conditions, because the join conditions are looked up in the entity model metadata.

Navigation Propagation: ? - I am not not sure how to articulate this, even though I think I understand what is meant by it.

Temporal Operation: Operations that are only supported on a DbSet that has entity model metadata for temporal table support. These Operations are: TemporalAsOf, TemporalAll, TemporalBetween, TemporalFromTo, TemporalContainedIn. When a query contains a temporal operation, EF will automatically propagate all temporal operations to navigation expansions as well.

Manual Joins: Also known as query expression syntax for join operators. Manual joins are explicit instructions for how to construct a join, and do not look at entity model metadata to determine things like key relationships. Put another way, manual joins are not computed via an algorithm, they are simply de facto join operations with direct translations to SQL.

Consistency of the Result Graph: Also known as referential integrity rules. Consistency refers to the object graph not violating the object dependency rules specified in the entity model metadata, e.g. required dependent, optional dependent, etc.

I don't know if the above gives you angina or is a relief and improvement - but my intentions are for the best. As a brief example, I used Google Books search and searched Entity Framework Core In Action for the phrase "navigation expansion" and it appears 0 times. So, I don't think I am just gesticulating when I say it's an esoteric term that could use a clear meaning. I also think clearer meanings produce better test cases because it will be clearer what the SUT is. In other words, even though the LINQ problem space is infinite, the meaning of these words cannot be.

[smitpatel]

It is easy to blur common terms in particular context if it is looked outside of context or applying very specific constraint set over it. While my years of working for EF Core, none of those terms have caused confusion for users hence we use those terms frequently. But alas, there is always first time. So I will describe them here.

Set operation - a mathematical set operation which refers to Union/Intersection/Except (Concat which is also Union but without removing duplicates). I have mentioned this in my earlier response too. We never called any DbSet operation set operation, we always refer to DbSet as DbSet even though API is called Set. Also from LINQ perspective there is no class operations which are only on DbSet because LINQ only understands a query root.

Navigation expansion - Navigations don't exist in database. Any navigation property configured in EF Core model needs to be transformed into an equivalent join in database. EF Core does this particular task for users. The big reason for using ORM is having an easy way to write queries using navigations rather than writing multiple joins as a user. While the part expansion refers to expanding the navigation into a join, anything relating to navigation expansion refers to using any kind of navigation which is in EF Core model in LINQ query (which starts with DbSet query root). Include is just an operator and the argument inside it refers to navigation. EF Core treats it as any navigation written by user and expand it into join accordingly.

There is no navigation propagation. There is propagation of facets from one entity to another entity which is traversed through a navigation property. So if you do AsOf on Customers and use Orders navigation which is also temporal, we apply same AsOf point in time on orders too.

Manual joins - Since EF Core expands navigations into join, when there are no navigation properties or they cannot be used, user is required to what EF Core does internally for navigation, write a join between tables themselves. Since user is initiating the join we call it manual join (rather than implicit join through navigation).

Consistency of result graph - When multiple entities are connected by referential integrity constraint then they contains foreign keys. The foreign key dictate the association. But when you pick a record from previous time in temporal table the associated record may not be present in current time. Though at that point in time the record existed without violating constraint, we can find associated entity in same point in time. This is what we do wrt navigation expansion and AsOf operation. Going beyond that which is over a duration, requires that we understand more than just FK configured as a single record with given primary key could have multiple records for different dates. So in order to connect we also need to look at the dates. Which becomes even harder since SqlServer stores start and end date for each record so we are not matching exact value but time periods. This is just creating results part, consuming results would involve similar complexity in terms of understanding it. So this become pit of failure especially if assumptions and computations made along the way are not what user expected. So we resolved it by blocking the scenario and letting user write joins the way they want and generate results however they want. The full user control mode.

I understand the difficulty in using preview build and it is not cup of tea for everyone. We would like feedback from customers about feature and hence we release preview. I believe that this feedback in not being useful as it doesn't incorporate the usage of feature in direct way, it doesn't point out any obvious gaps in the feature. Everything you asked query wise in other thread is possible to do with the feature we have shipped. If you cannot use preview builds then I suggest you wait till final release and once you have used feature, provide us feedback. Currently it is just spending time for both of us for this which works and very easy to see when you have running code.

[benjaminsaljooghi]

Thanks for this work. It is very useful.

Certain migrations seem to cause the exception:

The given key 'SqlServer:TemporalHistoryTableName' was not present in the dictionary.

Reproduce:

1. Create an anonymous temporal table:

modelBuilder.Entity<Student>().ToTable(tb => tb.IsTemporal());

1. Add a migration

dotnet ef migrations add TemporalTables

Output is the aforementioned exception.

A temporal table with a named history table does not cause the exception, and these steps do not cause the exception in preview 6 but do in preview 7.

I am also able to cause the exception under other circumstances (in both preview 6 and 7), but it's not yet clear to me what the exact conditions need to be to trigger it.

Any advice? Is this a known bug?

[maumar]

@benjaminsaljooghi Thanks, I will investigate. We were not aware of this bug.

[maumar]

@benjaminsaljooghi filed an issue: dotnet/efcore#25511, fix is on the way. Workaround is what you have already figured out - explicitly provide history table name. Thanks for the early report!

[benjaminsaljooghi]

Thanks @maumar that fixed the anonymous history tables and also fixed other issues I was getting pertaining to the same exception.

I am now able to generate migrations and it works perfectly, but I am getting build warnings caused by my migrations here:

.OldAnnotation("SqlServer:TemporalHistoryTableSchema", null)

where OldAnnotation is declared as:

public virtual AlterOperationBuilder<TOperation> OldAnnotation(string name, object value)

However, because the schema argument is null I get:

warning CS8625: Cannot convert null literal to non-nullable reference type.

Therefore, should the value parameter be object? or?

[maumar]

@benjaminsaljooghi apologies for a late reply. This issue is tracked by dotnet/efcore#18950. As a workaround we will temporarily opt out of NRT (tracked by dotnet/efcore#25624)

[jacobjmarks]

@maumar I am facing a similar issue to @benjaminsaljooghi above however it is not a warning but a runtime exception.

Utilising Microsoft.EntityFrameworkCore[.Design] version 6.0.0-rc.2.21461.6 the following Up and Down migrations are generated for the given code-first temporal table.

// OnModelCreating
modelBuilder.Entity<Post>().ToTable(tb => tb.IsTemporal(ttb =>
{
    ttb.UseHistoryTable("Posts_History");
    ttb.HasPeriodStart(temporalPeriodStartName); // "_PeriodStart"
    ttb.HasPeriodEnd(temporalPeriodEndName); // "_PeriodEnd"
}));

// Migration: Up
migrationBuilder.AlterTable(
    name: "Posts")
    .Annotation("SqlServer:IsTemporal", true)
    .Annotation("SqlServer:TemporalHistoryTableName", "Posts_History")
    .Annotation("SqlServer:TemporalPeriodEndColumnName", "_PeriodEnd")
    .Annotation("SqlServer:TemporalPeriodStartColumnName", "_PeriodStart");

// Migration: Down
migrationBuilder.AlterTable(
    name: "Posts")
    .OldAnnotation("SqlServer:IsTemporal", true)
    .OldAnnotation("SqlServer:TemporalHistoryTableName", "Posts_History")
    .OldAnnotation("SqlServer:TemporalHistoryTableSchema", null) // <--- Causing issues
    .OldAnnotation("SqlServer:TemporalPeriodEndColumnName", "_PeriodEnd")
    .OldAnnotation("SqlServer:TemporalPeriodStartColumnName", "_PeriodStart");

Issues are faced only when attempting to revert the migration whereby the line

.OldAnnotation("SqlServer:TemporalHistoryTableSchema", null)

causes the following exception

System.ArgumentNullException: Value cannot be null. (Parameter 'value')
   at Microsoft.EntityFrameworkCore.Utilities.Check.NotNull[T](T value, String parameterName) in Microsoft.EntityFrameworkCore.Relational.dll:token 0x6000291+0x1a
   at Microsoft.EntityFrameworkCore.Migrations.Operations.Builders.AlterOperationBuilder`1.OldAnnotation(String name, Object value) in Microsoft.EntityFrameworkCore.Relational.dll:token 0x6000f87+0xc
   at MediaLog.Infrastructure.Data.Migrations.TemporalTables.Down(MigrationBuilder migrationBuilder) in C:\Users\JacobMarks\source\repos\media-log\application-server\src\Infrastructure.Data\Migrations\20210913045430_TemporalTables.cs:line 530
   at Microsoft.EntityFrameworkCore.Migrations.Migration.BuildOperations(Action`1 buildAction) in Microsoft.EntityFrameworkCore.Relational.dll:token 0x6000d6d+0xc
   at Microsoft.EntityFrameworkCore.Migrations.Migration.get_DownOperations() in Microsoft.EntityFrameworkCore.Relational.dll:token 0x6000d67+0x19
   at Microsoft.EntityFrameworkCore.Migrations.Internal.Migrator.GenerateDownSql(Migration migration, Migration previousMigration, MigrationsSqlGenerationOptions options) in Microsoft.EntityFrameworkCore.Relational.dll:token 0x6001000+0x29
   at Microsoft.EntityFrameworkCore.Migrations.Internal.Migrator.<>c__DisplayClass16_1.<GetMigrationCommandLists>b__1() in Microsoft.EntityFrameworkCore.Relational.dll:token 0x6001dc9+0x26
   at Microsoft.EntityFrameworkCore.Migrations.Internal.Migrator.Migrate(String targetMigration) in Microsoft.EntityFrameworkCore.Relational.dll:token 0x6000ffa+0x93
   at Microsoft.EntityFrameworkCore.Design.Internal.MigrationsOperations.UpdateDatabase(String targetMigration, String connectionString, String contextType) in Microsoft.EntityFrameworkCore.Design.dll:token 0x6000487+0x2e
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.UpdateDatabaseImpl(String targetMigration, String connectionString, String contextType) in Microsoft.EntityFrameworkCore.Design.dll:token 0x60003f3+0x0
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.UpdateDatabase.<>c__DisplayClass0_0.<.ctor>b__0() in Microsoft.EntityFrameworkCore.Design.dll:token 0x60006ff+0x0
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.OperationBase.Execute(Action action) in Microsoft.EntityFrameworkCore.Design.dll:token 0x6000686+0xc
Value cannot be null. (Parameter 'value')

Replacing

.OldAnnotation("SqlServer:TemporalHistoryTableSchema", null)

with

.OldAnnotation("SqlServer:TemporalHistoryTableSchema", string.Empty)

resolves the issue and allows the revert to complete successfully however I'm not sure as to the validity of the statement and am pondering whether the line should even exist at all given the annotation is not assigned in the Up migration.

[ajcvickers]

Closing. Please file any new issues on https://github.com/dotnet/efcore

[MikeOtown]

I have

modelBuilder.Entity<StatisticalData>().ToTable(tb => tb.IsTemporal(ttb =>
            {
                ttb.HasPeriodStart("SysStartTime");
                ttb.HasPeriodEnd("SysEndTime");                
            }));

in my OnModelCreating() and SysStartTime and SysEndTime columns already exist in the table, but when I execute add-migration, migrations to add these columns are generated.