You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@kvrocks.apache.org by "PragmaTwice (via GitHub)" <gi...@apache.org> on 2024/05/30 14:42:02 UTC

[PR] chore(storage): remove FindKeyRangeWithPrefix and fix DeleteRange [kvrocks]

PragmaTwice opened a new pull request, #2343:
URL: https://github.com/apache/kvrocks/pull/2343

   The method `FindKeyRangeWithPrefix` seems useless since we can always construct a key range quickly, without knowing the exact first key and last key in the range.


-- 
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] chore(storage): remove FindKeyRangeWithPrefix and fix DeleteRange [kvrocks]

Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #2343:
URL: https://github.com/apache/kvrocks/pull/2343#discussion_r1622178210


##########
src/common/string_util.cc:
##########
@@ -377,4 +377,14 @@ std::string EscapeString(std::string_view s) {
   return str;
 }
 
+std::string StringNext(std::string s) {
+  for (auto iter = s.rbegin(); iter != s.rend(); ++iter) {
+    if (*iter != char(0xff)) {
+      (*iter)++;
+      break;
+    }
+  }
+  return s;

Review Comment:
   I think the semantics is different between appending 0x00 and increasing last char by 1.



-- 
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] chore(storage): remove FindKeyRangeWithPrefix and fix DeleteRange [kvrocks]

Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #2343:
URL: https://github.com/apache/kvrocks/pull/2343#discussion_r1622178210


##########
src/common/string_util.cc:
##########
@@ -377,4 +377,14 @@ std::string EscapeString(std::string_view s) {
   return str;
 }
 
+std::string StringNext(std::string s) {
+  for (auto iter = s.rbegin(); iter != s.rend(); ++iter) {
+    if (*iter != char(0xff)) {
+      (*iter)++;
+      break;
+    }
+  }
+  return s;

Review Comment:
   I think the semantics is different between append 0x00 and back char plus 1.



-- 
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] chore(storage): remove FindKeyRangeWithPrefix and fix DeleteRange [kvrocks]

Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU commented on code in PR #2343:
URL: https://github.com/apache/kvrocks/pull/2343#discussion_r1621647607


##########
src/common/string_util.cc:
##########
@@ -377,4 +377,14 @@ std::string EscapeString(std::string_view s) {
   return str;
 }
 
+std::string StringNext(std::string s) {
+  for (auto iter = s.rbegin(); iter != s.rend(); ++iter) {
+    if (*iter != char(0xff)) {
+      (*iter)++;
+      break;
+    }
+  }
+  return s;

Review Comment:
   should we append a '0x00' if `s == return s`?



-- 
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] chore(storage): remove FindKeyRangeWithPrefix and fix DeleteRange [kvrocks]

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #2343:
URL: https://github.com/apache/kvrocks/pull/2343#issuecomment-2141806398

   ## [![Quality Gate Failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-failed-20px.png 'Quality Gate Failed')](https://sonarcloud.io/dashboard?id=apache_kvrocks&pullRequest=2343) **Quality Gate failed**  
   Failed conditions  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/failed-16px.png '') [C Reliability Rating on New Code](https://sonarcloud.io/dashboard?id=apache_kvrocks&pullRequest=2343) (required ≥ A)  
     
   [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_kvrocks&pullRequest=2343)
   
   ##   
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/light_bulb-16px.png '') Catch issues before they fail your Quality Gate with our IDE extension ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/sonarlint-16px.png '') [SonarLint](https://www.sonarsource.com/products/sonarlint/features/connected-mode/?referrer=pull-request)
   
   


-- 
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] chore(storage): remove FindKeyRangeWithPrefix and fix DeleteRange [kvrocks]

Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #2343:
URL: https://github.com/apache/kvrocks/pull/2343#discussion_r1622178210


##########
src/common/string_util.cc:
##########
@@ -377,4 +377,14 @@ std::string EscapeString(std::string_view s) {
   return str;
 }
 
+std::string StringNext(std::string s) {
+  for (auto iter = s.rbegin(); iter != s.rend(); ++iter) {
+    if (*iter != char(0xff)) {
+      (*iter)++;
+      break;
+    }
+  }
+  return s;

Review Comment:
   I think the semantics is different between append 0x00 and last char plus 1.



##########
src/common/string_util.cc:
##########
@@ -377,4 +377,14 @@ std::string EscapeString(std::string_view s) {
   return str;
 }
 
+std::string StringNext(std::string s) {
+  for (auto iter = s.rbegin(); iter != s.rend(); ++iter) {
+    if (*iter != char(0xff)) {
+      (*iter)++;
+      break;
+    }
+  }
+  return s;

Review Comment:
   I think the semantics is different between appending 0x00 and last char plus 1.



-- 
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] chore(storage): remove FindKeyRangeWithPrefix and fix DeleteRange [kvrocks]

Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice merged PR #2343:
URL: https://github.com/apache/kvrocks/pull/2343


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