2rueSid commented on code in PR #1852:
URL: https://github.com/apache/kvrocks/pull/1852#discussion_r1382587401


##########
src/types/json.h:
##########
@@ -253,6 +255,54 @@ 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 (not_exists) {
+        // TODO:: Add ability to create an object from path.
+        return {Status::NotOK, "Path does not exist."};
+      }
+
+      if (path == json_root_path) {
+        // 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;
+        }

Review Comment:
   We need to delete the key if the merge value is `null`. Since `json_replace` 
does not provide access to the parent keys, we cannot use it in this case.
   
   Consider the following example:
   ```bash
   redis> JSON.SET doc $ '{"a":2}'
   OK
   redis> JSON.MERGE doc $.a 'null'
   OK
   redis> JSON.GET doc $
   ```
   In the `json_replace` function with this path, we would retrieve the value 
of `a`. However, to call the `erase()` function, we need to pass a key name, 
and the key `a` cannot point to itself. Therefore, to erase it, we need to have 
access to the parent key.
   
   We need something like this:
   
   ```cpp
   json_replace("{\"a\": 2}", path="$", pseudo_cb={
     // This callback function attempts to erase the key 'a' from the JSON 
object.
     value.erase("$.a");
   })
   ```



-- 
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]

Reply via email to