SET: Be more explicit about replies when using GET in conjunction wit…

…h XX/NX/IFEQ (#206) Explain the return value depending depending on the GET argument and add some notes about how to interpret the reply for the combination of GET with the XX/NX/IFEQ. Based on the conversation/comment valkey-io/valkey#1415 (comment) Signed-off-by: Simon Baatz <[email protected]>
valkey-io · Jan 8, 2025 · e839acd · e839acd
1 parent 7a4c92a
commit e839acd
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 11 deletions.
diff --git a/commands/set.md b/commands/set.md
@@ -15,7 +15,6 @@ The `SET` command supports a set of options that modify its behavior:
 * `IFEQ` *comparison-value* -- Set the key if the comparison value matches the existing value. An error is returned and `SET` aborted if the value stored at key is not a string.
 * `KEEPTTL` -- Retain the time to live associated with the key.
 * `GET` -- Return the old string stored at key, or nil if key did not exist. An error is returned and `SET` aborted if the value stored at key is not a string.
-    * Note: When `GET` is used together with `NX`/`XX`/`IFEQ`, it is impossible to determine whether the `SET` command executed successfully based on the reply alone.
 
 Note: Since the `SET` command options can replace `SETNX`, `SETEX`, `PSETEX`, `GETSET`, it is possible that in future versions of Valkey these commands will be deprecated and finally removed.
 

diff --git a/resp2_replies.json b/resp2_replies.json
@@ -974,11 +974,16 @@
     "[Simple string reply](../topics/protocol.md#simple-strings): `OK`."
   ],
   "SET": [
-    "Any of the following:",
-    "* [Nil reply](../topics/protocol.md#bulk-strings): `GET` not given: Operation was aborted (conflict with one of the `XX`/`NX` options).",
-    "* [Simple string reply](../topics/protocol.md#simple-strings): `OK`. `GET` not given: The key was set.",
-    "* [Nil reply](../topics/protocol.md#bulk-strings): `GET` given: The key didn't exist before the `SET`.",
-    "* [Bulk string reply](../topics/protocol.md#bulk-strings): `GET` given: The previous value of the key."
+    "If `GET` not given, any of the following:",
+    "* [Nil reply](../topics/protocol.md#bulk-strings): Operation was aborted (conflict with one of the `XX`/`NX` options).",
+    "* [Simple string reply](../topics/protocol.md#simple-strings): `OK`: The key was set.",
+    "If `GET` given, any of the following:",
+    "* [Nil reply](../topics/protocol.md#bulk-strings): The key didn't exist before the `SET`.",
+    "* [Bulk string reply](../topics/protocol.md#bulk-strings): The previous value of the key.",
+    "Note that when using `GET` together with `XX`/`NX`/`IFEQ`, the reply indirectly indicates whether the key was set:",
+    "* `GET` and `XX` given: Non-[Nil reply](../topics/protocol.md#bulk-strings) indicates the key was set.",
+    "* `GET` and `NX` given: [Nil reply](../topics/protocol.md#bulk-strings) indicates the key was set.",
+    "* `GET` and `IFEQ` given: The key was set if the reply is equal to `comparison-value`."
   ],
   "SETBIT": [
     "[Integer reply](../topics/protocol.md#integers): the original bit value stored at _offset_."

diff --git a/resp3_replies.json b/resp3_replies.json
@@ -1039,11 +1039,16 @@
     "[Array reply](../topics/protocol.md#arrays): List of monitored replicas, and their state."
   ],
   "SET": [
-    "Any of the following:",
-    "* [Null reply](../topics/protocol.md#nulls): `GET` not given: Operation was aborted (conflict with one of the `XX`/`NX` options).",
-    "* [Simple string reply](../topics/protocol.md#simple-strings): `OK`. `GET` not given: The key was set.",
-    "* [Null reply](../topics/protocol.md#nulls): `GET` given: The key didn't exist before the `SET`.",
-    "* [Bulk string reply](../topics/protocol.md#bulk-strings): `GET` given: The previous value of the key."
+    "If `GET` not given, any of the following:",
+    "* [Null reply](../topics/protocol.md#nulls): Operation was aborted (conflict with one of the `XX`/`NX` options).",
+    "* [Simple string reply](../topics/protocol.md#simple-strings): `OK`: The key was set.",
+    "If `GET` given, any of the following:",
+    "* [Null reply](../topics/protocol.md#nulls): The key didn't exist before the `SET`.",
+    "* [Bulk string reply](../topics/protocol.md#bulk-strings): The previous value of the key.",
+    "Note that when using `GET` together with `XX`/`NX`/`IFEQ`, the reply indirectly indicates whether the key was set:",
+    "* `GET` and `XX` given: Non-[Null reply](../topics/protocol.md#nulls) indicates the key was set.",
+    "* `GET` and `NX` given: [Null reply](../topics/protocol.md#nulls) indicates the key was set.",
+    "* `GET` and `IFEQ` given: The key was set if the reply is equal to `comparison-value`."
   ],
   "SETBIT": [
     "[Integer reply](../topics/protocol.md#integers): the original bit value stored at _offset_."