Some issues about the GET parameter in the SET command

[soloestoy]

SET key value [ nx | xx ] [ get ] [ EX seconds | PX milliseconds | EXAT unix-time-seconds | PXAT unix-time-milliseconds | keepttl ]

The usage of the SET command has become quite complex, as shown above. There are also some conflicts and ambiguities, particularly regarding the use of GET parameters and its reply. This complexity has also made it difficult for us to add new parameters, such as IFEQ in #1324.

Here are some examples:

1. NX parameter and the situation when it is used together with GET

   1.1 The situation of using NX only

    127.0.0.1:6379> del foo
    (integer) 1
    127.0.0.1:6379> set foo bar nx
    OK
    127.0.0.1:6379> set foo bar2 nx
    (nil)
   

   The result is clear: "OK" indicates successful, while "nil" indicates failure.

   1.2 The scenario of using NX with GET together

    127.0.0.1:6379> del foo
    (integer) 1
    127.0.0.1:6379> get foo
    (nil)
    127.0.0.1:6379> set foo bar nx get
    (nil)
   

   At this point, the client is unable to determine based on the result whether the SET operation was successful. Does "nil" indicate that the NX operation failed, or is it the value retrieved by the GET operation?
   The current code retrieves the value using GET, but we still cannot confirm whether the SET operation was successful (although it actually is).

2. XX parameter and the situation when it is used together with GET

   2.1 The situation of using NX only

    127.0.0.1:6379> del foo
    (integer) 1
    127.0.0.1:6379> set foo bar xx
    (nil)
    127.0.0.1:6379> set foo bar
    OK
    127.0.0.1:6379> set foo bar2 xx
    OK
   

   The result is clear: "OK" indicates successful, while "nil" indicates failure.

   2.2 The scenario of using NX with GET together

    127.0.0.1:6379> del foo
    (integer) 1
    127.0.0.1:6379> set foo bar xx get
    (nil)
    127.0.0.1:6379> get foo
    (nil)
    127.0.0.1:6379> set foo bar
    OK
    127.0.0.1:6379> set foo bar2 xx get
    "bar"
    127.0.0.1:6379> get foo
    "bar2"
   

   Similar to NX, we cannot determine the success of SET based on the results.

[zuiderkwast]

Yes, in IFEQ we discussed forbidding the combination of GET with IFEQ, but we decided to add it in the same way as already exists for XX and NX. It would make the syntax more complex if we want to forbid this combination, but I think almost nobody will use it.

• NX + GET is completely useless. It always returns (nil). The client can't use it for anything useful.
  EDIT: I looked up the PR where this was added, in 7.0 (2021). Actually, it returns (nil) on success and the old value on failure, so it's possible for a client to use it, but it's unintuitive.
• XX + GET is possible to use. If it returns (nil) it means that the command failed and the key didn't exist. If it returns a string, it means the key existed and the command succeeded.
• IFEQ + GET is possible but non-trivial to use. If SET foo bar IFEQ baz GET returns "baz" it means the command succeeded. If it returns anything else, the command failed.

[zuiderkwast]

It seems like some users actually want to use SET NX GET. Here someone wanted to add it in KeyDB: Snapchat/KeyDB#415

[soloestoy]

It's better if no one is using it, so we can prohibit such usage, but things are usually not that simple.

I think that since this usage is allowed, both the behavior and the results should be clear and definitive. However, the current mixing of GET and other parameters makes things overly complicated.

I'm thinking that we could even introduce a new command, such as SET2 or something other name, which would return an array where the first element indicates whether the SET was successful and the second element represents the value of GET.

[zuiderkwast]

I'm thinking that we could even introduce a new command, such as SET2

IMO a new command will create even more possibilities and more confusion. I think it's better to just use two commands GET and SET in a pipeline.

Let's describe clearly in the beginning of the docs for SET something like this?

Normal usage is just SET key value. Most users can ignore the other parameters.

[madolson]

I'm thinking that we could even introduce a new command, such as SET2

I also would prefer we not introduce a SET2.

[soloestoy]

We should document it: If GET parameters are used, it's not possible to determine from the result whether the SET command was successful. Then, see how users provide feedback during use.

[gmbnomis]

We should document it: If GET parameters are used, it's not possible to determine from the result whether the SET command was successful. Then, see how users provide feedback during use.

Sorry, but I find the new wording in the documentation confusing, since, as @zuiderkwast pointed out above (#1415 (comment)), GET + NX/XX are usable and one can determine from the result whether the "SET" was successful. But now, the documentation says that it is impossible to determine whether the SET command executed successfully based on the reply alone.

My "mental model" of e.g. NX + GET is

EVAL "local result = redis.call('GET', KEYS[1]); redis.call('SET', KEYS[1], ARGV[1], 'NX'); return result;" ...

So, if the result is (nil) the SET must have been successful (that's the condition for NX to set the value) unless I get an error.

If it isn't (nil) the set did not happen because the key did already exist (I just got it's value).

Am I missing a corner case here?

[zuiderkwast]

When we updated the documentation, I wanted to add the word "alone".

Yes, the user can determine if the command succeeded, but not from the reply alone. The client also needs to also consider which options were used in the command. The client can use this pseudo-code to find out if it was success or fail:

function is_success(reply, args...):
    if GET in args:
        if IFEQ in args:
            return (reply == comparison-value)
        else if NX in args:
            return (reply is nil)
        else if XX in args:
            return (reply is not nil)
        else:
            return true
    else:
        return (reply == OK)

[gmbnomis]

Yes, the user can determine if the command succeeded, but not from the reply alone. The client also needs to also consider which options were used in the command.

While this is technically true, isn't this a little bit subtle? (especially without further explanation)

What if we transform part of your pseudo code into the following documentation note? (to be added to the GET option)

Note: When using GET together with NX/XX/IFEQ, the reply indirectly indicates whether the key was set:

• GET and NX given: null reply indicates the key was set.
• GET and XX given: non-null reply indicates the key was set.
• GET and IFEQ given: the key was set if the reply is equal to comparison-value.

[zuiderkwast]

Sure, can you open a doc PR?