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-rc2] Also clear the store generated values when setting a normal value #22607

Merged
merged 2 commits into from
Sep 19, 2020

Conversation

AndriySvyryd
Copy link
Member

Fixes #22577

Description

Usually a value is only propagated to a foreign key and the foreign key value is not store-generated, however in TPT the property might get a store-generated value before the real value is propagated to it.

Customer Impact

The issue mostly affects a new feature - TPT when the principal key is store generated. There is a workaround where the entities to be saved are split and the dependent values cleared manually, but this isn't practical in most applications.

How found

Customer reported on RC1.

Test coverage

We were lacking test coverage for complex TPT models. This PR adds a test for the affected scenario.

Regression?

No, new feature in 5.0.

Risk

Low. The fix mostly affects a new feature - TPT and the values that are now discarded were already being discarded before, but on completion of SaveChanges instead of during execution

…are considered store-generated and ChangeTracker doesn't return store-generated values that are the default value.
Copy link
Member

@ajcvickers ajcvickers left a comment

Choose a reason for hiding this comment

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

Please have @smitpatel approve as well.

@ajcvickers
Copy link
Member

@AndriySvyryd Approved by Tactics, but have @smitpatel review too.

@AndriySvyryd AndriySvyryd merged commit 8df41e4 into release/5.0-rc2 Sep 19, 2020
@AndriySvyryd AndriySvyryd deleted the Issue22577 branch September 19, 2020 22:23
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.

3 participants