Skip to content

[release/9.0-staging] Fix to #35162 - Regression from EF Core 8 to 9: MigrationBuilder.DropTable Causes Issues with Subsequent Table Recreation #35776

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
Mar 13, 2025

Conversation

maumar
Copy link
Contributor

@maumar maumar commented Mar 12, 2025

Port of #35764

Fixes #35162

Description
Problem was that in 9.0 we changed how we process temporal tables, tracking the temporal table information as we process migrations. However, we didn't take into account the cases where users would manually modify their migrations, or squash multiple migrations together, resulting in corrupted dictionary access for cases where a table was deleted and created in the same migration. This fix just makes the temporal information map handling more robust, so those errors don't happen anymore. There is a broader issue with temporal tables but those will be addressed in a different PR.

Customer impact
Some manually modified/squashed migrations no longer work when they are being applied, throwing an unhelpful exception (The given key '...' was not present in the dictionary.'). No good workaround apart from breaking the migrations apart.

How found
Reported by multiple customers on 9.

Regression
Yes.

Testing
Added infra to test these types of scenarios (containing intermediate migration state), added multiple tests both for regular and temporal tables.

Risk
Low - Very targeted fix, accessing the temporal information dictionary in a more robust way. No quirk, since migrations are design time, so hard to set switches for them.

@maumar maumar requested review from Copilot and a team March 12, 2025 06:10
@maumar maumar added this to the 9.0.x milestone Mar 12, 2025
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.

Pull Request Overview

This PR fixes a regression in temporal table handling during migrations by making the temporal information map more robust and adding tests to cover composite migration scenarios.

  • Added composite migration tests to verify drop, rename, and create operations on temporal tables.
  • Introduced a new helper method, TestComposite, to generate and validate intermediate model states.
  • Updated SQL Server migrations SQL generator to handle temporal table information mapping for renamed and re-created tables.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
test/EFCore.Relational.Specification.Tests/Migrations/MigrationsTestBase.cs Added new composite migration tests and supporting helper method to improve test coverage for temporal table changes.
src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs Modified temporal table information mapping logic for rename and create operations to prevent dictionary errors.
Comments suppressed due to low confidence (2)

test/EFCore.Relational.Specification.Tests/Migrations/MigrationsTestBase.cs:3384

  • [nitpick] Consider adding more contextual information to this error message to help developers understand why at least 3 build actions are required in composite migration tests.
throw new InvalidOperationException("You need at least 3 build actions for the composite case.");

src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs:2560

  • Review the logic for initializing temporal table information for the new table name; ensure that this conditional mapping preserves the original temporal data consistently to avoid potential regression issues in table recreation scenarios.
if (!temporalTableInformationMap.ContainsKey((newTableName, newRawSchema)))

…Table Causes Issues with Subsequent Table Recreation

Port of #35764

Fixes #35162

Description
Problem was that in 9.0 we changed how we process temporal tables, tracking the temporal table information as we process migrations. However, we didn't take into account the cases where users would manually modify their migrations, or squash multiple migrations together, resulting in corrupted dictionary access for cases where a table was deleted and created in the same migration. This fix just makes the temporal information map handling more robust, so those errors don't happen anymore. There is a broader issue with temporal tables but those will be addressed in a different PR.

Customer impact
Some manually modified/squashed migrations no longer work when they are being applied, throwing an unhelpful exception (The given key '...' was not present in the dictionary.'). No good workaround apart from breaking the migrations apart.

How found
Reported by multiple customers on 9.

Regression
Yes.

Testing
Added infra to test these types of scenarios (containing intermediate migration state), added multiple tests both for regular and temporal tables.

Risk
Low - Very targeted fix, accessing the temporal information dictionary in a more robust way. No quirk, since migrations are design time, so hard to set switches for them.
@maumar maumar merged commit 85957ac into release/9.0-staging Mar 13, 2025
7 checks passed
@maumar maumar deleted the fix35162_90 branch March 13, 2025 06:38
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.

2 participants