torwig commented on code in PR #1710:
URL: https://github.com/apache/kvrocks/pull/1710#discussion_r1309902340


##########
src/types/redis_bloom_chain.cc:
##########
@@ -185,14 +178,35 @@ rocksdb::Status BloomChain::Exist(const Slice &user_key, 
const Slice &item, int
 
   std::string item_string = item.ToString();
   // check
+  bool exist = false;
   for (int i = metadata.n_filters - 1; i >= 0; --i) {  // TODO: to test which 
direction for searching is better
-    s = bloomCheck(bf_key_list[i], item_string, ret);
+    s = bloomCheck(bf_key_list[i], item_string, &exist);
     if (!s.ok()) return s;
-    if (*ret == 1) {
+    if (exist) {
+      *ret = 1;
       break;
     }
   }
 
   return rocksdb::Status::OK();
 }
+
+rocksdb::Status BloomChain::Info(const Slice &user_key, BloomFilterInfo *info) 
{
+  std::string ns_key = AppendNamespacePrefix(user_key);
+  LockGuard guard(storage_->GetLockManager(), ns_key);
+
+  BloomChainMetadata metadata;
+  rocksdb::Status s = getBloomChainMetadata(ns_key, &metadata);
+  if (s.IsNotFound()) return rocksdb::Status::NotFound("key is not found");

Review Comment:
   Why not simply use `if (!s.ok()) return s;` that will return the 
`rocksdb::Status::NotFound` status as well? Then check on the caller side if 
the status is NotFound if necessary.



##########
src/types/redis_bloom_chain.h:
##########
@@ -30,20 +30,38 @@ const uint32_t kBFDefaultInitCapacity = 100;
 const double kBFDefaultErrorRate = 0.01;
 const uint16_t kBFDefaultExpansion = 2;
 
+enum class BloomInfoType {
+  ALL,

Review Comment:
   You can format enum values as `kAll, kCapacity, etc` (see here: 
https://google.github.io/styleguide/cppguide.html#Enumerator_Names)



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