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

[5.0.1] Migrations: Skip DbFunctionAttributeConvention when creating model for history repository #23452

Merged
merged 1 commit into from
Nov 25, 2020

Conversation

smitpatel
Copy link
Member

@smitpatel smitpatel commented Nov 23, 2020

Resolves #23312

Description

When generating the script to create migration history table, we use a model builder to generate the model. This model builder is decoupled from the application DbContext so that it won't include application entity types in the model. However, this decoupling is broken in 5.0 because we still discover methods on DbContext annotated with DbFunctionAttribute. If the DbFunction is targeting table valued function (TVF), then we add that as an entity type to the model. This causes an incorrect model to be built for the history table.

Customer Impact

Customers who have TVF declared using DbFunction attribute on their DbContext will get error when applying first migration or generating migration script from first migration. We create the migration history table in first migration if it does not exist.

How found

Reported by customer on 5.0 release.

Test coverage

Updated existing tests which were verifying the scripts for migration history table to use actual DbContext which would match customer scenarios.

Regression?

No. TVF is a new feature in 5.0. Earlier even though we would have added DbFunctions to the model, it wouldn't have added additional entity types.

Risk

Low. The model is not supposed to discover any entity types. By removing convention it would not remove something which is required. Also there is a quirk mode to revert to previous behavior.

@smitpatel smitpatel requested a review from a team November 23, 2020 21:51
@AndriySvyryd
Copy link
Member

You can modify SqlServerHistoryRepositoryTest to use a custom DbContext that has [DbFunction] and DbSet

@smitpatel smitpatel changed the base branch from main to release/5.0 November 24, 2020 16:16
@smitpatel
Copy link
Member Author

Updated

@smitpatel smitpatel changed the title Migrations: Skip DbFunctionAttributeConvention when creating model fo… [5.0.1] Migrations: Skip DbFunctionAttributeConvention when creating model fo… Nov 24, 2020
@ajcvickers ajcvickers changed the title [5.0.1] Migrations: Skip DbFunctionAttributeConvention when creating model fo… [5.0.1] Migrations: Skip DbFunctionAttributeConvention when creating model for history repository Nov 24, 2020
@ajcvickers ajcvickers added this to the 5.0.x milestone Nov 24, 2020
@ajcvickers
Copy link
Member

Approved by Tactics for 5.0.1.

@ajcvickers
Copy link
Member

@smitpatel FYI, this needs to get merged by 4pm to get in 5.0.1.

@smitpatel smitpatel merged commit aaf724f into release/5.0 Nov 25, 2020
@smitpatel smitpatel deleted the smit/TheSqlThatShockedUs branch November 25, 2020 18:43
@ajcvickers ajcvickers removed this from the 5.0.1 milestone Dec 11, 2020
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.

SQL generated for migrations is incorrect when DbContext has TVF mapped using DbFunctionAttribute
4 participants