-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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 for 21041. Make all concurrency properties capable of storing "store-generated values" #21128
Conversation
…alues. Update TableSharingConcurrencyTokenConvention to create shadow concurrency property for all concurrency properties in the hierarchy, even non-store-generated ones.
@@ -141,8 +142,7 @@ out Dictionary<(string Table, string Schema), Dictionary<string, IList<IConventi | |||
|
|||
foreach (var property in entityType.GetDeclaredProperties()) | |||
{ | |||
if (property.IsConcurrencyToken | |||
&& (property.ValueGenerated & ValueGenerated.OnUpdate) != 0) | |||
if (property.IsConcurrencyToken) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RelationalModelValidator
needs a corresponding change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See PR #21136.
@@ -112,7 +112,8 @@ public static bool RequiresValueGenerator([NotNull] this IProperty property) | |||
/// </summary> | |||
public static bool MayBeStoreGenerated([NotNull] this IProperty property) | |||
{ | |||
if (property.ValueGenerated != ValueGenerated.Never) | |||
if (property.ValueGenerated != ValueGenerated.Never | |||
|| property.IsConcurrencyToken) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not enough. Any column mapped to two or more writable properties in table sharing could also be "store-generated"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you raise another issue please, with a test case that would fail?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixes #21041.
All concurrency properties should be able to store "store-generated values". Note: in this context "store-generated values" just means "can be injected from the update pipeline".
Also updated
TableSharingConcurrencyTokenConvention
to create shadow concurrency property for all concurrency properties in the hierarchy, even non-store-generated ones.