Fix parser

apache · Aug 31, 2023 · 83af435 · 83af435
1 parent 82d439a
commit 83af435
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 6 deletions.
diff --git a/src/commands/cmd_bloom_filter.cc b/src/commands/cmd_bloom_filter.cc
@@ -53,6 +53,7 @@ class CommandBFReserve : public Commander {
     while (parser.Good()) {
       if (parser.EatEqICase("nonscaling")) {
         is_nonscaling = true;
+        expansion_ = 0;
       } else if (parser.EatEqICase("expansion")) {
         has_expansion = true;
         expansion_ = GET_OR_RET(parser.TakeInt<uint16_t>());
@@ -116,8 +117,11 @@ class CommandBFExists : public Commander {
 class CommandBFInfo : public Commander {
  public:
   Status Parse(const std::vector<std::string> &args) override {
+    if (args.size() > 3) {
+      return {Status::RedisParseErr, errWrongNumOfArguments};
+    }
     CommandParser parser(args, 2);
-    while (parser.Good()) {
+    if (parser.Good()) {
       if (parser.EatEqICase("capacity")) {
         type_ = BloomInfoType::kCapacity;
       } else if (parser.EatEqICase("size")) {
@@ -155,7 +159,7 @@ class CommandBFInfo : public Commander {
         *output += redis::SimpleString("Number of items inserted");
         *output += redis::Integer(info.size);
         *output += redis::SimpleString("Expansion rate");
-        *output += redis::Integer(info.expansion);
+        *output += info.expansion == 0 ? redis::NilString() : redis::Integer(info.expansion);
         break;
       case BloomInfoType::kCapacity:
         *output = redis::Integer(info.capacity);
@@ -170,10 +174,8 @@ class CommandBFInfo : public Commander {
         *output = redis::Integer(info.size);
         break;
       case BloomInfoType::kExpansion:
-        *output = redis::Integer(info.expansion);
+        *output = info.expansion == 0 ? redis::NilString() : redis::Integer(info.expansion);
         break;
-      default:
-        LOG(ERROR) << "Failed to parse the type of BF.INFO command";
     }
 
     return Status::OK();

diff --git a/src/types/redis_bloom_chain.cc b/src/types/redis_bloom_chain.cc
@@ -190,7 +190,6 @@ rocksdb::Status BloomChain::Exist(const Slice &user_key, const Slice &item, int
 
 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);

diff --git a/tests/gocase/unit/type/bloom/bloom_test.go b/tests/gocase/unit/type/bloom/bloom_test.go
@@ -21,6 +21,7 @@ package bloom
 
 import (
 	"context"
+	"github.com/redis/go-redis/v9"
 	"testing"
 
 	"github.com/apache/kvrocks/tests/gocase/util"
@@ -146,6 +147,13 @@ func TestBloom(t *testing.T) {
 		require.NoError(t, rdb.Del(ctx, "no_exist_key").Err())
 	})
 
+	t.Run("Get info but wrong arguments", func(t *testing.T) {
+		require.NoError(t, rdb.Del(ctx, key).Err())
+		require.NoError(t, rdb.Do(ctx, "bf.reserve", key, "0.01", "2000", "nonscaling").Err())
+		require.ErrorContains(t, rdb.Do(ctx, "bf.info", key, "xxx").Err(), "Invalid info argument")
+		require.ErrorContains(t, rdb.Do(ctx, "bf.info", key, "capacity", "items").Err(), "wrong number of arguments")
+	})
+
 	t.Run("Get all info of bloom filter", func(t *testing.T) {
 		require.NoError(t, rdb.Del(ctx, key).Err())
 		require.NoError(t, rdb.Do(ctx, "bf.reserve", key, "0.02", "1000", "expansion", "3").Err())
@@ -171,6 +179,12 @@ func TestBloom(t *testing.T) {
 		require.Equal(t, int64(2), rdb.Do(ctx, "bf.info", key, "expansion").Val())
 	})
 
+	t.Run("Get expansion of nonscaling bloom filter", func(t *testing.T) {
+		require.NoError(t, rdb.Del(ctx, key).Err())
+		require.NoError(t, rdb.Do(ctx, "bf.reserve", key, "0.01", "2000", "nonscaling").Err())
+		require.Equal(t, redis.Nil, rdb.Do(ctx, "bf.info", key, "expansion").Err())
+	})
+
 	t.Run("Get size of bloom filter", func(t *testing.T) {
 		require.NoError(t, rdb.Del(ctx, key).Err())
 		require.NoError(t, rdb.Do(ctx, "bf.reserve", key, "0.02", "1000", "expansion", "1").Err())