jihuayu commented on code in PR #3467:
URL: https://github.com/apache/kvrocks/pull/3467#discussion_r3613374414


##########
src/types/redis_cuckoo_chain.cc:
##########
@@ -291,4 +291,55 @@ rocksdb::Status 
CuckooChain::commitSubFilterAndMetadata(engine::Context &ctx, co
   return storage_->Write(ctx, storage_->DefaultWriteOptions(), 
batch->GetWriteBatch());
 }
 
+rocksdb::Status CuckooChain::Exists(engine::Context &ctx, const Slice 
&user_key, const Slice &item, bool *exists) {
+  std::vector<bool> result;
+  auto s = MExists(ctx, user_key, std::vector<std::string>{item.ToString()}, 
&result);
+  if (!s.ok()) return s;
+  *exists = result[0];
+  return rocksdb::Status::OK();
+}
+
+rocksdb::Status CuckooChain::MExists(engine::Context &ctx, const Slice 
&user_key, const std::vector<std::string> &items,
+                                     std::vector<bool> *exists) {
+  exists->assign(items.size(), false);
+  std::string ns_key = AppendNamespacePrefix(user_key);
+
+  CuckooChainMetadata metadata(false);
+  auto s = getCuckooChainMetadata(ctx, ns_key, &metadata);
+  if (s.IsNotFound()) return rocksdb::Status::OK();
+  if (!s.ok()) return s;
+
+  s = validateMetadata(metadata);
+  if (!s.ok()) return s;
+
+  std::vector<uint64_t> hashes(items.size());
+  std::vector<uint8_t> fingerprints(items.size());
+  for (size_t i = 0; i < items.size(); ++i) {
+    hashes[i] = CuckooFilterHelper::Hash(items[i].data(), items[i].size());
+    fingerprints[i] = CuckooFilterHelper::GenerateFingerprint(hashes[i]);
+    CHECK(fingerprints[i] != 0);
+  }
+
+  for (int filter_idx = static_cast<int>(metadata.n_filters) - 1; filter_idx 
>= 0; --filter_idx) {
+    auto current_filter_idx = static_cast<uint16_t>(filter_idx);
+    uint32_t num_buckets = 0;
+    s = CuckooFilterHelper::GetFilterNumBuckets(metadata.base_capacity, 
metadata.expansion, metadata.bucket_size,
+                                                current_filter_idx, 
&num_buckets);
+    if (!s.ok()) return s;
+
+    CuckooSubFilter sub_filter(storage_, ctx, ns_key, 
storage_->IsSlotIdEncoded(), metadata.version,
+                               metadata.bucket_size, metadata.page_size, 
current_filter_idx, num_buckets);
+    for (size_t i = 0; i < items.size(); ++i) {
+      if ((*exists)[i]) continue;
+
+      bool item_exists = false;
+      s = sub_filter.Contains(hashes[i], fingerprints[i], &item_exists);
+      if (!s.ok()) return s;
+      if (item_exists) (*exists)[i] = true;

Review Comment:
   If we find all the items early, we should exit immediately instead of 
continuing the loop. This can improve performance.
   
   



##########
src/commands/cmd_cuckoo_filter.cc:
##########
@@ -131,8 +131,69 @@ class CommandCFAdd : public Commander {
   }
 };
 
-// Register the CF.RESERVE and CF.ADD commands
+class CommandCFExists : public Commander {
+ public:
+  Status Parse(const std::vector<std::string> &args) override {
+    // CF.EXISTS key item
+    if (args.size() != 3) {
+      return {Status::RedisParseErr, errWrongNumOfArguments};
+    }
+    return Commander::Parse(args);
+  }
+
+  Status Execute(engine::Context &ctx, Server *srv, Connection *conn, 
std::string *output) override {
+    redis::CuckooChain cuckoo_db(srv->storage, conn->GetNamespace());
+    bool exists = false;
+    auto s = cuckoo_db.Exists(ctx, args_[1], args_[2], &exists);
+
+    if (!s.ok()) {
+      return {Status::RedisExecErr, s.ToString()};
+    }
+
+    // Return 1 if exists (might exist), 0 if doesn't exist (definitely not)
+    *output = redis::Integer(exists ? 1 : 0);

Review Comment:
   We need return bool `*output = conn->Bool(exists);`
   
   <img width="1277" height="561" alt="Image" 
src="https://github.com/user-attachments/assets/d8865d81-4733-49ed-98ff-85db3a58b385";
 />
   
   https://redis.io/docs/latest/commands/cf.exists/



##########
src/commands/cmd_cuckoo_filter.cc:
##########
@@ -131,8 +131,69 @@ class CommandCFAdd : public Commander {
   }
 };
 
-// Register the CF.RESERVE and CF.ADD commands
+class CommandCFExists : public Commander {
+ public:
+  Status Parse(const std::vector<std::string> &args) override {
+    // CF.EXISTS key item
+    if (args.size() != 3) {
+      return {Status::RedisParseErr, errWrongNumOfArguments};
+    }
+    return Commander::Parse(args);
+  }
+
+  Status Execute(engine::Context &ctx, Server *srv, Connection *conn, 
std::string *output) override {
+    redis::CuckooChain cuckoo_db(srv->storage, conn->GetNamespace());
+    bool exists = false;
+    auto s = cuckoo_db.Exists(ctx, args_[1], args_[2], &exists);
+
+    if (!s.ok()) {
+      return {Status::RedisExecErr, s.ToString()};
+    }
+
+    // Return 1 if exists (might exist), 0 if doesn't exist (definitely not)
+    *output = redis::Integer(exists ? 1 : 0);
+    return Status::OK();
+  }
+};
+
+class CommandCFMExists : public Commander {
+ public:
+  Status Parse(const std::vector<std::string> &args) override {
+    // CF.MEXISTS key item [item ...]
+    if (args.size() < 3) {
+      return {Status::RedisParseErr, errWrongNumOfArguments};
+    }
+    items_.reserve(args.size() - 2);
+    for (size_t i = 2; i < args.size(); ++i) {
+      items_.emplace_back(args[i]);
+    }
+    return Commander::Parse(args);
+  }
+
+  Status Execute(engine::Context &ctx, Server *srv, Connection *conn, 
std::string *output) override {
+    redis::CuckooChain cuckoo_db(srv->storage, conn->GetNamespace());
+    std::vector<bool> exists(items_.size(), false);
+    auto s = cuckoo_db.MExists(ctx, args_[1], items_, &exists);
+
+    if (!s.ok()) {
+      return {Status::RedisExecErr, s.ToString()};
+    }
+
+    *output = redis::MultiLen(items_.size());
+    for (bool exist : exists) {
+      *output += redis::Integer(exist ? 1 : 0);

Review Comment:
   ```suggestion
         *output += conn->Bool(exists);
   ```



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