2rueSid commented on code in PR #1852:
URL: https://github.com/apache/kvrocks/pull/1852#discussion_r1381662584
##########
src/types/json.h:
##########
@@ -215,6 +218,80 @@ struct JsonValue {
return Status::OK();
}
+ StatusOr<bool> Merge(const std::string_view path, const std::string
&merge_value) {
+ bool is_updated = false;
+ const std::string json_root_path = "$";
+ try {
+ jsoncons::json patch_value = jsoncons::json::parse(merge_value);
+ bool not_exists = jsoncons::jsonpath::json_query(value, path).empty();
+
+ if (path == json_root_path || not_exists) {
+ // For non-existing keys the path must be $
+ if (not_exists) {
+ // Create an actual object from non-existing path
+ jsoncons::json created_object = GenerateJsonFromPath(path.data(),
patch_value);
+
+ // Merge it with patch value
+ auto patch = jsoncons::mergepatch::from_diff(patch_value,
created_object);
+ jsoncons::mergepatch::apply_merge_patch(patch_value, patch);
+ }
+
+ // Merge with the root. Patch function complies with RFC7396 Json
Merge Patch
+ jsoncons::mergepatch::apply_merge_patch(value, patch_value);
+ is_updated = true;
+ } else if (!patch_value.is_null()) {
+ // Replace value by path
+ jsoncons::jsonpath::json_replace(
+ value, path, [&patch_value, &is_updated](const std::string &
/*path*/, jsoncons::json &target) {
+ target = patch_value;
+ is_updated = true;
+ });
+ } else {
+ // Handle null case
+ // Unify path expression.
+ auto expr = jsoncons::jsonpath::make_expression<jsoncons::json>(path);
+ std::string converted_path;
+ expr.evaluate(
+ value, [&](const jsoncons::string_view &p, const jsoncons::json
&val) { converted_path = p; },
+ jsoncons::jsonpath::result_options::path);
+ // Unify object state
+ jsoncons::json flattened = jsoncons::jsonpath::flatten(value);
+ if (flattened.contains(converted_path)) {
+ flattened.erase(converted_path);
+ value = jsoncons::jsonpath::unflatten(flattened);
+ is_updated = true;
+ }
+ }
+ } catch (const jsoncons::jsonpath::jsonpath_error &e) {
+ return {Status::NotOK, e.what()};
+ } catch (const jsoncons::ser_error &e) {
+ return {Status::NotOK, e.what()};
+ }
+
+ return is_updated;
+ }
+
+ static jsoncons::json GenerateJsonFromPath(const std::string &path, const
jsoncons::json &value) {
+ std::vector<std::string> parts;
+ std::stringstream ss(path);
+ std::string part;
+
+ while (std::getline(ss, part, '.')) {
+ if (part != "$") {
+ parts.push_back(part);
+ }
+ }
+
+ jsoncons::json result = value;
+ for (auto it = parts.rbegin(); it != parts.rend(); ++it) {
+ jsoncons::json obj;
+ obj[*it] = result;
+ result = obj;
+ }
+
+ return result;
Review Comment:
A brief update: I have researched a bit, and it appears that we might
require the path parser.
The function `jsonpath::json_query` can return a normalized path instead of
a value, but only if a value exists at that path. The same applies to
`jsonpath::json_replace`—it operates only when a value exists.
Moreover, the parser could be beneficial for the Set function, as it
currently cannot process the following expression:
```bash
127.0.0.1:6666> JSON.SET doc $ '{"a":2}'
OK
127.0.0.1:6666> JSON.SET doc $.b '8'
OK
127.0.0.1:6666> JSON.GET doc $
"[{\"a\":2}]"
```
Ideally, in the `Merge()` function, we should invoke the `Set()` function
for this case, I presume.
I will also reach out to the maintainer of `jsoncons` to check whether there
is a more elegant implementation of this.
##########
src/types/json.h:
##########
@@ -215,6 +218,80 @@ struct JsonValue {
return Status::OK();
}
+ StatusOr<bool> Merge(const std::string_view path, const std::string
&merge_value) {
+ bool is_updated = false;
+ const std::string json_root_path = "$";
+ try {
+ jsoncons::json patch_value = jsoncons::json::parse(merge_value);
+ bool not_exists = jsoncons::jsonpath::json_query(value, path).empty();
+
+ if (path == json_root_path || not_exists) {
+ // For non-existing keys the path must be $
+ if (not_exists) {
+ // Create an actual object from non-existing path
+ jsoncons::json created_object = GenerateJsonFromPath(path.data(),
patch_value);
+
+ // Merge it with patch value
+ auto patch = jsoncons::mergepatch::from_diff(patch_value,
created_object);
+ jsoncons::mergepatch::apply_merge_patch(patch_value, patch);
+ }
+
+ // Merge with the root. Patch function complies with RFC7396 Json
Merge Patch
+ jsoncons::mergepatch::apply_merge_patch(value, patch_value);
+ is_updated = true;
+ } else if (!patch_value.is_null()) {
+ // Replace value by path
+ jsoncons::jsonpath::json_replace(
+ value, path, [&patch_value, &is_updated](const std::string &
/*path*/, jsoncons::json &target) {
+ target = patch_value;
+ is_updated = true;
+ });
+ } else {
+ // Handle null case
+ // Unify path expression.
+ auto expr = jsoncons::jsonpath::make_expression<jsoncons::json>(path);
+ std::string converted_path;
+ expr.evaluate(
+ value, [&](const jsoncons::string_view &p, const jsoncons::json
&val) { converted_path = p; },
+ jsoncons::jsonpath::result_options::path);
+ // Unify object state
+ jsoncons::json flattened = jsoncons::jsonpath::flatten(value);
+ if (flattened.contains(converted_path)) {
+ flattened.erase(converted_path);
+ value = jsoncons::jsonpath::unflatten(flattened);
+ is_updated = true;
+ }
+ }
+ } catch (const jsoncons::jsonpath::jsonpath_error &e) {
+ return {Status::NotOK, e.what()};
+ } catch (const jsoncons::ser_error &e) {
+ return {Status::NotOK, e.what()};
+ }
+
+ return is_updated;
+ }
+
+ static jsoncons::json GenerateJsonFromPath(const std::string &path, const
jsoncons::json &value) {
+ std::vector<std::string> parts;
+ std::stringstream ss(path);
+ std::string part;
+
+ while (std::getline(ss, part, '.')) {
+ if (part != "$") {
+ parts.push_back(part);
+ }
+ }
+
+ jsoncons::json result = value;
+ for (auto it = parts.rbegin(); it != parts.rend(); ++it) {
+ jsoncons::json obj;
+ obj[*it] = result;
+ result = obj;
+ }
+
+ return result;
Review Comment:
A brief update: I have researched a bit, and it appears that we might
require the path parser.
The function `jsonpath::json_query` can return a normalized path instead of
a value, but only if a value exists at that path. The same applies to
`jsonpath::json_replace`—it operates only when a value exists.
Moreover, the parser could be beneficial for the Set function, as it
currently cannot process the following expression:
```bash
127.0.0.1:6666> JSON.SET doc $ '{"a":2}'
OK
127.0.0.1:6666> JSON.SET doc $.b '8'
OK
127.0.0.1:6666> JSON.GET doc $
"[{\"a\":2}]"
```
Ideally, in the `Merge()` function, we should invoke the `Set()` function
for this case, I presume.
I will also reach out to the maintainer of `jsoncons` to check whether there
is a more elegant implementation of this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]