This is an automated email from the ASF dual-hosted git repository.
hulk pushed a commit to branch unstable
in repository https://gitbox.apache.org/repos/asf/incubator-kvrocks.git
The following commit(s) were added to refs/heads/unstable by this push:
new d0d5e751 Align the expire option behavior of redis (#1017)
d0d5e751 is described below
commit d0d5e751fe362cc3c4760a63b022e3891cc4e2e3
Author: Ruixiang Tan <[email protected]>
AuthorDate: Sun Oct 23 10:15:11 2022 +0800
Align the expire option behavior of redis (#1017)
Co-authored-by: tison <[email protected]>
---
src/commands/redis_cmd.cc | 23 +++-
tests/gocase/unit/type/strings/strings_test.go | 139 +++++++++++++++++++++++--
2 files changed, 151 insertions(+), 11 deletions(-)
diff --git a/src/commands/redis_cmd.cc b/src/commands/redis_cmd.cc
index 1054c30c..233acada 100644
--- a/src/commands/redis_cmd.cc
+++ b/src/commands/redis_cmd.cc
@@ -43,6 +43,7 @@
#include "server/server.h"
#include "stats/disk_stats.h"
#include "stats/log_collector.h"
+#include "status.h"
#include "storage/redis_db.h"
#include "storage/redis_pubsub.h"
#include "storage/scripting.h"
@@ -103,24 +104,28 @@ Status ParseTTL(const std::vector<std::string> &args,
std::unordered_map<std::st
int ttl = 0;
int64_t expire = 0;
bool last_arg = false;
+ bool has_ex = false, has_exat = false, has_pxat = false, has_px = false;
for (size_t i = 0; i < args.size(); i++) {
last_arg = (i == args.size() - 1);
std::string opt = Util::ToLower(args[i]);
- if (opt == "ex" && !ttl && !last_arg) {
+ if (opt == "ex" && !last_arg) {
+ has_ex = 1;
auto parse_result = ParseInt<int>(args[++i], 10);
if (!parse_result) {
return Status(Status::RedisParseErr, errValueNotInteger);
}
ttl = *parse_result;
if (ttl <= 0) return Status(Status::RedisParseErr, errInvalidExpireTime);
- } else if (opt == "exat" && !ttl && !expire && !last_arg) {
+ } else if (opt == "exat" && !last_arg) {
+ has_exat = 1;
auto parse_result = ParseInt<int64_t>(args[++i], 10);
if (!parse_result) {
return Status(Status::RedisParseErr, errValueNotInteger);
}
expire = *parse_result;
if (expire <= 0) return Status(Status::RedisParseErr,
errInvalidExpireTime);
- } else if (opt == "pxat" && !ttl && !expire && !last_arg) {
+ } else if (opt == "pxat" && !last_arg) {
+ has_pxat = 1;
auto parse_result = ParseInt<uint64_t>(args[++i], 10);
if (!parse_result) {
return Status(Status::RedisParseErr, errValueNotInteger);
@@ -132,7 +137,8 @@ Status ParseTTL(const std::vector<std::string> &args,
std::unordered_map<std::st
} else {
expire = static_cast<int64_t>(expire_ms / 1000);
}
- } else if (opt == "px" && !ttl && !last_arg) {
+ } else if (opt == "px" && !last_arg) {
+ has_px = 1;
int64_t ttl_ms = 0;
auto parse_result = ParseInt<int64_t>(args[++i], 10);
if (!parse_result) {
@@ -154,6 +160,9 @@ Status ParseTTL(const std::vector<std::string> &args,
std::unordered_map<std::st
}
}
}
+ if (has_px + has_ex + has_exat + has_pxat > 1) {
+ return Status(Status::NotOK, errInvalidSyntax);
+ }
if (!ttl && expire) {
int64_t now;
rocksdb::Env::Default()->GetCurrentTime(&now);
@@ -829,10 +838,12 @@ class CommandCAS : public Commander {
public:
Status Parse(const std::vector<std::string> &args) override {
bool last_arg;
+ bool ex_exist = false, px_exist = false;
for (size_t i = 4; i < args.size(); i++) {
last_arg = (i == args.size() - 1);
std::string opt = Util::ToLower(args[i]);
if (opt == "ex") {
+ ex_exist = true;
if (last_arg) return Status(Status::NotOK, errWrongNumOfArguments);
auto parse_result = ParseInt<int>(args_[++i].c_str(), 10);
if (!parse_result) {
@@ -841,6 +852,7 @@ class CommandCAS : public Commander {
ttl_ = *parse_result;
if (ttl_ <= 0) return Status(Status::RedisParseErr,
errInvalidExpireTime);
} else if (opt == "px") {
+ px_exist = true;
if (last_arg) return Status(Status::NotOK, errWrongNumOfArguments);
auto parse_result = ParseInt<int>(args[++i].c_str(), 10);
if (!parse_result) {
@@ -858,6 +870,9 @@ class CommandCAS : public Commander {
return Status(Status::NotOK, errInvalidSyntax);
}
}
+ if (ex_exist + px_exist > 1) {
+ return Status(Status::NotOK, errInvalidSyntax);
+ }
return Commander::Parse(args);
}
diff --git a/tests/gocase/unit/type/strings/strings_test.go
b/tests/gocase/unit/type/strings/strings_test.go
index e76a8f4d..ebfb6e4f 100644
--- a/tests/gocase/unit/type/strings/strings_test.go
+++ b/tests/gocase/unit/type/strings/strings_test.go
@@ -128,7 +128,7 @@ func TestString(t *testing.T) {
require.NoError(t, rdb.Expire(ctx, "x", time.Second).Err())
// Wait for the key to expire
- time.Sleep(2000 * time.Millisecond)
+ time.Sleep(2 * time.Second)
require.NoError(t, rdb.SetNX(ctx, "x", "20", 0).Err())
require.Equal(t, "20", rdb.Get(ctx, "x").Val())
@@ -141,11 +141,26 @@ func TestString(t *testing.T) {
util.BetweenValues(t, rdb.TTL(ctx, "foo").Val(), 5*time.Second,
10*time.Second)
})
+ t.Run("GETEX Duplicate EX option", func(t *testing.T) {
+ require.NoError(t, rdb.Del(ctx, "foo").Err())
+ require.NoError(t, rdb.Set(ctx, "foo", "bar", 0).Err())
+ require.NoError(t, rdb.GetEx(ctx, "foo", 10*time.Second).Err())
+ require.NoError(t, rdb.Do(ctx, "getex", "foo", "ex", 1, "ex",
10).Err())
+ util.BetweenValues(t, rdb.TTL(ctx, "foo").Val(), 5*time.Second,
10*time.Second)
+ })
+
t.Run("GETEX PX option", func(t *testing.T) {
require.NoError(t, rdb.Del(ctx, "foo").Err())
require.NoError(t, rdb.Set(ctx, "foo", "bar", 0).Err())
- require.NoError(t, rdb.GetEx(ctx, "foo",
10000*time.Millisecond).Err())
- util.BetweenValues(t, rdb.TTL(ctx, "foo").Val(),
5000*time.Millisecond, 10000*time.Millisecond)
+ require.NoError(t, rdb.GetEx(ctx, "foo", 10*time.Second).Err())
+ util.BetweenValues(t, rdb.TTL(ctx, "foo").Val(), 5*time.Second,
10*time.Second)
+ })
+
+ t.Run("GETEX Duplicate PX option", func(t *testing.T) {
+ require.NoError(t, rdb.Del(ctx, "foo").Err())
+ require.NoError(t, rdb.Set(ctx, "foo", "bar", 0).Err())
+ require.NoError(t, rdb.Do(ctx, "getex", "foo", "px", 1, "px",
10000).Err())
+ util.BetweenValues(t, rdb.TTL(ctx, "foo").Val(), 5*time.Second,
10*time.Second)
})
t.Run("GETEX EXAT option", func(t *testing.T) {
@@ -155,11 +170,25 @@ func TestString(t *testing.T) {
util.BetweenValues(t, rdb.TTL(ctx, "foo").Val(), 5*time.Second,
10*time.Second)
})
+ t.Run("GETEX Duplicate EXAT option", func(t *testing.T) {
+ require.NoError(t, rdb.Del(ctx, "foo").Err())
+ require.NoError(t, rdb.Set(ctx, "foo", "bar", 0).Err())
+ require.NoError(t, rdb.Do(ctx, "getex", "foo", "exat",
time.Now().Add(100*time.Second).Unix(), "exat",
time.Now().Add(10*time.Second).Unix()).Err())
+ util.BetweenValues(t, rdb.TTL(ctx, "foo").Val(), 5*time.Second,
10*time.Second)
+ })
+
t.Run("GETEX PXAT option", func(t *testing.T) {
require.NoError(t, rdb.Del(ctx, "foo").Err())
require.NoError(t, rdb.Set(ctx, "foo", "bar", 0).Err())
- require.NoError(t, rdb.Do(ctx, "getex", "foo", "pxat",
time.Now().Add(10000*time.Millisecond).UnixMilli()).Err())
- util.BetweenValues(t, rdb.TTL(ctx, "foo").Val(),
5000*time.Millisecond, 10000*time.Millisecond)
+ require.NoError(t, rdb.Do(ctx, "getex", "foo", "pxat",
time.Now().Add(10*time.Second).UnixMilli()).Err())
+ util.BetweenValues(t, rdb.TTL(ctx, "foo").Val(), 5*time.Second,
10*time.Second)
+ })
+
+ t.Run("GETEX Duplicate PXAT option", func(t *testing.T) {
+ require.NoError(t, rdb.Del(ctx, "foo").Err())
+ require.NoError(t, rdb.Set(ctx, "foo", "bar", 0).Err())
+ require.NoError(t, rdb.Do(ctx, "getex", "foo", "pxat",
time.Now().Add(1000*time.Second).UnixMilli(), "pxat",
time.Now().Add(10*time.Second).UnixMilli()).Err())
+ util.BetweenValues(t, rdb.TTL(ctx, "foo").Val(), 5*time.Second,
10*time.Second)
})
t.Run("GETEX PERSIST option", func(t *testing.T) {
@@ -168,6 +197,32 @@ func TestString(t *testing.T) {
util.BetweenValues(t, rdb.TTL(ctx, "foo").Val(), 5*time.Second,
10*time.Second)
require.NoError(t, rdb.Do(ctx, "getex", "foo", "persist").Err())
require.EqualValues(t, -1, rdb.TTL(ctx, "foo").Val())
+ require.ErrorContains(t, rdb.Do(ctx, "getex", "foo", "ex", 10,
"persist").Err(), "syntax err")
+ require.ErrorContains(t, rdb.Do(ctx, "getex", "foo", "px",
10000, "persist").Err(), "syntax err")
+ require.ErrorContains(t, rdb.Do(ctx, "getex", "foo", "pxat",
time.Now().Add(10*time.Second).UnixMilli(), "persist").Err(), "syntax err")
+ require.ErrorContains(t, rdb.Do(ctx, "getex", "foo", "exat",
time.Now().Add(100*time.Second).Unix(), "persist").Err(), "syntax err")
+
+ require.ErrorContains(t, rdb.Do(ctx, "getex", "foo", "persist",
"ex", 10).Err(), "syntax err")
+ require.ErrorContains(t, rdb.Do(ctx, "getex", "foo", "persist",
"px", 10000).Err(), "syntax err")
+ require.ErrorContains(t, rdb.Do(ctx, "getex", "foo", "persist",
"pxat", time.Now().Add(10*time.Second).UnixMilli()).Err(), "syntax err")
+ require.ErrorContains(t, rdb.Do(ctx, "getex", "foo", "persist",
"exat", time.Now().Add(100*time.Second).Unix()).Err(), "syntax err")
+
+ })
+
+ t.Run("GETEX with incorrect use of multi options should result in
syntax err", func(t *testing.T) {
+ require.ErrorContains(t, rdb.Do(ctx, "getex", "foo", "px", 100,
"ex", 10).Err(), "syntax err")
+ require.ErrorContains(t, rdb.Do(ctx, "getex", "foo", "px", 100,
"pxat", time.Now().Add(10*time.Second).UnixMilli()).Err(), "syntax err")
+ require.ErrorContains(t, rdb.Do(ctx, "getex", "foo", "px", 100,
"exat", time.Now().Add(10*time.Second).Unix()).Err(), "syntax err")
+ require.ErrorContains(t, rdb.Do(ctx, "getex", "foo", "ex", 10,
"pxat", time.Now().Add(10*time.Second).UnixMilli()).Err(), "syntax err")
+ require.ErrorContains(t, rdb.Do(ctx, "getex", "foo", "ex", 10,
"exat", time.Now().Add(10*time.Second).Unix()).Err(), "syntax err")
+ require.ErrorContains(t, rdb.Do(ctx, "getex", "foo", "pxat",
time.Now().Add(10*time.Second).UnixMilli(), "exat",
time.Now().Add(10*time.Second).Unix()).Err(), "syntax err")
+
+ require.ErrorContains(t, rdb.Do(ctx, "getex", "foo", "ex", 10,
"px", 100).Err(), "syntax err")
+ require.ErrorContains(t, rdb.Do(ctx, "getex", "foo", "pxat",
time.Now().Add(10*time.Second).UnixMilli(), "px", 100).Err(), "syntax err")
+ require.ErrorContains(t, rdb.Do(ctx, "getex", "foo", "exat",
time.Now().Add(10*time.Second).Unix(), "px", 100).Err(), "syntax err")
+ require.ErrorContains(t, rdb.Do(ctx, "getex", "foo", "pxat",
time.Now().Add(10*time.Second).UnixMilli(), "ex", 10).Err(), "syntax err")
+ require.ErrorContains(t, rdb.Do(ctx, "getex", "foo", "exat",
time.Now().Add(10*time.Second).Unix(), "ex", 10).Err(), "syntax err")
+ require.ErrorContains(t, rdb.Do(ctx, "getex", "foo", "exat",
time.Now().Add(10*time.Second).Unix(), "pxat",
time.Now().Add(10*time.Second).UnixMilli()).Err(), "syntax err")
})
t.Run("GETEX no option", func(t *testing.T) {
@@ -445,6 +500,13 @@ func TestString(t *testing.T) {
util.BetweenValues(t, ttl, 5*time.Second, 10*time.Second)
})
+ t.Run("Extended SET Duplicate EX option", func(t *testing.T) {
+ require.NoError(t, rdb.Del(ctx, "foo").Err())
+ require.Equal(t, "OK", rdb.Do(ctx, "SET", "foo", "bar", "ex",
"1", "ex", "2", "ex", "10").Val())
+ ttl := rdb.TTL(ctx, "foo").Val()
+ util.BetweenValues(t, ttl, 5*time.Second, 10*time.Second)
+ })
+
t.Run("Extended SET PX option", func(t *testing.T) {
require.NoError(t, rdb.Del(ctx, "foo").Err())
require.Equal(t, "OK", rdb.Do(ctx, "SET", "foo", "bar", "px",
"10000").Val())
@@ -452,6 +514,13 @@ func TestString(t *testing.T) {
util.BetweenValues(t, ttl, 5*time.Second, 10*time.Second)
})
+ t.Run("Extended SET Duplicate PX option", func(t *testing.T) {
+ require.NoError(t, rdb.Del(ctx, "foo").Err())
+ require.Equal(t, "OK", rdb.Do(ctx, "SET", "foo", "bar", "px",
"100", "px", "1000", "px", "10000").Val())
+ ttl := rdb.TTL(ctx, "foo").Val()
+ util.BetweenValues(t, ttl, 5*time.Second, 10*time.Second)
+ })
+
t.Run("Extended SET EXAT option", func(t *testing.T) {
require.NoError(t, rdb.Del(ctx, "foo").Err())
@@ -461,6 +530,16 @@ func TestString(t *testing.T) {
util.BetweenValues(t, ttl, 5*time.Second, 10*time.Second)
})
+ t.Run("Extended SET Duplicate EXAT option", func(t *testing.T) {
+ require.NoError(t, rdb.Del(ctx, "foo").Err())
+
+ expireFirst :=
strconv.FormatInt(time.Now().Add(1*time.Second).Unix(), 10)
+ expireSecond :=
strconv.FormatInt(time.Now().Add(10*time.Second).Unix(), 10)
+ require.Equal(t, "OK", rdb.Do(ctx, "SET", "foo", "bar", "exat",
expireFirst, "exat", expireSecond).Val())
+ ttl := rdb.TTL(ctx, "foo").Val()
+ util.BetweenValues(t, ttl, 5*time.Second, 10*time.Second)
+ })
+
t.Run("Extended SET EXAT option with expired timestamp", func(t
*testing.T) {
require.NoError(t, rdb.Del(ctx, "foo").Err())
require.Equal(t, "OK", rdb.Do(ctx, "SET", "foo", "bar", "exat",
"1").Val())
@@ -484,6 +563,16 @@ func TestString(t *testing.T) {
util.BetweenValues(t, ttl, 5*time.Second, 10*time.Second)
})
+ t.Run("Extended SET Duplicate PXAT option", func(t *testing.T) {
+ require.NoError(t, rdb.Del(ctx, "foo").Err())
+
+ expireFirst :=
strconv.FormatInt(time.Now().Add(1*time.Second).UnixMilli(), 10)
+ expireSecond :=
strconv.FormatInt(time.Now().Add(10*time.Second).UnixMilli(), 10)
+ require.Equal(t, "OK", rdb.Do(ctx, "SET", "foo", "bar", "pxat",
expireFirst, "pxat", expireSecond).Val())
+ ttl := rdb.TTL(ctx, "foo").Val()
+ util.BetweenValues(t, ttl, 5*time.Second, 10*time.Second)
+ })
+
t.Run("Extended SET PXAT option with expired timestamp", func(t
*testing.T) {
require.NoError(t, rdb.Del(ctx, "foo").Err())
require.Equal(t, "OK", rdb.Do(ctx, "SET", "foo", "bar", "pxat",
"1").Val())
@@ -497,8 +586,22 @@ func TestString(t *testing.T) {
})
t.Run("Extended SET with incorrect use of multi options should result
in syntax err", func(t *testing.T) {
- require.ErrorContains(t, rdb.Do(ctx, "SET", "foo", "bar", "ex",
"10", "px", "10000").Err(), "syntax err")
+ require.ErrorContains(t, rdb.Do(ctx, "SET", "foo", "bar", "px",
100, "ex", 10).Err(), "syntax err")
+ require.ErrorContains(t, rdb.Do(ctx, "SET", "foo", "bar", "px",
100, "pxat", time.Now().Add(10*time.Second).UnixMilli()).Err(), "syntax err")
+ require.ErrorContains(t, rdb.Do(ctx, "SET", "foo", "bar", "px",
100, "exat", time.Now().Add(10*time.Second).Unix()).Err(), "syntax err")
+ require.ErrorContains(t, rdb.Do(ctx, "SET", "foo", "bar", "ex",
10, "pxat", time.Now().Add(10*time.Second).UnixMilli()).Err(), "syntax err")
+ require.ErrorContains(t, rdb.Do(ctx, "SET", "foo", "bar", "ex",
10, "exat", time.Now().Add(10*time.Second).Unix()).Err(), "syntax err")
+ require.ErrorContains(t, rdb.Do(ctx, "SET", "foo", "bar",
"pxat", time.Now().Add(10*time.Second).UnixMilli(), "exat",
time.Now().Add(10*time.Second).Unix()).Err(), "syntax err")
+
+ require.ErrorContains(t, rdb.Do(ctx, "SET", "foo", "bar", "ex",
10, "px", 100).Err(), "syntax err")
+ require.ErrorContains(t, rdb.Do(ctx, "SET", "foo", "bar",
"pxat", time.Now().Add(10*time.Second).UnixMilli(), "px", 100).Err(), "syntax
err")
+ require.ErrorContains(t, rdb.Do(ctx, "SET", "foo", "bar",
"exat", time.Now().Add(10*time.Second).Unix(), "px", 100).Err(), "syntax err")
+ require.ErrorContains(t, rdb.Do(ctx, "SET", "foo", "bar",
"pxat", time.Now().Add(10*time.Second).UnixMilli(), "ex", 10).Err(), "syntax
err")
+ require.ErrorContains(t, rdb.Do(ctx, "SET", "foo", "bar",
"exat", time.Now().Add(10*time.Second).Unix(), "ex", 10).Err(), "syntax err")
+ require.ErrorContains(t, rdb.Do(ctx, "SET", "foo", "bar",
"exat", time.Now().Add(10*time.Second).Unix(), "pxat",
time.Now().Add(10*time.Second).UnixMilli()).Err(), "syntax err")
+
require.ErrorContains(t, rdb.Do(ctx, "SET", "foo", "bar", "NX",
"XX").Err(), "syntax err")
+ require.ErrorContains(t, rdb.Do(ctx, "SET", "foo", "bar", "XX",
"NX").Err(), "syntax err")
})
t.Run("Extended SET with incorrect expire value", func(t *testing.T) {
@@ -553,11 +656,33 @@ func TestString(t *testing.T) {
require.EqualValues(t, 1, rdb.Do(ctx, "CAS", "cas_key", "123",
"234", "ex", "1").Val())
require.Equal(t, "234", rdb.Get(ctx, "cas_key").Val())
- time.Sleep(2000 * time.Millisecond)
+ time.Sleep(2 * time.Second)
require.Equal(t, "", rdb.Get(ctx, "cas_key").Val())
})
+ t.Run("CAS expire Duplicate EX option", func(t *testing.T) {
+ require.NoError(t, rdb.Del(ctx, "cas_key").Err())
+ require.NoError(t, rdb.Set(ctx, "cas_key", "123", 0).Err())
+ require.EqualValues(t, 1, rdb.Do(ctx, "CAS", "cas_key", "123",
"234", "ex", 100, "ex", 10).Val())
+ util.BetweenValues(t, rdb.TTL(ctx, "cas_key").Val(),
5*time.Second, 10*time.Second)
+ })
+
+ t.Run("CAS expire Duplicate PX option", func(t *testing.T) {
+ require.NoError(t, rdb.Del(ctx, "cas_key").Err())
+ require.NoError(t, rdb.Set(ctx, "cas_key", "123", 0).Err())
+ require.EqualValues(t, 1, rdb.Do(ctx, "CAS", "cas_key", "123",
"234", "px", 1000, "px", 10000).Val())
+ util.BetweenValues(t, rdb.TTL(ctx, "cas_key").Val(),
5*time.Second, 10*time.Second)
+ })
+
+ t.Run("CAS expire PX option and EX option exist at the same time",
func(t *testing.T) {
+ require.NoError(t, rdb.Del(ctx, "cas_key").Err())
+ require.NoError(t, rdb.Set(ctx, "cas_key", "123", 0).Err())
+ require.ErrorContains(t, rdb.Do(ctx, "CAS", "cas_key", "123",
"234", "ex", 100, "px", 100000).Err(), "syntax error")
+ require.ErrorContains(t, rdb.Do(ctx, "CAS", "cas_key", "123",
"234", "ex", 100, "ex", 10, "px", 10000).Err(), "syntax error")
+ require.ErrorContains(t, rdb.Do(ctx, "CAS", "cas_key", "123",
"234", "px", 10000, "ex", 100, "ex", 10, "px", 10000).Err(), "syntax error")
+ })
+
t.Run("CAD normal case", func(t *testing.T) {
require.EqualValues(t, -1, rdb.Do(ctx, "CAD", "cad_key",
"123").Val())
require.NoError(t, rdb.Set(ctx, "cad_key", "123", 0).Err())
