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.3] Use IUpdateEntry.EntityState instead of ModificationCommand.EntityState for entities using table splitting. #23692

Merged
merged 2 commits into from
Jan 13, 2021

Conversation

AndriySvyryd
Copy link
Member

Fixes #23668

Description

ModificationCommand.EntityState is used in many places throughout CommandBatchPreparer and in most cases it is the same as IUpdateEntry.EntityState for the corresponding entry, however when adding an optional dependent to an existing row ModificationCommand.EntityState is Modified while IUpdateEntry.EntityState is Added

Customer Impact

Adding an optional dependent that is referencing a newly added principal entity or deleting both of them can result in a SQL error as the operations aren't ordered correctly. A workaround would be to call SaveChanges immediately after the first operation that should be performed, but this isn't feasible in some scenarios.

How found

Customer reported on 5.0.1.

Test coverage

We have added more test coverage in this PR.

Regression?

Yes

Risk

Low. In most cases other cases ModificationCommand.EntityState and IUpdateEntry.EntityState are the same

@ajcvickers ajcvickers added this to the 5.0.x milestone Dec 16, 2020
@leecow leecow modified the milestones: 5.0.x, 5.0.3 Dec 17, 2020
@AndriySvyryd
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ajcvickers ajcvickers merged commit d382ab9 into release/5.0 Jan 13, 2021
@ajcvickers ajcvickers deleted the Issue23668 branch January 13, 2021 23:37
@ajcvickers ajcvickers removed this from the 5.0.3 milestone Jan 14, 2021
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