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

Don't consider a generated value stable just because it was explicitly set #32497

Merged
merged 2 commits into from
Dec 10, 2023

Conversation

ajcvickers
Copy link
Member

Fixes #32084

@ajcvickers ajcvickers requested a review from a team December 2, 2023 23:04
Copy link
Member

@AndriySvyryd AndriySvyryd left a comment

Choose a reason for hiding this comment

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

This is a breaking change for the cases where the PK is not an FK, has a non-stable value generator and is assigned an explicit value

@ajcvickers
Copy link
Member Author

This is a breaking change for the cases where the PK is not an FK, has a non-stable value generator and is assigned an explicit value

Are you sure? We always start with

        var hasStableValues = false;
        var hasNonStableValues = false;

In the new code we see the property has a non-stable generator, so in one case hasNonStableValues becomes true.

We then:

return hasStableValues && !hasNonStableValues;

Which is false in both cases.

Or does there also have to be a stable generator in there for the break you are thinking of?

@AndriySvyryd
Copy link
Member

Or does there also have to be a stable generator in there for the break you are thinking of?

Sorry, it just needs to be one stable generator, like the one we use in Cosmos.

@ajcvickers
Copy link
Member Author

Or does there also have to be a stable generator in there for the break you are thinking of?

Sorry, it just needs to be one stable generator, like the one we use in Cosmos.

So you mean PK: yes; FK: no; Explicit value: no; stable generator: yes?

@ajcvickers
Copy link
Member Author

@AndriySvyryd Wrote taste cases that execute this path. The behavior has changed when detecting changes and a new entity is found that has an explicitly set stable value for its PK. We previous detected this as Modified, as we would if the value was non-stable and explicitly set, but the presence of a stable value does not indicate that the entity must already exist, so the correct thing to do is fall back to Added.

@ajcvickers ajcvickers merged commit 9af27b0 into main Dec 10, 2023
7 checks passed
@ajcvickers ajcvickers deleted the 231202_BlueSunday branch December 10, 2023 15:00
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.

Differnce in behavior between TPH and TPT/TPC with new entity added through navigation property
2 participants