From 444d5c6b949e57926c1ae7ca4509809b40697bd7 Mon Sep 17 00:00:00 2001 From: Sarthak Aggarwal Date: Tue, 19 Nov 2024 11:12:17 -0800 Subject: [PATCH 01/11] set with ifeq support Signed-off-by: Sarthak Aggarwal --- src/t_string.c | 43 +++++++++++++++++++++++++++++--------- tests/unit/type/string.tcl | 13 ++++++++++++ 2 files changed, 46 insertions(+), 10 deletions(-) diff --git a/src/t_string.c b/src/t_string.c index 1c90eabf3e..55a565f10b 100644 --- a/src/t_string.c +++ b/src/t_string.c @@ -76,6 +76,7 @@ static int checkStringLength(client *c, long long size, long long append) { #define OBJ_EXAT (1 << 6) /* Set if timestamp in second is given */ #define OBJ_PXAT (1 << 7) /* Set if timestamp in ms is given */ #define OBJ_PERSIST (1 << 8) /* Set if we need to remove the ttl */ +#define OBJ_SET_IFEQ (1 << 9) /* Set if we need compare and set */ /* Forward declaration */ static int getExpireMillisecondsOrReply(client *c, robj *expire, int flags, int unit, long long *milliseconds); @@ -87,7 +88,8 @@ void setGenericCommand(client *c, robj *expire, int unit, robj *ok_reply, - robj *abort_reply) { + robj *abort_reply, + robj *comparison) { long long milliseconds = 0; /* initialized to avoid any harmness warning */ int found = 0; int setkey_flags = 0; @@ -102,6 +104,15 @@ void setGenericCommand(client *c, found = (lookupKeyWrite(c->db, key) != NULL); + /* Handle the IFEQ conditional check */ + if ((flags & OBJ_SET_IFEQ) && found) { + robj *current_value = lookupKeyRead(c->db, key); + if (current_value == NULL || compareStringObjects(current_value, comparison) != 0) { + addReply(c, abort_reply ? abort_reply : shared.null[c->resp]); + return; + } + } + if ((flags & OBJ_SET_NX && found) || (flags & OBJ_SET_XX && !found)) { if (!(flags & OBJ_SET_GET)) { addReply(c, abort_reply ? abort_reply : shared.null[c->resp]); @@ -208,7 +219,7 @@ static int getExpireMillisecondsOrReply(client *c, robj *expire, int flags, int * string arguments used in SET and GET command. * * Get specific commands - PERSIST/DEL - * Set specific commands - XX/NX/GET + * Set specific commands - XX/NX/GET/IFEQ * Common commands - EX/EXAT/PX/PXAT/KEEPTTL * * Function takes pointers to client, flags, unit, pointer to pointer of expire obj if needed @@ -219,7 +230,7 @@ static int getExpireMillisecondsOrReply(client *c, robj *expire, int flags, int * Input flags are updated upon parsing the arguments. Unit and expire are updated if there are any * EX/EXAT/PX/PXAT arguments. Unit is updated to millisecond if PX/PXAT is set. */ -int parseExtendedStringArgumentsOrReply(client *c, int *flags, int *unit, robj **expire, int command_type) { +int parseExtendedStringArgumentsOrReply(client *c, int *flags, int *unit, robj **expire, robj **compare_val, int command_type) { int j = command_type == COMMAND_GET ? 2 : 3; for (; j < c->argc; j++) { char *opt = c->argv[j]->ptr; @@ -295,7 +306,17 @@ int parseExtendedStringArgumentsOrReply(client *c, int *flags, int *unit, robj * *unit = UNIT_MILLISECONDS; *expire = next; j++; - } else { + } else if ((opt[0] == 'i' || opt[0] == 'I') && + (opt[1] == 'f' || opt[1] == 'F') && + (opt[2] == 'e' || opt[2] == 'E') && + (opt[3] == 'q' || opt[3] == 'Q') && opt[4] == '\0' && + next && (command_type == COMMAND_SET)) + { + *flags |= OBJ_SET_IFEQ; + *compare_val = next; + j++; + } + else { addReplyErrorObject(c,shared.syntaxerr); return C_ERR; } @@ -308,30 +329,31 @@ int parseExtendedStringArgumentsOrReply(client *c, int *flags, int *unit, robj * * [EXAT ][PXAT ] */ void setCommand(client *c) { robj *expire = NULL; + robj *comparison = NULL; int unit = UNIT_SECONDS; int flags = OBJ_NO_FLAGS; - if (parseExtendedStringArgumentsOrReply(c, &flags, &unit, &expire, COMMAND_SET) != C_OK) { + if (parseExtendedStringArgumentsOrReply(c, &flags, &unit, &expire, &comparison, COMMAND_SET) != C_OK) { return; } c->argv[2] = tryObjectEncoding(c->argv[2]); - setGenericCommand(c, flags, c->argv[1], c->argv[2], expire, unit, NULL, NULL); + setGenericCommand(c, flags, c->argv[1], c->argv[2], expire, unit, NULL, NULL, comparison); } void setnxCommand(client *c) { c->argv[2] = tryObjectEncoding(c->argv[2]); - setGenericCommand(c, OBJ_SET_NX, c->argv[1], c->argv[2], NULL, 0, shared.cone, shared.czero); + setGenericCommand(c, OBJ_SET_NX, c->argv[1], c->argv[2], NULL, 0, shared.cone, shared.czero, NULL); } void setexCommand(client *c) { c->argv[3] = tryObjectEncoding(c->argv[3]); - setGenericCommand(c, OBJ_EX, c->argv[1], c->argv[3], c->argv[2], UNIT_SECONDS, NULL, NULL); + setGenericCommand(c, OBJ_EX, c->argv[1], c->argv[3], c->argv[2], UNIT_SECONDS, NULL, NULL, NULL); } void psetexCommand(client *c) { c->argv[3] = tryObjectEncoding(c->argv[3]); - setGenericCommand(c, OBJ_PX, c->argv[1], c->argv[3], c->argv[2], UNIT_MILLISECONDS, NULL, NULL); + setGenericCommand(c, OBJ_PX, c->argv[1], c->argv[3], c->argv[2], UNIT_MILLISECONDS, NULL, NULL, NULL); } int getGenericCommand(client *c) { @@ -374,10 +396,11 @@ void getCommand(client *c) { */ void getexCommand(client *c) { robj *expire = NULL; + robj *comparison = NULL; int unit = UNIT_SECONDS; int flags = OBJ_NO_FLAGS; - if (parseExtendedStringArgumentsOrReply(c, &flags, &unit, &expire, COMMAND_GET) != C_OK) { + if (parseExtendedStringArgumentsOrReply(c, &flags, &unit, &expire, &comparison, COMMAND_GET) != C_OK) { return; } diff --git a/tests/unit/type/string.tcl b/tests/unit/type/string.tcl index d7969b5b3e..31e88674d7 100644 --- a/tests/unit/type/string.tcl +++ b/tests/unit/type/string.tcl @@ -582,6 +582,19 @@ if {[string match {*jemalloc*} [s mem_allocator]]} { set err1 } {*WRONGTYPE*} + test "SET with IFEQ conditional" { + # Setting an initial value for the key + r set foo "initial_value" + + # Trying to set the key only if the value is exactly "initial_value" + assert_equal OK [r set foo "new_value" ifeq "initial_value"] + assert_equal "new_value" [r get foo] + + # Trying to set the key only if the value is NOT "initial_value" + assert_equal {} [r set foo "should_not_set" ifeq "wrong_value"] + assert_equal "new_value" [r get foo] + } + test {Extended SET EX option} { r del foo r set foo bar ex 10 From 9da3598d700283e884ec96e5c21171c08bd3ce2f Mon Sep 17 00:00:00 2001 From: Sarthak Aggarwal Date: Tue, 19 Nov 2024 19:18:18 -0800 Subject: [PATCH 02/11] addressing comments and adding tests Signed-off-by: Sarthak Aggarwal --- src/commands.def | 3 ++- src/commands/set.json | 13 +++++++++++ src/t_string.c | 22 ++++++++++++++--- tests/unit/type/string.tcl | 48 ++++++++++++++++++++++++++++++++++---- 4 files changed, 78 insertions(+), 8 deletions(-) diff --git a/src/commands.def b/src/commands.def index ecc77126af..dfee996144 100644 --- a/src/commands.def +++ b/src/commands.def @@ -10666,6 +10666,7 @@ struct COMMAND_ARG SET_expiration_Subargs[] = { struct COMMAND_ARG SET_Args[] = { {MAKE_ARG("key",ARG_TYPE_KEY,0,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)}, {MAKE_ARG("value",ARG_TYPE_STRING,-1,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)}, +{MAKE_ARG("ifeq",ARG_TYPE_STRING,-1,NULL,"Sets the key's value only if the current value matches the specified comparison value.",NULL,CMD_ARG_OPTIONAL,0,NULL)}, {MAKE_ARG("condition",ARG_TYPE_ONEOF,-1,NULL,NULL,"2.6.12",CMD_ARG_OPTIONAL,2,NULL),.subargs=SET_condition_Subargs}, {MAKE_ARG("get",ARG_TYPE_PURE_TOKEN,-1,"GET",NULL,"6.2.0",CMD_ARG_OPTIONAL,0,NULL)}, {MAKE_ARG("expiration",ARG_TYPE_ONEOF,-1,NULL,NULL,NULL,CMD_ARG_OPTIONAL,5,NULL),.subargs=SET_expiration_Subargs}, @@ -11139,7 +11140,7 @@ struct COMMAND_STRUCT serverCommandTable[] = { {MAKE_CMD("mset","Atomically creates or modifies the string values of one or more keys.","O(N) where N is the number of keys to set.","1.0.1",CMD_DOC_NONE,NULL,NULL,"string",COMMAND_GROUP_STRING,MSET_History,0,MSET_Tips,2,msetCommand,-3,CMD_WRITE|CMD_DENYOOM,ACL_CATEGORY_STRING,MSET_Keyspecs,1,NULL,1),.args=MSET_Args}, {MAKE_CMD("msetnx","Atomically modifies the string values of one or more keys only when all keys don't exist.","O(N) where N is the number of keys to set.","1.0.1",CMD_DOC_NONE,NULL,NULL,"string",COMMAND_GROUP_STRING,MSETNX_History,0,MSETNX_Tips,0,msetnxCommand,-3,CMD_WRITE|CMD_DENYOOM,ACL_CATEGORY_STRING,MSETNX_Keyspecs,1,NULL,1),.args=MSETNX_Args}, {MAKE_CMD("psetex","Sets both string value and expiration time in milliseconds of a key. The key is created if it doesn't exist.","O(1)","2.6.0",CMD_DOC_DEPRECATED,"`SET` with the `PX` argument","2.6.12","string",COMMAND_GROUP_STRING,PSETEX_History,0,PSETEX_Tips,0,psetexCommand,4,CMD_WRITE|CMD_DENYOOM,ACL_CATEGORY_STRING,PSETEX_Keyspecs,1,NULL,3),.args=PSETEX_Args}, -{MAKE_CMD("set","Sets the string value of a key, ignoring its type. The key is created if it doesn't exist.","O(1)","1.0.0",CMD_DOC_NONE,NULL,NULL,"string",COMMAND_GROUP_STRING,SET_History,4,SET_Tips,0,setCommand,-3,CMD_WRITE|CMD_DENYOOM,ACL_CATEGORY_STRING,SET_Keyspecs,1,setGetKeys,5),.args=SET_Args}, +{MAKE_CMD("set","Sets the string value of a key, ignoring its type. The key is created if it doesn't exist.","O(1)","1.0.0",CMD_DOC_NONE,NULL,NULL,"string",COMMAND_GROUP_STRING,SET_History,4,SET_Tips,0,setCommand,-3,CMD_WRITE|CMD_DENYOOM,ACL_CATEGORY_STRING,SET_Keyspecs,1,setGetKeys,6),.args=SET_Args}, {MAKE_CMD("setex","Sets the string value and expiration time of a key. Creates the key if it doesn't exist.","O(1)","2.0.0",CMD_DOC_DEPRECATED,"`SET` with the `EX` argument","2.6.12","string",COMMAND_GROUP_STRING,SETEX_History,0,SETEX_Tips,0,setexCommand,4,CMD_WRITE|CMD_DENYOOM,ACL_CATEGORY_STRING,SETEX_Keyspecs,1,NULL,3),.args=SETEX_Args}, {MAKE_CMD("setnx","Set the string value of a key only when the key doesn't exist.","O(1)","1.0.0",CMD_DOC_DEPRECATED,"`SET` with the `NX` argument","2.6.12","string",COMMAND_GROUP_STRING,SETNX_History,0,SETNX_Tips,0,setnxCommand,3,CMD_WRITE|CMD_DENYOOM|CMD_FAST,ACL_CATEGORY_STRING,SETNX_Keyspecs,1,NULL,2),.args=SETNX_Args}, {MAKE_CMD("setrange","Overwrites a part of a string value with another by an offset. Creates the key if it doesn't exist.","O(1), not counting the time taken to copy the new string in place. Usually, this string is very small so the amortized complexity is O(1). Otherwise, complexity is O(M) with M being the length of the value argument.","2.2.0",CMD_DOC_NONE,NULL,NULL,"string",COMMAND_GROUP_STRING,SETRANGE_History,0,SETRANGE_Tips,0,setrangeCommand,4,CMD_WRITE|CMD_DENYOOM,ACL_CATEGORY_STRING,SETRANGE_Keyspecs,1,NULL,3),.args=SETRANGE_Args}, diff --git a/src/commands/set.json b/src/commands/set.json index 8236bc7bb9..169ee2fbf5 100644 --- a/src/commands/set.json +++ b/src/commands/set.json @@ -85,6 +85,19 @@ "name": "value", "type": "string" }, + { + "name": "ifeq", + "type": "string", + "optional": true, + "summary": "Sets the key's value only if the current value matches the specified comparison value.", + "arguments": [ + { + "name": "comparison-value", + "type": "string", + "summary": "The value to compare with the current key's value before setting." + } + ] + }, { "name": "condition", "type": "oneof", diff --git a/src/t_string.c b/src/t_string.c index 55a565f10b..f704c6b396 100644 --- a/src/t_string.c +++ b/src/t_string.c @@ -105,12 +105,28 @@ void setGenericCommand(client *c, found = (lookupKeyWrite(c->db, key) != NULL); /* Handle the IFEQ conditional check */ - if ((flags & OBJ_SET_IFEQ) && found) { + if (flags & OBJ_SET_IFEQ && found) { robj *current_value = lookupKeyRead(c->db, key); - if (current_value == NULL || compareStringObjects(current_value, comparison) != 0) { - addReply(c, abort_reply ? abort_reply : shared.null[c->resp]); + + if (current_value == NULL || current_value->type != OBJ_STRING || checkType(c, comparison, OBJ_STRING) != 0) { + if (!(flags & OBJ_SET_GET)) { + addReplyError(c, "value(s) must be present or string"); + } + return; + } + + if (compareStringObjects(current_value, comparison) != 0) { + if (!(flags & OBJ_SET_GET)) { + addReply(c, abort_reply ? abort_reply : shared.null[c->resp]); + } return; } + + } else if (flags & OBJ_SET_IFEQ && !found) { + if (!(flags & OBJ_SET_GET)) { + addReply(c, abort_reply ? abort_reply : shared.null[c->resp]); + } + return; } if ((flags & OBJ_SET_NX && found) || (flags & OBJ_SET_XX && !found)) { diff --git a/tests/unit/type/string.tcl b/tests/unit/type/string.tcl index 31e88674d7..7d950e6533 100644 --- a/tests/unit/type/string.tcl +++ b/tests/unit/type/string.tcl @@ -583,18 +583,58 @@ if {[string match {*jemalloc*} [s mem_allocator]]} { } {*WRONGTYPE*} test "SET with IFEQ conditional" { - # Setting an initial value for the key + r del foo + r set foo "initial_value" - # Trying to set the key only if the value is exactly "initial_value" - assert_equal OK [r set foo "new_value" ifeq "initial_value"] + assert_equal {OK} [r set foo "new_value" ifeq "initial_value"] assert_equal "new_value" [r get foo] - # Trying to set the key only if the value is NOT "initial_value" assert_equal {} [r set foo "should_not_set" ifeq "wrong_value"] assert_equal "new_value" [r get foo] } + test "SET with IFEQ conditional - non-string current value" { + r del foo + + r sadd foo "some_set_value" + assert_error {ERR value(s) must be present or string} {r set foo "new_value" ifeq "some_set_value"} + } + + test "SET with IFEQ conditional - with get" { + r del foo + + assert_equal {} [r set foo "new_value" ifeq "initial_value" get] + + r set foo "initial_value" + + assert_equal "initial_value" [r set foo "new_value" ifeq "initial_value" get] + assert_equal "new_value" [r get foo] + } + + + test "SET with IFEQ conditional - with xx" { + r del foo + + assert_error {} {r set foo "new_value" ifeq "initial_value" xx} + + r set foo "initial_value" + + assert_equal {OK} [r set foo "new_value" ifeq "initial_value" xx] + assert_equal "new_value" [r get foo] + } + + test "SET with IFEQ conditional - with nx" { + r del foo + + assert_error {ERR value(s) must be present or string} {r set foo "new_value" ifeq "initial_value" nx} + + r set foo "initial_value" + + assert_equal {} [r set foo "new_value" ifeq "initial_value" nx] + assert_equal "initial_value" [r get foo] + } + test {Extended SET EX option} { r del foo r set foo bar ex 10 From 294d1ade3f60390125072180f96ef4a80ec0502a Mon Sep 17 00:00:00 2001 From: Sarthak Aggarwal Date: Tue, 19 Nov 2024 21:59:02 -0800 Subject: [PATCH 03/11] fixes Signed-off-by: Sarthak Aggarwal --- src/commands.def | 2 +- src/commands/set.json | 3 ++- src/t_string.c | 4 ++-- tests/assets/test_cli_hint_suite.txt | 24 ++++++++++++------------ tests/unit/type/string.tcl | 4 ++-- 5 files changed, 19 insertions(+), 18 deletions(-) diff --git a/src/commands.def b/src/commands.def index dfee996144..84fe1ec42f 100644 --- a/src/commands.def +++ b/src/commands.def @@ -10666,7 +10666,7 @@ struct COMMAND_ARG SET_expiration_Subargs[] = { struct COMMAND_ARG SET_Args[] = { {MAKE_ARG("key",ARG_TYPE_KEY,0,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)}, {MAKE_ARG("value",ARG_TYPE_STRING,-1,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)}, -{MAKE_ARG("ifeq",ARG_TYPE_STRING,-1,NULL,"Sets the key's value only if the current value matches the specified comparison value.",NULL,CMD_ARG_OPTIONAL,0,NULL)}, +{MAKE_ARG("comparison-value",ARG_TYPE_STRING,-1,"IFEQ","Sets the key's value only if the current value matches the specified comparison value.",NULL,CMD_ARG_OPTIONAL,0,NULL)}, {MAKE_ARG("condition",ARG_TYPE_ONEOF,-1,NULL,NULL,"2.6.12",CMD_ARG_OPTIONAL,2,NULL),.subargs=SET_condition_Subargs}, {MAKE_ARG("get",ARG_TYPE_PURE_TOKEN,-1,"GET",NULL,"6.2.0",CMD_ARG_OPTIONAL,0,NULL)}, {MAKE_ARG("expiration",ARG_TYPE_ONEOF,-1,NULL,NULL,NULL,CMD_ARG_OPTIONAL,5,NULL),.subargs=SET_expiration_Subargs}, diff --git a/src/commands/set.json b/src/commands/set.json index 169ee2fbf5..2af77fe048 100644 --- a/src/commands/set.json +++ b/src/commands/set.json @@ -86,8 +86,9 @@ "type": "string" }, { - "name": "ifeq", + "name": "comparison-value", "type": "string", + "token": "IFEQ", "optional": true, "summary": "Sets the key's value only if the current value matches the specified comparison value.", "arguments": [ diff --git a/src/t_string.c b/src/t_string.c index f704c6b396..a92f25aeae 100644 --- a/src/t_string.c +++ b/src/t_string.c @@ -341,8 +341,8 @@ int parseExtendedStringArgumentsOrReply(client *c, int *flags, int *unit, robj * return C_OK; } -/* SET key value [NX] [XX] [KEEPTTL] [GET] [EX ] [PX ] - * [EXAT ][PXAT ] */ +/* SET key value [IFEQ ] [NX] [XX] [KEEPTTL] [GET] [EX ] + * [PX ] [EXAT ][PXAT ] */ void setCommand(client *c) { robj *expire = NULL; robj *comparison = NULL; diff --git a/tests/assets/test_cli_hint_suite.txt b/tests/assets/test_cli_hint_suite.txt index 3cebf5229c..a37fe21979 100644 --- a/tests/assets/test_cli_hint_suite.txt +++ b/tests/assets/test_cli_hint_suite.txt @@ -68,18 +68,18 @@ "ZRANGE k 1 2 WITHSCORES " "[BYSCORE|BYLEX] [REV] [LIMIT offset count]" # Optional one-of args with parameters: SET key value [NX|XX] [GET] [EX seconds|PX milliseconds|EXAT unix-time-seconds|PXAT unix-time-milliseconds|KEEPTTL] -"SET key value " "[NX|XX] [GET] [EX seconds|PX milliseconds|EXAT unix-time-seconds|PXAT unix-time-milliseconds|KEEPTTL]" -"SET key value EX" "[NX|XX] [GET] [EX seconds|PX milliseconds|EXAT unix-time-seconds|PXAT unix-time-milliseconds|KEEPTTL]" -"SET key value EX " "seconds [NX|XX] [GET]" -"SET key value EX 23 " "[NX|XX] [GET]" -"SET key value EXAT" "[NX|XX] [GET] [EX seconds|PX milliseconds|EXAT unix-time-seconds|PXAT unix-time-milliseconds|KEEPTTL]" -"SET key value EXAT " "unix-time-seconds [NX|XX] [GET]" -"SET key value PX" "[NX|XX] [GET] [EX seconds|PX milliseconds|EXAT unix-time-seconds|PXAT unix-time-milliseconds|KEEPTTL]" -"SET key value PX " "milliseconds [NX|XX] [GET]" -"SET key value PXAT" "[NX|XX] [GET] [EX seconds|PX milliseconds|EXAT unix-time-seconds|PXAT unix-time-milliseconds|KEEPTTL]" -"SET key value PXAT " "unix-time-milliseconds [NX|XX] [GET]" -"SET key value KEEPTTL " "[NX|XX] [GET]" -"SET key value XX " "[GET] [EX seconds|PX milliseconds|EXAT unix-time-seconds|PXAT unix-time-milliseconds|KEEPTTL]" +"SET key value " " [IFEQ comparison-value] [NX|XX] [GET] [EX seconds|PX milliseconds|EXAT unix-time-seconds|PXAT unix-time-milliseconds|KEEPTTL]" +"SET key value EX" "[IFEQ comparison-value] [NX|XX] [GET] [EX seconds|PX milliseconds|EXAT unix-time-seconds|PXAT unix-time-milliseconds|KEEPTTL]" +"SET key value EX " "seconds [IFEQ comparison-value] [NX|XX] [GET]" +"SET key value EX 23 " "[IFEQ comparison-value] [NX|XX] [GET]" +"SET key value EXAT" "[IFEQ comparison-value] [NX|XX] [GET] [EX seconds|PX milliseconds|EXAT unix-time-seconds|PXAT unix-time-milliseconds|KEEPTTL]" +"SET key value EXAT " "unix-time-seconds [IFEQ comparison-value] [NX|XX] [GET]" +"SET key value PX" "[IFEQ comparison-value] [NX|XX] [GET] [EX seconds|PX milliseconds|EXAT unix-time-seconds|PXAT unix-time-milliseconds|KEEPTTL]" +"SET key value PX " "milliseconds [IFEQ comparison-value] [NX|XX] [GET]" +"SET key value PXAT" "[IFEQ comparison-value] [NX|XX] [GET] [EX seconds|PX milliseconds|EXAT unix-time-seconds|PXAT unix-time-milliseconds|KEEPTTL]" +"SET key value PXAT " "unix-time-milliseconds [IFEQ comparison-value] [NX|XX] [GET]" +"SET key value KEEPTTL " "[IFEQ comparison-value] [NX|XX] [GET]" +"SET key value XX " "[IFEQ comparison-value] [GET] [EX seconds|PX milliseconds|EXAT unix-time-seconds|PXAT unix-time-milliseconds|KEEPTTL]" # If an input word can't be matched, stop hinting. "SET key value FOOBAR " "" diff --git a/tests/unit/type/string.tcl b/tests/unit/type/string.tcl index 7d950e6533..849abc9f63 100644 --- a/tests/unit/type/string.tcl +++ b/tests/unit/type/string.tcl @@ -616,7 +616,7 @@ if {[string match {*jemalloc*} [s mem_allocator]]} { test "SET with IFEQ conditional - with xx" { r del foo - assert_error {} {r set foo "new_value" ifeq "initial_value" xx} + assert_equal {} [r set foo "new_value" ifeq "initial_value" xx] r set foo "initial_value" @@ -627,7 +627,7 @@ if {[string match {*jemalloc*} [s mem_allocator]]} { test "SET with IFEQ conditional - with nx" { r del foo - assert_error {ERR value(s) must be present or string} {r set foo "new_value" ifeq "initial_value" nx} + assert_equal {} [r set foo "new_value" ifeq "initial_value" nx] r set foo "initial_value" From a7e5e40100bbc864fa175846f3da311969330ad8 Mon Sep 17 00:00:00 2001 From: Sarthak Aggarwal Date: Tue, 19 Nov 2024 22:01:10 -0800 Subject: [PATCH 04/11] addressing comments Signed-off-by: Sarthak Aggarwal --- src/t_string.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/t_string.c b/src/t_string.c index a92f25aeae..730ba06447 100644 --- a/src/t_string.c +++ b/src/t_string.c @@ -108,7 +108,7 @@ void setGenericCommand(client *c, if (flags & OBJ_SET_IFEQ && found) { robj *current_value = lookupKeyRead(c->db, key); - if (current_value == NULL || current_value->type != OBJ_STRING || checkType(c, comparison, OBJ_STRING) != 0) { + if (checkType(c, current_value, OBJ_STRING) != 0) { if (!(flags & OBJ_SET_GET)) { addReplyError(c, "value(s) must be present or string"); } From 1366e85f8d7501c9501b45ed7a957e308ffe4520 Mon Sep 17 00:00:00 2001 From: Sarthak Aggarwal Date: Wed, 20 Nov 2024 10:06:56 -0800 Subject: [PATCH 05/11] addressing tests Signed-off-by: Sarthak Aggarwal --- src/t_string.c | 2 +- tests/assets/test_cli_hint_suite.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/t_string.c b/src/t_string.c index 730ba06447..1b532c579f 100644 --- a/src/t_string.c +++ b/src/t_string.c @@ -108,7 +108,7 @@ void setGenericCommand(client *c, if (flags & OBJ_SET_IFEQ && found) { robj *current_value = lookupKeyRead(c->db, key); - if (checkType(c, current_value, OBJ_STRING) != 0) { + if (current_value->type != OBJ_STRING) { if (!(flags & OBJ_SET_GET)) { addReplyError(c, "value(s) must be present or string"); } diff --git a/tests/assets/test_cli_hint_suite.txt b/tests/assets/test_cli_hint_suite.txt index a37fe21979..6e5e6c0303 100644 --- a/tests/assets/test_cli_hint_suite.txt +++ b/tests/assets/test_cli_hint_suite.txt @@ -68,7 +68,7 @@ "ZRANGE k 1 2 WITHSCORES " "[BYSCORE|BYLEX] [REV] [LIMIT offset count]" # Optional one-of args with parameters: SET key value [NX|XX] [GET] [EX seconds|PX milliseconds|EXAT unix-time-seconds|PXAT unix-time-milliseconds|KEEPTTL] -"SET key value " " [IFEQ comparison-value] [NX|XX] [GET] [EX seconds|PX milliseconds|EXAT unix-time-seconds|PXAT unix-time-milliseconds|KEEPTTL]" +"SET key value " "[IFEQ comparison-value] [NX|XX] [GET] [EX seconds|PX milliseconds|EXAT unix-time-seconds|PXAT unix-time-milliseconds|KEEPTTL]" "SET key value EX" "[IFEQ comparison-value] [NX|XX] [GET] [EX seconds|PX milliseconds|EXAT unix-time-seconds|PXAT unix-time-milliseconds|KEEPTTL]" "SET key value EX " "seconds [IFEQ comparison-value] [NX|XX] [GET]" "SET key value EX 23 " "[IFEQ comparison-value] [NX|XX] [GET]" From 2b3b00e133660631ac8a64f44937eca1f03f391f Mon Sep 17 00:00:00 2001 From: Sarthak Aggarwal Date: Wed, 20 Nov 2024 10:15:44 -0800 Subject: [PATCH 06/11] clang check Signed-off-by: Sarthak Aggarwal --- src/t_string.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/t_string.c b/src/t_string.c index 1b532c579f..161565e47b 100644 --- a/src/t_string.c +++ b/src/t_string.c @@ -67,15 +67,15 @@ static int checkStringLength(client *c, long long size, long long append) { * If abort_reply is NULL, "$-1" is used. */ #define OBJ_NO_FLAGS 0 -#define OBJ_SET_NX (1 << 0) /* Set if key not exists. */ -#define OBJ_SET_XX (1 << 1) /* Set if key exists. */ -#define OBJ_EX (1 << 2) /* Set if time in seconds is given */ -#define OBJ_PX (1 << 3) /* Set if time in ms in given */ -#define OBJ_KEEPTTL (1 << 4) /* Set and keep the ttl */ -#define OBJ_SET_GET (1 << 5) /* Set if want to get key before set */ -#define OBJ_EXAT (1 << 6) /* Set if timestamp in second is given */ -#define OBJ_PXAT (1 << 7) /* Set if timestamp in ms is given */ -#define OBJ_PERSIST (1 << 8) /* Set if we need to remove the ttl */ +#define OBJ_SET_NX (1 << 0) /* Set if key not exists. */ +#define OBJ_SET_XX (1 << 1) /* Set if key exists. */ +#define OBJ_EX (1 << 2) /* Set if time in seconds is given */ +#define OBJ_PX (1 << 3) /* Set if time in ms in given */ +#define OBJ_KEEPTTL (1 << 4) /* Set and keep the ttl */ +#define OBJ_SET_GET (1 << 5) /* Set if want to get key before set */ +#define OBJ_EXAT (1 << 6) /* Set if timestamp in second is given */ +#define OBJ_PXAT (1 << 7) /* Set if timestamp in ms is given */ +#define OBJ_PERSIST (1 << 8) /* Set if we need to remove the ttl */ #define OBJ_SET_IFEQ (1 << 9) /* Set if we need compare and set */ /* Forward declaration */ From 7e693f1de960f18f9328baa103d9e5c6f6a3515e Mon Sep 17 00:00:00 2001 From: Sarthak Aggarwal Date: Wed, 20 Nov 2024 12:38:59 -0800 Subject: [PATCH 07/11] remove dedup lookup Signed-off-by: Sarthak Aggarwal --- src/t_string.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/t_string.c b/src/t_string.c index 161565e47b..f194add734 100644 --- a/src/t_string.c +++ b/src/t_string.c @@ -102,20 +102,20 @@ void setGenericCommand(client *c, if (getGenericCommand(c) == C_ERR) return; } - found = (lookupKeyWrite(c->db, key) != NULL); + const robj *value = lookupKeyWrite(c->db, key); + found = value != NULL; /* Handle the IFEQ conditional check */ if (flags & OBJ_SET_IFEQ && found) { - robj *current_value = lookupKeyRead(c->db, key); - if (current_value->type != OBJ_STRING) { + if (value->type != OBJ_STRING) { if (!(flags & OBJ_SET_GET)) { addReplyError(c, "value(s) must be present or string"); } return; } - if (compareStringObjects(current_value, comparison) != 0) { + if (compareStringObjects(value, comparison) != 0) { if (!(flags & OBJ_SET_GET)) { addReply(c, abort_reply ? abort_reply : shared.null[c->resp]); } From d5ec915613aab187f02ef08434be9b091f22b922 Mon Sep 17 00:00:00 2001 From: Sarthak Aggarwal Date: Wed, 20 Nov 2024 14:23:53 -0800 Subject: [PATCH 08/11] addressing comments Signed-off-by: Sarthak Aggarwal --- src/t_string.c | 13 ++++--------- tests/unit/type/string.tcl | 10 +++++++++- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/t_string.c b/src/t_string.c index f194add734..5adc38980d 100644 --- a/src/t_string.c +++ b/src/t_string.c @@ -102,26 +102,21 @@ void setGenericCommand(client *c, if (getGenericCommand(c) == C_ERR) return; } - const robj *value = lookupKeyWrite(c->db, key); - found = value != NULL; + robj *existing_value = lookupKeyWrite(c->db, key); + found = existing_value != NULL; /* Handle the IFEQ conditional check */ if (flags & OBJ_SET_IFEQ && found) { - - if (value->type != OBJ_STRING) { - if (!(flags & OBJ_SET_GET)) { - addReplyError(c, "value(s) must be present or string"); - } + if (!(flags & OBJ_SET_GET) && checkType(c, existing_value, OBJ_STRING)) { return; } - if (compareStringObjects(value, comparison) != 0) { + if (compareStringObjects(existing_value, comparison) != 0) { if (!(flags & OBJ_SET_GET)) { addReply(c, abort_reply ? abort_reply : shared.null[c->resp]); } return; } - } else if (flags & OBJ_SET_IFEQ && !found) { if (!(flags & OBJ_SET_GET)) { addReply(c, abort_reply ? abort_reply : shared.null[c->resp]); diff --git a/tests/unit/type/string.tcl b/tests/unit/type/string.tcl index 849abc9f63..751e86e7de 100644 --- a/tests/unit/type/string.tcl +++ b/tests/unit/type/string.tcl @@ -598,9 +598,10 @@ if {[string match {*jemalloc*} [s mem_allocator]]} { r del foo r sadd foo "some_set_value" - assert_error {ERR value(s) must be present or string} {r set foo "new_value" ifeq "some_set_value"} + assert_error {WRONGTYPE Operation against a key holding the wrong kind of value} {r set foo "new_value" ifeq "some_set_value"} } + test "SET with IFEQ conditional - with get" { r del foo @@ -612,6 +613,13 @@ if {[string match {*jemalloc*} [s mem_allocator]]} { assert_equal "new_value" [r get foo] } + test "SET with IFEQ conditional - non string current value with get" { + r del foo + + r sadd foo "some_set_value" + + assert_error {WRONGTYPE Operation against a key holding the wrong kind of value} {r set foo "new_value" ifeq "initial_value" get} + } test "SET with IFEQ conditional - with xx" { r del foo From e3b477712b617c07b6a0f3517770025b14267e44 Mon Sep 17 00:00:00 2001 From: Sarthak Aggarwal Date: Mon, 25 Nov 2024 13:54:22 -0800 Subject: [PATCH 09/11] addressing comments Signed-off-by: Sarthak Aggarwal --- src/commands/set.json | 5 +++++ src/t_string.c | 4 ++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/commands/set.json b/src/commands/set.json index 2af77fe048..d6bf28d191 100644 --- a/src/commands/set.json +++ b/src/commands/set.json @@ -23,6 +23,10 @@ [ "7.0.0", "Allowed the `NX` and `GET` options to be used together." + ], + [ + "8.2.0", + "Added the `IFEQ` option." ] ], "command_flags": [ @@ -89,6 +93,7 @@ "name": "comparison-value", "type": "string", "token": "IFEQ", + "since": "8.2.0", "optional": true, "summary": "Sets the key's value only if the current value matches the specified comparison value.", "arguments": [ diff --git a/src/t_string.c b/src/t_string.c index 5adc38980d..7e83baa43b 100644 --- a/src/t_string.c +++ b/src/t_string.c @@ -336,8 +336,8 @@ int parseExtendedStringArgumentsOrReply(client *c, int *flags, int *unit, robj * return C_OK; } -/* SET key value [IFEQ ] [NX] [XX] [KEEPTTL] [GET] [EX ] - * [PX ] [EXAT ][PXAT ] */ +/* SET key value [NX] [XX] [KEEPTTL] [GET] [EX ] [PX ] + * [EXAT ][PXAT ] [IFEQ ] */ void setCommand(client *c) { robj *expire = NULL; robj *comparison = NULL; From 66ad52df9bc1552169eeafb918ab63d14f633673 Mon Sep 17 00:00:00 2001 From: Sarthak Aggarwal Date: Tue, 3 Dec 2024 19:13:52 -0800 Subject: [PATCH 10/11] addressing comments Signed-off-by: Sarthak Aggarwal --- src/commands.def | 11 ++++---- src/commands/set.json | 39 ++++++++++++++-------------- src/t_string.c | 5 ++-- tests/assets/test_cli_hint_suite.txt | 24 ++++++++--------- 4 files changed, 40 insertions(+), 39 deletions(-) diff --git a/src/commands.def b/src/commands.def index 84fe1ec42f..dfb9ec1f68 100644 --- a/src/commands.def +++ b/src/commands.def @@ -10632,6 +10632,7 @@ commandHistory SET_History[] = { {"6.0.0","Added the `KEEPTTL` option."}, {"6.2.0","Added the `GET`, `EXAT` and `PXAT` option."}, {"7.0.0","Allowed the `NX` and `GET` options to be used together."}, +{"8.1.0","Added the `IFEQ` option."}, }; #endif @@ -10649,8 +10650,9 @@ keySpec SET_Keyspecs[1] = { /* SET condition argument table */ struct COMMAND_ARG SET_condition_Subargs[] = { -{MAKE_ARG("nx",ARG_TYPE_PURE_TOKEN,-1,"NX",NULL,NULL,CMD_ARG_NONE,0,NULL)}, -{MAKE_ARG("xx",ARG_TYPE_PURE_TOKEN,-1,"XX",NULL,NULL,CMD_ARG_NONE,0,NULL)}, +{MAKE_ARG("nx",ARG_TYPE_PURE_TOKEN,-1,"NX",NULL,"2.6.12",CMD_ARG_NONE,0,NULL)}, +{MAKE_ARG("xx",ARG_TYPE_PURE_TOKEN,-1,"XX",NULL,"2.6.12",CMD_ARG_NONE,0,NULL)}, +{MAKE_ARG("comparison-value",ARG_TYPE_STRING,-1,"IFEQ","Sets the key's value only if the current value matches the specified comparison value.","8.1.0",CMD_ARG_OPTIONAL,0,NULL)}, }; /* SET expiration argument table */ @@ -10666,8 +10668,7 @@ struct COMMAND_ARG SET_expiration_Subargs[] = { struct COMMAND_ARG SET_Args[] = { {MAKE_ARG("key",ARG_TYPE_KEY,0,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)}, {MAKE_ARG("value",ARG_TYPE_STRING,-1,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)}, -{MAKE_ARG("comparison-value",ARG_TYPE_STRING,-1,"IFEQ","Sets the key's value only if the current value matches the specified comparison value.",NULL,CMD_ARG_OPTIONAL,0,NULL)}, -{MAKE_ARG("condition",ARG_TYPE_ONEOF,-1,NULL,NULL,"2.6.12",CMD_ARG_OPTIONAL,2,NULL),.subargs=SET_condition_Subargs}, +{MAKE_ARG("condition",ARG_TYPE_ONEOF,-1,NULL,NULL,NULL,CMD_ARG_OPTIONAL,3,NULL),.subargs=SET_condition_Subargs}, {MAKE_ARG("get",ARG_TYPE_PURE_TOKEN,-1,"GET",NULL,"6.2.0",CMD_ARG_OPTIONAL,0,NULL)}, {MAKE_ARG("expiration",ARG_TYPE_ONEOF,-1,NULL,NULL,NULL,CMD_ARG_OPTIONAL,5,NULL),.subargs=SET_expiration_Subargs}, }; @@ -11140,7 +11141,7 @@ struct COMMAND_STRUCT serverCommandTable[] = { {MAKE_CMD("mset","Atomically creates or modifies the string values of one or more keys.","O(N) where N is the number of keys to set.","1.0.1",CMD_DOC_NONE,NULL,NULL,"string",COMMAND_GROUP_STRING,MSET_History,0,MSET_Tips,2,msetCommand,-3,CMD_WRITE|CMD_DENYOOM,ACL_CATEGORY_STRING,MSET_Keyspecs,1,NULL,1),.args=MSET_Args}, {MAKE_CMD("msetnx","Atomically modifies the string values of one or more keys only when all keys don't exist.","O(N) where N is the number of keys to set.","1.0.1",CMD_DOC_NONE,NULL,NULL,"string",COMMAND_GROUP_STRING,MSETNX_History,0,MSETNX_Tips,0,msetnxCommand,-3,CMD_WRITE|CMD_DENYOOM,ACL_CATEGORY_STRING,MSETNX_Keyspecs,1,NULL,1),.args=MSETNX_Args}, {MAKE_CMD("psetex","Sets both string value and expiration time in milliseconds of a key. The key is created if it doesn't exist.","O(1)","2.6.0",CMD_DOC_DEPRECATED,"`SET` with the `PX` argument","2.6.12","string",COMMAND_GROUP_STRING,PSETEX_History,0,PSETEX_Tips,0,psetexCommand,4,CMD_WRITE|CMD_DENYOOM,ACL_CATEGORY_STRING,PSETEX_Keyspecs,1,NULL,3),.args=PSETEX_Args}, -{MAKE_CMD("set","Sets the string value of a key, ignoring its type. The key is created if it doesn't exist.","O(1)","1.0.0",CMD_DOC_NONE,NULL,NULL,"string",COMMAND_GROUP_STRING,SET_History,4,SET_Tips,0,setCommand,-3,CMD_WRITE|CMD_DENYOOM,ACL_CATEGORY_STRING,SET_Keyspecs,1,setGetKeys,6),.args=SET_Args}, +{MAKE_CMD("set","Sets the string value of a key, ignoring its type. The key is created if it doesn't exist.","O(1)","1.0.0",CMD_DOC_NONE,NULL,NULL,"string",COMMAND_GROUP_STRING,SET_History,5,SET_Tips,0,setCommand,-3,CMD_WRITE|CMD_DENYOOM,ACL_CATEGORY_STRING,SET_Keyspecs,1,setGetKeys,5),.args=SET_Args}, {MAKE_CMD("setex","Sets the string value and expiration time of a key. Creates the key if it doesn't exist.","O(1)","2.0.0",CMD_DOC_DEPRECATED,"`SET` with the `EX` argument","2.6.12","string",COMMAND_GROUP_STRING,SETEX_History,0,SETEX_Tips,0,setexCommand,4,CMD_WRITE|CMD_DENYOOM,ACL_CATEGORY_STRING,SETEX_Keyspecs,1,NULL,3),.args=SETEX_Args}, {MAKE_CMD("setnx","Set the string value of a key only when the key doesn't exist.","O(1)","1.0.0",CMD_DOC_DEPRECATED,"`SET` with the `NX` argument","2.6.12","string",COMMAND_GROUP_STRING,SETNX_History,0,SETNX_Tips,0,setnxCommand,3,CMD_WRITE|CMD_DENYOOM|CMD_FAST,ACL_CATEGORY_STRING,SETNX_Keyspecs,1,NULL,2),.args=SETNX_Args}, {MAKE_CMD("setrange","Overwrites a part of a string value with another by an offset. Creates the key if it doesn't exist.","O(1), not counting the time taken to copy the new string in place. Usually, this string is very small so the amortized complexity is O(1). Otherwise, complexity is O(M) with M being the length of the value argument.","2.2.0",CMD_DOC_NONE,NULL,NULL,"string",COMMAND_GROUP_STRING,SETRANGE_History,0,SETRANGE_Tips,0,setrangeCommand,4,CMD_WRITE|CMD_DENYOOM,ACL_CATEGORY_STRING,SETRANGE_Keyspecs,1,NULL,3),.args=SETRANGE_Args}, diff --git a/src/commands/set.json b/src/commands/set.json index d6bf28d191..fa466e852f 100644 --- a/src/commands/set.json +++ b/src/commands/set.json @@ -25,7 +25,7 @@ "Allowed the `NX` and `GET` options to be used together." ], [ - "8.2.0", + "8.1.0", "Added the `IFEQ` option." ] ], @@ -89,36 +89,37 @@ "name": "value", "type": "string" }, - { - "name": "comparison-value", - "type": "string", - "token": "IFEQ", - "since": "8.2.0", - "optional": true, - "summary": "Sets the key's value only if the current value matches the specified comparison value.", - "arguments": [ - { - "name": "comparison-value", - "type": "string", - "summary": "The value to compare with the current key's value before setting." - } - ] - }, { "name": "condition", "type": "oneof", "optional": true, - "since": "2.6.12", "arguments": [ { "name": "nx", "type": "pure-token", - "token": "NX" + "token": "NX", + "since": "2.6.12" }, { "name": "xx", "type": "pure-token", - "token": "XX" + "token": "XX", + "since": "2.6.12" + }, + { + "name": "comparison-value", + "type": "string", + "token": "IFEQ", + "since": "8.1.0", + "optional": true, + "summary": "Sets the key's value only if the current value matches the specified comparison value.", + "arguments": [ + { + "name": "comparison-value", + "type": "string", + "summary": "The value to compare with the current key's value before setting." + } + ] } ] }, diff --git a/src/t_string.c b/src/t_string.c index 7e83baa43b..618aa17509 100644 --- a/src/t_string.c +++ b/src/t_string.c @@ -337,7 +337,7 @@ int parseExtendedStringArgumentsOrReply(client *c, int *flags, int *unit, robj * } /* SET key value [NX] [XX] [KEEPTTL] [GET] [EX ] [PX ] - * [EXAT ][PXAT ] [IFEQ ] */ + * [EXAT ][PXAT ] [IFEQ comparison-value] */ void setCommand(client *c) { robj *expire = NULL; robj *comparison = NULL; @@ -407,11 +407,10 @@ void getCommand(client *c) { */ void getexCommand(client *c) { robj *expire = NULL; - robj *comparison = NULL; int unit = UNIT_SECONDS; int flags = OBJ_NO_FLAGS; - if (parseExtendedStringArgumentsOrReply(c, &flags, &unit, &expire, &comparison, COMMAND_GET) != C_OK) { + if (parseExtendedStringArgumentsOrReply(c, &flags, &unit, &expire, NULL, COMMAND_GET) != C_OK) { return; } diff --git a/tests/assets/test_cli_hint_suite.txt b/tests/assets/test_cli_hint_suite.txt index 6e5e6c0303..91f6f4cf7a 100644 --- a/tests/assets/test_cli_hint_suite.txt +++ b/tests/assets/test_cli_hint_suite.txt @@ -68,18 +68,18 @@ "ZRANGE k 1 2 WITHSCORES " "[BYSCORE|BYLEX] [REV] [LIMIT offset count]" # Optional one-of args with parameters: SET key value [NX|XX] [GET] [EX seconds|PX milliseconds|EXAT unix-time-seconds|PXAT unix-time-milliseconds|KEEPTTL] -"SET key value " "[IFEQ comparison-value] [NX|XX] [GET] [EX seconds|PX milliseconds|EXAT unix-time-seconds|PXAT unix-time-milliseconds|KEEPTTL]" -"SET key value EX" "[IFEQ comparison-value] [NX|XX] [GET] [EX seconds|PX milliseconds|EXAT unix-time-seconds|PXAT unix-time-milliseconds|KEEPTTL]" -"SET key value EX " "seconds [IFEQ comparison-value] [NX|XX] [GET]" -"SET key value EX 23 " "[IFEQ comparison-value] [NX|XX] [GET]" -"SET key value EXAT" "[IFEQ comparison-value] [NX|XX] [GET] [EX seconds|PX milliseconds|EXAT unix-time-seconds|PXAT unix-time-milliseconds|KEEPTTL]" -"SET key value EXAT " "unix-time-seconds [IFEQ comparison-value] [NX|XX] [GET]" -"SET key value PX" "[IFEQ comparison-value] [NX|XX] [GET] [EX seconds|PX milliseconds|EXAT unix-time-seconds|PXAT unix-time-milliseconds|KEEPTTL]" -"SET key value PX " "milliseconds [IFEQ comparison-value] [NX|XX] [GET]" -"SET key value PXAT" "[IFEQ comparison-value] [NX|XX] [GET] [EX seconds|PX milliseconds|EXAT unix-time-seconds|PXAT unix-time-milliseconds|KEEPTTL]" -"SET key value PXAT " "unix-time-milliseconds [IFEQ comparison-value] [NX|XX] [GET]" -"SET key value KEEPTTL " "[IFEQ comparison-value] [NX|XX] [GET]" -"SET key value XX " "[IFEQ comparison-value] [GET] [EX seconds|PX milliseconds|EXAT unix-time-seconds|PXAT unix-time-milliseconds|KEEPTTL]" +"SET key value " "[NX|XX|[IFEQ comparison-value]] [GET] [EX seconds|PX milliseconds|EXAT unix-time-seconds|PXAT unix-time-milliseconds|KEEPTTL]" +"SET key value EX" "[NX|XX|[IFEQ comparison-value]] [GET] [EX seconds|PX milliseconds|EXAT unix-time-seconds|PXAT unix-time-milliseconds|KEEPTTL]" +"SET key value EX " "seconds [NX|XX|[IFEQ comparison-value]] [GET]" +"SET key value EX 23 " "[NX|XX|[IFEQ comparison-value]] [GET]" +"SET key value EXAT" "[NX|XX|[IFEQ comparison-value]] [GET] [EX seconds|PX milliseconds|EXAT unix-time-seconds|PXAT unix-time-milliseconds|KEEPTTL]" +"SET key value EXAT " "unix-time-seconds [NX|XX|[IFEQ comparison-value]] [GET]" +"SET key value PX" "[NX|XX|[IFEQ comparison-value]] [GET] [EX seconds|PX milliseconds|EXAT unix-time-seconds|PXAT unix-time-milliseconds|KEEPTTL]" +"SET key value PX " "milliseconds [NX|XX|[IFEQ comparison-value]] [GET]" +"SET key value PXAT" "[NX|XX|[IFEQ comparison-value]] [GET] [EX seconds|PX milliseconds|EXAT unix-time-seconds|PXAT unix-time-milliseconds|KEEPTTL]" +"SET key value PXAT " "unix-time-milliseconds [NX|XX|[IFEQ comparison-value]] [GET]" +"SET key value KEEPTTL " "[NX|XX|[IFEQ comparison-value]] [GET]" +"SET key value XX " "[GET] [EX seconds|PX milliseconds|EXAT unix-time-seconds|PXAT unix-time-milliseconds|KEEPTTL]" # If an input word can't be matched, stop hinting. "SET key value FOOBAR " "" From f18468f07fa6a16d1db0f0e594d65811a51d40ad Mon Sep 17 00:00:00 2001 From: Sarthak Aggarwal Date: Tue, 3 Dec 2024 20:25:53 -0800 Subject: [PATCH 11/11] disallowing xx and nx with ifeq Signed-off-by: Sarthak Aggarwal --- src/t_string.c | 6 +++--- tests/unit/type/string.tcl | 16 ++-------------- 2 files changed, 5 insertions(+), 17 deletions(-) diff --git a/src/t_string.c b/src/t_string.c index 618aa17509..450da84505 100644 --- a/src/t_string.c +++ b/src/t_string.c @@ -250,12 +250,12 @@ int parseExtendedStringArgumentsOrReply(client *c, int *flags, int *unit, robj * /* clang-format off */ if ((opt[0] == 'n' || opt[0] == 'N') && (opt[1] == 'x' || opt[1] == 'X') && opt[2] == '\0' && - !(*flags & OBJ_SET_XX) && (command_type == COMMAND_SET)) + !(*flags & OBJ_SET_XX || *flags & OBJ_SET_IFEQ) && (command_type == COMMAND_SET)) { *flags |= OBJ_SET_NX; } else if ((opt[0] == 'x' || opt[0] == 'X') && (opt[1] == 'x' || opt[1] == 'X') && opt[2] == '\0' && - !(*flags & OBJ_SET_NX) && (command_type == COMMAND_SET)) + !(*flags & OBJ_SET_NX || *flags & OBJ_SET_IFEQ) && (command_type == COMMAND_SET)) { *flags |= OBJ_SET_XX; } else if ((opt[0] == 'g' || opt[0] == 'G') && @@ -321,7 +321,7 @@ int parseExtendedStringArgumentsOrReply(client *c, int *flags, int *unit, robj * (opt[1] == 'f' || opt[1] == 'F') && (opt[2] == 'e' || opt[2] == 'E') && (opt[3] == 'q' || opt[3] == 'Q') && opt[4] == '\0' && - next && (command_type == COMMAND_SET)) + next && !(*flags & OBJ_SET_NX || *flags & OBJ_SET_XX) && (command_type == COMMAND_SET)) { *flags |= OBJ_SET_IFEQ; *compare_val = next; diff --git a/tests/unit/type/string.tcl b/tests/unit/type/string.tcl index 751e86e7de..f49425c4f5 100644 --- a/tests/unit/type/string.tcl +++ b/tests/unit/type/string.tcl @@ -623,24 +623,12 @@ if {[string match {*jemalloc*} [s mem_allocator]]} { test "SET with IFEQ conditional - with xx" { r del foo - - assert_equal {} [r set foo "new_value" ifeq "initial_value" xx] - - r set foo "initial_value" - - assert_equal {OK} [r set foo "new_value" ifeq "initial_value" xx] - assert_equal "new_value" [r get foo] + assert_error {ERR syntax error} {r set foo "new_value" ifeq "initial_value" xx} } test "SET with IFEQ conditional - with nx" { r del foo - - assert_equal {} [r set foo "new_value" ifeq "initial_value" nx] - - r set foo "initial_value" - - assert_equal {} [r set foo "new_value" ifeq "initial_value" nx] - assert_equal "initial_value" [r get foo] + assert_error {ERR syntax error} {r set foo "new_value" ifeq "initial_value" nx} } test {Extended SET EX option} {