Skip to content
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

Fix to #26676 - EF Core 6 - IsTemporal adds an unnecessary migration when DefaultSchema is configured at the DbContext Level #27198

Merged
merged 1 commit into from
Jan 21, 2022

Conversation

maumar
Copy link
Contributor

@maumar maumar commented Jan 17, 2022

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 improve the temporal convention so that history table schema in current model (more) closely resembles the temporal table schema and therefore is similar to RelationalModel generated in runtime.

Fixes #26676

@maumar maumar requested a review from AndriySvyryd January 17, 2022 07:32
@maumar maumar force-pushed the fix26676_convention branch 4 times, most recently from 00c29ef to 66e9565 Compare January 18, 2022 08:47
@AndriySvyryd
Copy link
Member

I assume that the issue is that migration snapshot doesn't contain the history schema annotation because it's the default. In cases like these we keep the default logic in the extensions, but add a redundant annotation using a convention, see SqlServerValueGenerationStrategyConvention

This allows the model to be more functional if the convention is removed and potentially can be improved upon when #9329 is implemented.

@maumar maumar force-pushed the fix26676_convention branch from 66e9565 to 669fef0 Compare January 20, 2022 02:50
@maumar maumar force-pushed the fix26676_convention branch from 669fef0 to a1ef33d Compare January 20, 2022 21:50
@maumar maumar force-pushed the fix26676_convention branch 2 times, most recently from a5e1dcd to a4349c9 Compare January 20, 2022 23:25
…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
@maumar maumar force-pushed the fix26676_convention branch from a4349c9 to cf46ae0 Compare January 21, 2022 08:30
@maumar maumar merged commit cf46ae0 into main Jan 21, 2022
@maumar maumar deleted the fix26676_convention branch January 21, 2022 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

EF Core 6 - IsTemporal adds an unnecessary migration when DefaultSchema is configured at the DbContext Level
2 participants