From 39b0f0dd73110989ee3ccdba3c202565dac41c7b Mon Sep 17 00:00:00 2001 From: Fabian Eichinger Date: Mon, 7 Jun 2021 15:47:58 +0200 Subject: [PATCH] Add support for combining NX and GET flags on SET command (#8906) Till now GET and NX were mutually exclusive. This change make their combination mean a "Get or Set" command. If the key exists it returns the old value and avoids setting, and if it does't exist it returns nil and sets it to the new value (possibly with expiry time) --- src/t_string.c | 16 +++++++++------- tests/unit/type/string.tcl | 34 +++++++++++++++++++++++++++++----- 2 files changed, 38 insertions(+), 12 deletions(-) diff --git a/src/t_string.c b/src/t_string.c index ef1a147e0..bbbe6af65 100644 --- a/src/t_string.c +++ b/src/t_string.c @@ -81,17 +81,19 @@ void setGenericCommand(client *c, int flags, robj *key, robj *val, robj *expire, return; } + if (flags & OBJ_SET_GET) { + if (getGenericCommand(c) == C_ERR) return; + } + if ((flags & OBJ_SET_NX && lookupKeyWrite(c->db,key) != NULL) || (flags & OBJ_SET_XX && lookupKeyWrite(c->db,key) == NULL)) { - addReply(c, abort_reply ? abort_reply : shared.null[c->resp]); + if (!(flags & OBJ_SET_GET)) { + addReply(c, abort_reply ? abort_reply : shared.null[c->resp]); + } return; } - if (flags & OBJ_SET_GET) { - if (getGenericCommand(c) == C_ERR) return; - } - genericSetKey(c,c->db,key, val,flags & OBJ_KEEPTTL,1); server.dirty++; notifyKeyspaceEvent(NOTIFY_STRING,"set",key,c->db->id); @@ -196,7 +198,7 @@ int parseExtendedStringArgumentsOrReply(client *c, int *flags, int *unit, robj * if ((opt[0] == 'n' || opt[0] == 'N') && (opt[1] == 'x' || opt[1] == 'X') && opt[2] == '\0' && - !(*flags & OBJ_SET_XX) && !(*flags & OBJ_SET_GET) && (command_type == COMMAND_SET)) + !(*flags & OBJ_SET_XX) && (command_type == COMMAND_SET)) { *flags |= OBJ_SET_NX; } else if ((opt[0] == 'x' || opt[0] == 'X') && @@ -207,7 +209,7 @@ int parseExtendedStringArgumentsOrReply(client *c, int *flags, int *unit, robj * } else if ((opt[0] == 'g' || opt[0] == 'G') && (opt[1] == 'e' || opt[1] == 'E') && (opt[2] == 't' || opt[2] == 'T') && opt[3] == '\0' && - !(*flags & OBJ_SET_NX) && (command_type == COMMAND_SET)) + (command_type == COMMAND_SET)) { *flags |= OBJ_SET_GET; } else if (!strcasecmp(opt, "KEEPTTL") && !(*flags & OBJ_PERSIST) && diff --git a/tests/unit/type/string.tcl b/tests/unit/type/string.tcl index 43968b26b..5dd9130c0 100644 --- a/tests/unit/type/string.tcl +++ b/tests/unit/type/string.tcl @@ -494,11 +494,35 @@ start_server {tags {"string"}} { list $old_value $new_value } {{} bar} - test {Extended SET GET with NX option should result in syntax err} { - catch {r set foo bar NX GET} err1 - catch {r set foo bar NX GET} err2 - list $err1 $err2 - } {*syntax err* *syntax err*} + test {Extended SET GET option with XX} { + r del foo + r set foo bar + set old_value [r set foo baz GET XX] + set new_value [r get foo] + list $old_value $new_value + } {bar baz} + + test {Extended SET GET option with XX and no previous value} { + r del foo + set old_value [r set foo bar GET XX] + set new_value [r get foo] + list $old_value $new_value + } {{} {}} + + test {Extended SET GET option with NX} { + r del foo + set old_value [r set foo bar GET NX] + set new_value [r get foo] + list $old_value $new_value + } {{} bar} + + test {Extended SET GET option with NX and previous value} { + r del foo + r set foo bar + set old_value [r set foo baz GET NX] + set new_value [r get foo] + list $old_value $new_value + } {bar bar} test {Extended SET GET with incorrect type should result in wrong type error} { r del foo