addressing comments

Signed-off-by: Sarthak Aggarwal <[email protected]>
valkey-io · Dec 4, 2024 · 66ad52d · 66ad52d
1 parent e3b4777
commit 66ad52d
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 39 deletions.
diff --git 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
@@ -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
@@ -337,7 +337,7 @@ int parseExtendedStringArgumentsOrReply(client *c, int *flags, int *unit, robj *
 }
 
 /* SET key value [NX] [XX] [KEEPTTL] [GET] [EX <seconds>] [PX <milliseconds>]
- *     [EXAT <seconds-timestamp>][PXAT <milliseconds-timestamp>] [IFEQ <comparison-value>] */
+ *     [EXAT <seconds-timestamp>][PXAT <milliseconds-timestamp>] [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
@@ -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 " ""