Targeted fix for #35162 - Regression from EF Core 8 to 9: MigrationBuilder.DropTable Causes Issues with Subsequent Table Recreation #35764
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
PR Overview
This PR targets regression issue #35162 by making the temporal table information map handling more robust during migration operations. Key changes include:
- Adding new composite tests for various scenarios, including dropping/creating and renaming tables.
- Introducing a new TestComposite helper method for testing multiple model transitions.
- Updating the SQL Server migrations SQL generator to improve temporal table information handling.
Reviewed Changes
| File | Description |
|---|---|
| test/EFCore.Relational.Specification.Tests/Migrations/MigrationsTestBase.cs | Added multiple composite tests and the TestComposite helper to validate complex migration scenarios. |
| test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.cs | Added corresponding functional tests with SQL assertions for the new migration operations. |
| src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs | Refactored the temporal information mapping logic to handle table renaming and creation more robustly. |
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
maumar
force-pushed
the
targetedfix35162
branch
2 times, most recently
from
425d14c to
c96f8a2
Compare
test/EFCore.Relational.Specification.Tests/Migrations/MigrationsTestBase.cs
Show resolved
Hide resolved
test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.cs
Show resolved
Hide resolved
AndriySvyryd
approved these changes
Mar 11, 2025
maumar
force-pushed
the
targetedfix35162
branch
from
c96f8a2 to
35733e2
Compare
…ilder.DropTable Causes Issues with Subsequent Table Recreation 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. Fixes #35162
maumar
force-pushed
the
targetedfix35162
branch
from
35733e2 to
d5fc85c
Compare
maumar
added a commit
that referenced
this pull request
Mar 12, 2025
…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
added a commit
that referenced
this pull request
Mar 12, 2025
…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
added a commit
that referenced
this pull request
Mar 13, 2025
…Table Causes Issues with Subsequent Table Recreation (#35776) 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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Fixes #35162