addressing comments and adding tests

Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>

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

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

src/t_string.c

@@ -105,12 +105,21 @@ 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) {
+
+        if (current_value == NULL || current_value->type != OBJ_STRING || checkType(c, comparison, OBJ_STRING) != 0) {
+            addReplyError(c, "value(s) must be present or string");
+            return;
+        }
+
+        if (compareStringObjects(current_value, comparison) != 0) {
             addReply(c, abort_reply ? abort_reply : shared.null[c->resp]);
             return;
         }
+    } else if (flags & OBJ_SET_IFEQ && !found) {
+        addReply(c, abort_reply ? abort_reply : shared.null[c->resp]);
+        return;
     }
 
     if ((flags & OBJ_SET_NX && found) || (flags & OBJ_SET_XX && !found)) {

tests/unit/type/string.tcl

@@ -583,16 +583,56 @@ if {[string match {*jemalloc*} [s mem_allocator]]} {
     } {*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 {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]
+
+        r del foo
+    }
+
+    test "SET with IFEQ conditional - non-string current value" {
+        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"}
+
+        r del foo
+    }
+
+    test "SET with IFEQ conditional - with get" {
+        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]
+
+        r del foo
+    }
+
+
+    test "SET with IFEQ conditional - with xx" {
+        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]
+
+        r del foo
+    }
+
+    test "SET with IFEQ conditional - with nx" {
+        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]
+
+        r del foo
     }
 
     test {Extended SET EX option} {