You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@kvrocks.apache.org by "Qiaolin-Yu (via GitHub)" <gi...@apache.org> on 2023/10/29 04:50:13 UTC
[PR] Support for the JSON.DEL command [kvrocks]
Qiaolin-Yu opened a new pull request, #1859:
URL: https://github.com/apache/kvrocks/pull/1859
Support JSON.DEL. This PR aims to close https://github.com/apache/kvrocks/issues/1813.
[JSON.DEL command](https://redis.io/commands/json.del/)
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support for the JSON.DEL command [kvrocks]
Posted by "Qiaolin-Yu (via GitHub)" <gi...@apache.org>.
Qiaolin-Yu commented on PR #1859:
URL: https://github.com/apache/kvrocks/pull/1859#issuecomment-1783998759
Can you help me review this? @PragmaTwice
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support for the JSON.DEL command [kvrocks]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #1859:
URL: https://github.com/apache/kvrocks/pull/1859#issuecomment-1789537864
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_kvrocks&pullRequest=1859)
[](https://sonarcloud.io/project/issues?id=apache_kvrocks&pullRequest=1859&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_kvrocks&pullRequest=1859&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_kvrocks&pullRequest=1859&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_kvrocks&pullRequest=1859&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_kvrocks&pullRequest=1859&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_kvrocks&pullRequest=1859&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_kvrocks&pullRequest=1859&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_kvrocks&pullRequest=1859&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_kvrocks&pullRequest=1859&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_kvrocks&pullRequest=1859&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_kvrocks&pullRequest=1859&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_kvrocks&pullRequest=1859&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_kvrocks&pullRequest=1859) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_kvrocks&pullRequest=1859&metric=duplicated_lines_density&view=list) No Duplication information
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support for the JSON.DEL command [kvrocks]
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #1859:
URL: https://github.com/apache/kvrocks/pull/1859#discussion_r1375370893
##########
src/types/json.h:
##########
@@ -215,6 +215,30 @@ struct JsonValue {
return Status::OK();
}
+ StatusOr<size_t> Del(const std::string &path) {
+ const std::string parent_path = path + "^";
+ const std::string child_path = path.substr(path.find_last_of('.') + 1);
+ size_t count = 0;
+ try {
+ jsoncons::jsonpath::json_replace(value, parent_path,
+ [&count, &child_path](const std::string &path, jsoncons::json &value) {
+ if (value.is_object()) {
Review Comment:
It can also be an array rather than object.
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support for the JSON.DEL command [kvrocks]
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #1859:
URL: https://github.com/apache/kvrocks/pull/1859#discussion_r1375370486
##########
src/types/json.h:
##########
@@ -215,6 +215,30 @@ struct JsonValue {
return Status::OK();
}
+ StatusOr<size_t> Del(const std::string &path) {
+ const std::string parent_path = path + "^";
+ const std::string child_path = path.substr(path.find_last_of('.') + 1);
Review Comment:
It is not a general way to retrieve the key, e.g. it can be wrong while processing `$[1]`, `$["a"]`, `$.a[*]`.
TBH it is hard to retrieve the key here, but you can try to cover more cases.
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support for the JSON.DEL command [kvrocks]
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #1859:
URL: https://github.com/apache/kvrocks/pull/1859#discussion_r1375370875
##########
src/types/json.h:
##########
@@ -215,6 +215,30 @@ struct JsonValue {
return Status::OK();
}
+ StatusOr<size_t> Del(const std::string &path) {
+ const std::string parent_path = path + "^";
+ const std::string child_path = path.substr(path.find_last_of('.') + 1);
Review Comment:
Refer to https://github.com/danielaparker/jsoncons/issues/467#issuecomment-1784002194.
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support for the JSON.DEL command [kvrocks]
Posted by "Qiaolin-Yu (via GitHub)" <gi...@apache.org>.
Qiaolin-Yu commented on code in PR #1859:
URL: https://github.com/apache/kvrocks/pull/1859#discussion_r1375503356
##########
src/types/json.h:
##########
@@ -215,6 +215,30 @@ struct JsonValue {
return Status::OK();
}
+ StatusOr<size_t> Del(const std::string &path) {
+ const std::string parent_path = path + "^";
+ const std::string child_path = path.substr(path.find_last_of('.') + 1);
Review Comment:
According to https://github.com/danielaparker/jsoncons/issues/467#issuecomment-1784123066, it seems that `jsoncons` has not such feature. 😠@PragmaTwice
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support for the JSON.DEL command [kvrocks]
Posted by "git-hulk (via GitHub)" <gi...@apache.org>.
git-hulk merged PR #1859:
URL: https://github.com/apache/kvrocks/pull/1859
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support for the JSON.DEL command [kvrocks]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #1859:
URL: https://github.com/apache/kvrocks/pull/1859#issuecomment-1828251947
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_kvrocks&pullRequest=1859)
[](https://sonarcloud.io/project/issues?id=apache_kvrocks&pullRequest=1859&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_kvrocks&pullRequest=1859&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_kvrocks&pullRequest=1859&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_kvrocks&pullRequest=1859&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_kvrocks&pullRequest=1859&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_kvrocks&pullRequest=1859&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_kvrocks&pullRequest=1859&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_kvrocks&pullRequest=1859&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_kvrocks&pullRequest=1859&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_kvrocks&pullRequest=1859&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_kvrocks&pullRequest=1859&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_kvrocks&pullRequest=1859&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_kvrocks&pullRequest=1859) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_kvrocks&pullRequest=1859&metric=duplicated_lines_density&view=list) No Duplication information
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support for the JSON.DEL command [kvrocks]
Posted by "Qiaolin-Yu (via GitHub)" <gi...@apache.org>.
Qiaolin-Yu commented on PR #1859:
URL: https://github.com/apache/kvrocks/pull/1859#issuecomment-1828251892
I have updated this PR based on https://github.com/apache/kvrocks/pull/1911. PTAL @PragmaTwice .
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support for the JSON.DEL command [kvrocks]
Posted by "Qiaolin-Yu (via GitHub)" <gi...@apache.org>.
Qiaolin-Yu commented on code in PR #1859:
URL: https://github.com/apache/kvrocks/pull/1859#discussion_r1375503356
##########
src/types/json.h:
##########
@@ -215,6 +215,30 @@ struct JsonValue {
return Status::OK();
}
+ StatusOr<size_t> Del(const std::string &path) {
+ const std::string parent_path = path + "^";
+ const std::string child_path = path.substr(path.find_last_of('.') + 1);
Review Comment:
According to https://github.com/danielaparker/jsoncons/issues/467#issuecomment-1784123066, it seems that `jsoncons` has not such feature. ðŸ˜
##########
src/types/json.h:
##########
@@ -215,6 +215,30 @@ struct JsonValue {
return Status::OK();
}
+ StatusOr<size_t> Del(const std::string &path) {
+ const std::string parent_path = path + "^";
+ const std::string child_path = path.substr(path.find_last_of('.') + 1);
Review Comment:
According to https://github.com/danielaparker/jsoncons/issues/467#issuecomment-1784123066, it seems that `jsoncons` has not such feature. ðŸ˜
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org