Adds HGETDEL Support to Valkey (#2851)

roshkhatri · ranshid · web-flow · commit 56ab3c4a8135 · 2025-11-26T15:19:53.000+02:00
Fixes this: #2850 Adds support for HGETDEL to Valkey and aligns with Redis 8.0 feature. Maintains syntax compatibility Retrieves all the values, and null if fields dont exists and deletes once retrieved. ``` 127.0.0.1:6379> HGETDEL key FIELDS numfields field [ field ... ] ``` ``` 127.0.0.1:6379> HSET foo field1 bar1 field2 bar2 field3 bar3 (integer) 3 127.0.0.1:6379> HGETDEL foo FIELDS 1 field2 1) "bar2" 127.0.0.1:6379> HGETDEL foo FIELDS 1 field2 1) (nil) 127.0.0.1:6379> HGETALL foo 1) "field1" 2) "bar1" 3) "field3" 4) "bar3" 127.0.0.1:6379> HGETDEL foo FIELDS 2 field2 field3 1) (nil) 2) "bar3" 127.0.0.1:6379> HGETALL foo 1) "field1" 2) "bar1" 127.0.0.1:6379> HGETDEL foo FIELDS 3 field1 non-exist-field (error) ERR syntax error 127.0.0.1:6379> HGETDEL foo FIELDS 2 field1 non-exist-field 1) "bar1" 2) (nil) 127.0.0.1:6379> HGETALL foo (empty array) 127.0.0.1:6379> ``` --------- Signed-off-by: Roshan Khatri <rvkhatri@amazon.com> Signed-off-by: Ran Shidlansik <ranshid@amazon.com> Co-authored-by: Ran Shidlansik <ranshid@amazon.com>
diff --git a/src/commands.def b/src/commands.def
@@ -3823,6 +3823,37 @@ struct COMMAND_ARG HGETALL_Args[] = {
 {MAKE_ARG("key",ARG_TYPE_KEY,0,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
 };
 
+/********** HGETDEL ********************/
+
+#ifndef SKIP_CMD_HISTORY_TABLE
+/* HGETDEL history */
+#define HGETDEL_History NULL
+#endif
+
+#ifndef SKIP_CMD_TIPS_TABLE
+/* HGETDEL tips */
+#define HGETDEL_Tips NULL
+#endif
+
+#ifndef SKIP_CMD_KEY_SPECS_TABLE
+/* HGETDEL key specs */
+keySpec HGETDEL_Keyspecs[1] = {
+{NULL,CMD_KEY_RW|CMD_KEY_ACCESS|CMD_KEY_DELETE,KSPEC_BS_INDEX,.bs.index={1},KSPEC_FK_RANGE,.fk.range={0,1,0}}
+};
+#endif
+
+/* HGETDEL fields argument table */
+struct COMMAND_ARG HGETDEL_fields_Subargs[] = {
+{MAKE_ARG("numfields",ARG_TYPE_INTEGER,0,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
+{MAKE_ARG("field",ARG_TYPE_STRING,-1,NULL,NULL,NULL,CMD_ARG_MULTIPLE,0,NULL)},
+};
+
+/* HGETDEL argument table */
+struct COMMAND_ARG HGETDEL_Args[] = {
+{MAKE_ARG("key",ARG_TYPE_KEY,0,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)},
+{MAKE_ARG("fields",ARG_TYPE_BLOCK,-1,"FIELDS",NULL,NULL,CMD_ARG_NONE,2,NULL),.subargs=HGETDEL_fields_Subargs},
+};
+
 /********** HGETEX ********************/
 
 #ifndef SKIP_CMD_HISTORY_TABLE
@@ -11800,6 +11831,7 @@ struct COMMAND_STRUCT serverCommandTable[] = {
 {MAKE_CMD("hexpiretime","Returns Unix timestamps in seconds since the epoch at which the given key's field(s) will expire","O(N) where N is the number of specified fields.","9.0.0",CMD_DOC_NONE,NULL,NULL,"hash",COMMAND_GROUP_HASH,HEXPIRETIME_History,0,HEXPIRETIME_Tips,0,hexpiretimeCommand,-5,CMD_READONLY|CMD_FAST,ACL_CATEGORY_HASH,HEXPIRETIME_Keyspecs,1,NULL,2),.args=HEXPIRETIME_Args},
 {MAKE_CMD("hget","Returns the value of a field in a hash.","O(1)","2.0.0",CMD_DOC_NONE,NULL,NULL,"hash",COMMAND_GROUP_HASH,HGET_History,0,HGET_Tips,0,hgetCommand,3,CMD_READONLY|CMD_FAST,ACL_CATEGORY_HASH,HGET_Keyspecs,1,NULL,2),.args=HGET_Args},
 {MAKE_CMD("hgetall","Returns all fields and values in a hash.","O(N) where N is the size of the hash.","2.0.0",CMD_DOC_NONE,NULL,NULL,"hash",COMMAND_GROUP_HASH,HGETALL_History,0,HGETALL_Tips,1,hgetallCommand,2,CMD_READONLY,ACL_CATEGORY_HASH,HGETALL_Keyspecs,1,NULL,1),.args=HGETALL_Args},
+{MAKE_CMD("hgetdel","Returns the values of one or more fields and deletes them from a hash.","O(N) where N is the number of fields to be retrieved and deleted.","9.1.0",CMD_DOC_NONE,NULL,NULL,"hash",COMMAND_GROUP_HASH,HGETDEL_History,0,HGETDEL_Tips,0,hgetdelCommand,-5,CMD_WRITE|CMD_FAST,ACL_CATEGORY_HASH,HGETDEL_Keyspecs,1,NULL,2),.args=HGETDEL_Args},
 {MAKE_CMD("hgetex","Get the value of one or more fields of a given hash key, and optionally set their expiration time or time-to-live (TTL).","O(N) where N is the number of specified fields.","9.0.0",CMD_DOC_NONE,NULL,NULL,"hash",COMMAND_GROUP_HASH,HGETEX_History,0,HGETEX_Tips,0,hgetexCommand,-5,CMD_WRITE|CMD_FAST,ACL_CATEGORY_HASH,HGETEX_Keyspecs,1,NULL,3),.args=HGETEX_Args},
 {MAKE_CMD("hincrby","Increments the integer value of a field in a hash by a number. Uses 0 as initial value if the field doesn't exist.","O(1)","2.0.0",CMD_DOC_NONE,NULL,NULL,"hash",COMMAND_GROUP_HASH,HINCRBY_History,0,HINCRBY_Tips,0,hincrbyCommand,4,CMD_WRITE|CMD_DENYOOM|CMD_FAST,ACL_CATEGORY_HASH,HINCRBY_Keyspecs,1,NULL,3),.args=HINCRBY_Args},
 {MAKE_CMD("hincrbyfloat","Increments the floating point value of a field by a number. Uses 0 as initial value if the field doesn't exist.","O(1)","2.6.0",CMD_DOC_NONE,NULL,NULL,"hash",COMMAND_GROUP_HASH,HINCRBYFLOAT_History,0,HINCRBYFLOAT_Tips,0,hincrbyfloatCommand,4,CMD_WRITE|CMD_DENYOOM|CMD_FAST,ACL_CATEGORY_HASH,HINCRBYFLOAT_Keyspecs,1,NULL,3),.args=HINCRBYFLOAT_Args},
diff --git a/src/commands/hgetdel.json b/src/commands/hgetdel.json
@@ -0,0 +1,79 @@
+{
+    "HGETDEL": {
+        "summary": "Returns the values of one or more fields and deletes them from a hash.",
+        "complexity": "O(N) where N is the number of fields to be retrieved and deleted.",
+        "group": "hash",
+        "since": "9.1.0",
+        "arity": -5,
+        "function": "hgetdelCommand",
+        "command_flags": [
+            "WRITE",
+            "FAST"
+        ],
+        "acl_categories": [
+            "HASH"
+        ],
+        "key_specs": [
+            {
+                "flags": [
+                    "RW",
+                    "ACCESS",
+                    "DELETE"
+                ],
+                "begin_search": {
+                    "index": {
+                        "pos": 1
+                    }
+                },
+                "find_keys": {
+                    "range": {
+                        "lastkey": 0,
+                        "step": 1,
+                        "limit": 0
+                    }
+                }
+            }
+        ],
+        "reply_schema": {
+            "description": "List of values associated with the given fields, in the same order as they are requested. Returns nil for fields that do not exist.",
+            "type": "array",
+            "minItems": 1,
+            "items": {
+                "oneOf": [
+                    {
+                        "type": "string"
+                    },
+                    {
+                        "type": "null"
+                    }
+                ]
+            }
+        },
+        "arguments": [
+            {
+                "name": "key",
+                "type": "key",
+                "key_spec_index": 0
+            },
+            {
+                "name": "fields",
+                "token": "FIELDS",
+                "type": "block",
+                "arguments": [
+                    {
+                        "name": "numfields",
+                        "type": "integer",
+                        "key_spec_index": 0,
+                        "multiple": false,
+                        "minimum": 1
+                    },
+                    {
+                        "name": "field",
+                        "type": "string",
+                        "multiple": true
+                    }
+                ]
+            }
+        ]
+    }
+}
diff --git a/src/server.h b/src/server.h
@@ -3926,6 +3926,7 @@ void hsetnxCommand(client *c);
 void hsetexCommand(client *c);
 void hgetexCommand(client *c);
 void hgetCommand(client *c);
+void hgetdelCommand(client *c);
 void hmgetCommand(client *c);
 void hdelCommand(client *c);
 void hlenCommand(client *c);
diff --git a/src/t_hash.c b/src/t_hash.c
@@ -1053,6 +1053,57 @@ void hdelCommand(client *c) {
     addReplyLongLong(c, deleted);
 }
 
+void hgetdelCommand(client *c) {
+    /* argv: [0]=HGETDEL, [1]=key, [2]=FIELDS, [3]=numfields, [4...]=fields */
+    int fields_index = 4;
+    int i, deleted = 0;
+    long long num_fields = 0;
+    bool keyremoved = false;
+
+    if (getLongLongFromObjectOrReply(c, c->argv[fields_index - 1], &num_fields, NULL) != C_OK) return;
+
+    /* Check that the parsed fields number matches the real provided number of fields */
+    if (!num_fields || num_fields != (c->argc - fields_index)) {
+        addReplyError(c, "numfields should be greater than 0 and match the provided number of fields");
+        return;
+    }
+
+    /* Don't abort when the key cannot be found. Non-existing keys are empty
+     * hashes, where HGETDEL should respond with a series of null bulks. */
+    robj *o = lookupKeyWrite(c->db, c->argv[1]);
+    if (checkType(c, o, OBJ_HASH)) return;
+
+    bool hash_volatile_items = hashTypeHasVolatileFields(o);
+
+    /* Reply with array of values and delete at the same time */
+    addReplyArrayLen(c, num_fields);
+    for (i = fields_index; i < c->argc; i++) {
+        addHashFieldToReply(c, o, c->argv[i]->ptr);
+
+        /* If hash doesn't exist, continue as already replied with NULL */
+        if (o == NULL) continue;
+        if (hashTypeDelete(o, c->argv[i]->ptr)) {
+            deleted++;
+            if (hashTypeLength(o) == 0) {
+                if (hash_volatile_items) dbUntrackKeyWithVolatileItems(c->db, o);
+                dbDelete(c->db, c->argv[1]);
+                keyremoved = true;
+                o = NULL;
+            }
+        }
+    }
+
+    if (deleted) {
+        if (!keyremoved && hash_volatile_items != hashTypeHasVolatileFields(o)) {
+            dbUpdateObjectWithVolatileItemsTracking(c->db, o);
+        }
+        signalModifiedKey(c, c->db, c->argv[1]);
+        notifyKeyspaceEvent(NOTIFY_HASH, "hdel", c->argv[1], c->db->id);
+        if (keyremoved) notifyKeyspaceEvent(NOTIFY_GENERIC, "del", c->argv[1], c->db->id);
+        server.dirty += deleted;
+    }
+}
+
 void hlenCommand(client *c) {
     robj *o;
 
diff --git a/tests/unit/type/hash.tcl b/tests/unit/type/hash.tcl
@@ -439,6 +439,85 @@ start_server {tags {"hash"}} {
         r hgetall htest
     } {}
 
+    test {HGETDEL - single field} {
+        r del myhash
+        r hset myhash field1 value1
+        set rv {}
+        lappend rv [r hgetdel myhash FIELDS 1 field1]
+        lappend rv [r hexists myhash field1]
+        lappend rv [r exists myhash]
+        set _ $rv
+    } {value1 0 0}
+
+    test {HGETDEL - multiple fields} {
+        r del myhash
+        r hmset myhash field1 value1 field2 value2 field3 value3
+        set rv {}
+        lappend rv [r hgetdel myhash FIELDS 2 field1 field3]
+        lappend rv [r hexists myhash field1]
+        lappend rv [r hexists myhash field2]
+        lappend rv [r hexists myhash field3]
+        lappend rv [r hget myhash field2]
+        set _ $rv
+    } {{value1 value3} 0 1 0 value2}
+
+    test {HGETDEL - non-existing field} {
+        r del myhash
+        r hset myhash field1 value1
+        set rv {}
+        lappend rv [r hgetdel myhash FIELDS 1 nonexisting]
+        lappend rv [r hexists myhash field1]
+        set _ $rv
+    } {{{}} 1}
+
+    test {HGETDEL - non-existing key and hash after the key is deleted } {
+        r del myhash
+        r hset myhash field1 value1
+        assert_equal {value1 {}} [r hgetdel myhash FIELDS 2 field1 field2]
+    }
+
+    test {HGETDEL - non-existing key} {
+        r del myhash
+        assert_equal {{}} [r hgetdel myhash FIELDS 1 field1]
+    }
+
+    test {HGETDEL - mix of existing and non-existing fields} {
+        r del myhash
+        r hmset myhash a 1 b 2 c 3
+        set rv {}
+        lappend rv [r hgetdel myhash FIELDS 3 a nonexist b]
+        lappend rv [r hexists myhash a]
+        lappend rv [r hexists myhash b]
+        lappend rv [r hexists myhash c]
+        set _ $rv
+    } {{1 {} 2} 0 0 1}
+
+    test {HGETDEL - hash becomes empty after deletion} {
+        r del myhash
+        r hmset myhash a 1 b 2
+        set rv {}
+        lappend rv [r hgetdel myhash FIELDS 2 a b]
+        lappend rv [r exists myhash]
+        set _ $rv
+    } {{1 2} 0}
+
+    test {HGETDEL - wrong type} {
+        r del wrongtype
+        r set wrongtype somevalue
+        assert_error "*WRONGTYPE*" {r hgetdel wrongtype FIELDS 1 field1}
+    }
+
+    test {HGETDEL - wrong number of arguments} {
+        assert_error "*wrong number of arguments*" {r hgetdel myhash}
+    }
+
+    test {HGETDEL - check for syntax and type errors} {
+        assert_error "*value is not an integer or out of range" {r hgetdel myhash a b c}
+        assert_error "*value is not an integer or out of range" {r hgetdel myhash FIELDS a b c}
+        assert_error "*numfields should be greater than 0 and match the provided number of fields" {r hgetdel myhash FIELDS 2 a b c}
+        assert_error "*numfields should be greater than 0 and match the provided number of fields" {r hgetdel myhash FIELDS 4 a b c}
+    }
+
     test {HDEL and return value} {
         set rv {}
         lappend rv [r hdel smallhash nokey]
