-
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 to #28845 - JSON: add support for partial update to update just a single scalar property, rather than entire entities #28862
Conversation
@@ -251,6 +252,13 @@ public virtual IColumnModification AddColumnModification(in ColumnModificationPa | |||
protected virtual IColumnModification CreateColumnModification(in ColumnModificationParameters columnModificationParameters) | |||
=> new ColumnModification(columnModificationParameters); | |||
|
|||
private sealed class JsonPartialUpdateInfo |
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.
private sealed class JsonPartialUpdateInfo | |
private struct JsonPartialUpdateInfo |
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.
I'm modifying Property and PropertyValue later and also can return null JsonPartialUpdateInfo from FindJsonPartialUpdateInfo, so I find it more convenient to keep as class. Will change to struct if you feel strongly about it tho.
|
||
stringBuilder.Append("JSON_VALUE("); | ||
base.AppendUpdateColumnValue(updateSqlGeneratorHelper, columnModification, stringBuilder, name, schema); | ||
stringBuilder.Append(", '$.v')"); |
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 ideal, have you tried using an array instead, so we don't need to have magic strings?
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.
I went for a smallest structure in terms of characters that we need to send over the wire. I can switch to array and access the 1st element, should work just the same
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.
array actually saves a few characters (1 char longer on the access but 4 chars shorter on the json string itself
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.
Ok. And have you tried sending the naked value and using $
?
… single scalar property, rather than entire entities Optimization for partial updates - when only one property is modified on the entity we can change it directly, rather than rewriting the JSON element for the entire entity. To avoid string formatting issues we construct faux JSON array and insert the value that is to be the replacement, and then in the update statement we extract the value using JSON_VALUE. In case of numeric or bool we need an additional convert statement around the value. Fixes #28845
Optimization for partial updates - when only one property is modified on the entity we can change it directly, rather than rewriting the JSON element for the entire entity.
To avoid string formatting issues we construct faux JSON element and insert the value that is to be the replacement, and then in the update statement we extract the value using JSON_VALUE.
In case of numeric or bool we need an additional convert statement around the value.
Fixes #28845