Skip to content

[release/9.0-staging] Fix to #35108 - Temporal table migration regression from EF Core 8 to 9 #35289

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.

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 10, 2024

Conversation

maumar
Copy link
Contributor

@maumar maumar commented Dec 7, 2024

Port of #35283
Fixes #35108

Description

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.

Customer impact

Customers using temporal tables, who created migrations with EF Core 8 will see exception when trying to execute those migrations with EF Core 9 tooling. Workaround is to manually modify the migration code, but the solution is not obvious.

How found

Multiple customer reports on 9.0 bits

Regression

Yes, these migrations worked fine in EF 8.0

Testing

Extensive tests added for reported scenarios as well as other common temporal operations.

Risk

Low. Only the legacy scenarios are affected, so we should not regress 9.0 scenarios any further. We essentially normalize the old annotations so that old migration operations resemble the new ones and work well with the current logic. Also brought back some code from EF8 to filter redundant migration operations (so that code as been tested in EF8 already).

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.
@maumar maumar requested a review from a team as a code owner December 7, 2024 12:18
@maumar maumar requested review from Copilot and a team and removed request for a team December 7, 2024 12:18
@maumar maumar added this to the 9.0.x milestone Dec 7, 2024
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 2 out of 2 changed files in this pull request and generated no suggestions.

@maumar maumar changed the base branch from main to release/9.0-staging December 7, 2024 13:14
@maumar maumar merged commit 4b3e12f into release/9.0-staging Dec 10, 2024
7 checks passed
@maumar maumar deleted the fix35108_90 branch December 10, 2024 03:44
@maumar maumar modified the milestones: 9.0.x, 9.0.1, 9.0.2 Dec 10, 2024
@maumar
Copy link
Contributor Author

maumar commented Dec 10, 2024

approved by email

This was referenced Aug 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Temporal table migration regression from EF Core 8 to 9
2 participants