redis reactive client always returns null on set command

[dhiaeddineTounekti]

Describe the bug

Quarkus redis reactive client always returns null when using set api with other arguments, also here.

for example this code

redisCommands.set("myKey", "myValue").awaitSuspending()
        val request = Request.cmd(Command.SET)
            .arg("myKey")
            .arg("Value")
            .arg("NX")
            .arg("EX")
            .arg(10)
val res1 = redisClient.redis.send(request).awaitSuspending()
val res2 = redisClient.redis.send(request).awaitSuspending()
println("res1: $res1, res2: $res2")

this code prints "OK" and null

while this code

var res1 = redisCommands.set("myKey", "myValue", SetArgs().nx().ex(10)).awaitSuspending()
var res2 = redisCommands.set("myKey", "myValue", SetArgs().nx().ex(10)).awaitSuspending()
println("res1: $res1, res2: $res2")

print null and null

Expected behavior

return true, false or return Ok and null like the database does.

Actual behavior

always returns null.

How to Reproduce?

No response

Output of uname -a or ver

No response

Output of java -version

No response

Quarkus version or git rev

No response

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

No response

[quarkus-bot]

/cc @Ladicek (redis), @cescoffier (redis), @machi1990 (redis)

[cescoffier]

The Set command always returns null. If you want the previously stored value, use setGet.

[dhiaeddineTounekti]

Yes, I'm aware of that. However, I find the current behaviour somewhat misleading, as it doesn't align with Redis semantics. Redis clearly distinguishes between operations that result in a change (returning OK) and those that do not (returning null).
I think this behaviour should change what do you think @cescoffier ?

[cescoffier]

These operations return a Uni, meaning that the outcome (null) is just a signal to indicate the command was successfully executed. We follow the Mutiny semantic. Producing an OK string would be odd:

• you get a completion event: it was successful
• you get a failure event: it was not successful

[dhiaeddineTounekti]

I agree with the general principle; however, in this specific case, there are two distinct successful outcomes, and it is not possible to distinguish between them if the returned result is always null.

[cescoffier]

What do you mean by two distinct successful outcomes? SET can only return OK expect when used with the GET parameter which is implemented as a second method (setGet)

[dhiaeddineTounekti]

When the SET command is used in combination with the NX option (i.e., "set if not exists"), it can result in two distinct successful outcomes:

The command executes successfully but does not update the key and value.
The command executes successfully and does update the key and value.
To differentiate between these two cases, the setnx command can be used, which returns true if the key was set (i.e., the second case), and false if it was not (i.e., the first case).

However, if I want to combine SET with both NX and EX (to set the key only if it does not exist and also assign an expiration time), I must use the SET command. Unfortunately, in this configuration, the command always returns null, making it impossible to determine whether the key was actually set or not.

[cescoffier]

Ok, so we will have to introduce new set variant.

It does not seems to be covered by the ´return' definition in the set command doc.

[dhiaeddineTounekti]

yes, that sounds like a good idea.

[cescoffier]

I'm still surprised that the return part of the command definition does not capture this:

• Nil reply: GET not given: Operation was aborted (conflict with one of the XX/NX options). --> This is a failure
• Simple string reply: OK. GET not given: The key was set. --> This is null item
• Nil reply: GET given: The key didn't exist before the SET. --> This is a failure in our case
• Bulk string reply: GET given: The previous value of the key. --> This is the item

https://redis.io/docs/latest/commands/set/

Also, I don't see the behavior you are describing:

EX seconds -- Set the specified expire time, in seconds (a positive integer).
NX -- Only set the key if it does not already exist.

Nothing is said about the return value. However, the doc mentions specific SETNX and SETEX commands. These commands are covered by:

• setnx returning a boolean ({@code true} the key was set {@code false} the key was not set)
• setex returning void

[dhiaeddineTounekti]

what do you mean here by a failure ? Does it throw something ? because in my tests it always returns null.

• Nil reply: GET not given: Operation was aborted (conflict with one of the XX/NX options). --> This is a failure

unless I am doing something wrong. This is the code I am using to test

/** Just for reference
val redisClient: ReactiveRedisDataSource
val redisQueue: ReactiveListCommands<String, String> =
        reactiveRedisDataSource.list(String::class.java)
*/

suspend fun test() {
        val res = redisCommands.set("test", "abc", SetArgs().nx().ex(100)).awaitSuspending() // (succeeds) SET test abc NX EX 100
        val res2 = redisCommands.set("test", "abc", SetArgs().nx().ex(100)).awaitSuspending() // (fails) SET test abc NX EX 100
        println("$res , $res2")
    }

If I understood you correctly, res should be null while res2 should throw an error, is that right ?
If it is the case then this is not what happens res2 is also null.