feat(hset_family): Add NX option to HSETEX

[BagritsevichStepan]

fixes #3265

[BagritsevichStepan]

IMHO, the returned value from HSETEX [NX] is a bit confusing:

1. If NX option not specified, then the returned value means the following:
   • (integer) 1: a new field is created.
   • (integer) 0: the field already existed and was updated.
2. If NX option specified, then the returned value means the following:
   • (integer) 1: a new field is created.
   • (integer) 0: the field already existed and was not updated.

Thus, the contract varies depending on NX option.

Example

127.0.0.1:6379> HSETEX myhash 1000 field1 "value1"
(integer) 1
127.0.0.1:6379> HSETEX myhash 1000 field1 "value2"
(integer) 0
127.0.0.1:6379> HGET myhash field1
"value2"
127.0.0.1:6379> HSETEX myhash 1000 field2 "value1"
(integer) 1
127.0.0.1:6379> HSETEX myhash NX 1000 field2 "value2"
(integer) 0
127.0.0.1:6379> HGET myhash field2
"value1"

Is it okay (I will describe all the cases in the documentation)? Or is it better to add some new value for the case when the field already exists but the value was updated?

[romange]

I agree it's confusing. We should fix it.

[romange]

@BagritsevichStepan seems that documentation is not aligned with the code, can you confirm?
https://www.dragonflydb.io/docs/command-reference/hashes/hsetex

[BagritsevichStepan]

@BagritsevichStepan seems that documentation is not aligned with the code, can you confirm? https://www.dragonflydb.io/docs/command-reference/hashes/hsetex

Yes, I wanted to create an issue about the errors in the documentation

[romange]

Sure, but please assign it to yourself :)
our docs are located at https://github.com/dragonflydb/documentation and you will need to update then with NX option anyways.

[BagritsevichStepan]

Sure, but please assign it to yourself :) our docs are located at https://github.com/dragonflydb/documentation and you will need to update then with NX option anyways.

Yes, that's exactly what I meant, that I'll fix the create and then fix this issue

[BagritsevichStepan]

Upd.: After discussion, it was decided to return an array of two values: [(number of added fields), (number of updated fields)]

[adiholden]

@romange
I want to re discuss the need to do the breaking change with the return value from HSETEX.
Today HSETEX return value is the number new fields added to hash set, when adding the flag NX we can still just return the number new fields added the the hash set.
In case that user uses the NX option the return value meaning does not change, only that the internal behavior that the field was not update as the user asked by using the NX option.
From my point of view changing the return value to array of [(number of added fields), (number of updated fields)] is redundant unless there is acutally need from user for the value of updated fields which I dont think this is the case.
Today we just return the first field in this array.

Trying compare with other commands using the NX optinal field, SET command f.e
running set x y and than running set x z or running set NX x z will both return OK it does not matter that using the NX did not update the field. I know its not the same example as above but the idea of having different internal behavior (due to optional fields) with the same return value is there.

[romange]

Oh, I did not mean to change the return type based on the option. This is of course is not consistent.

I understand what you are saying. Let's keep the original single scalar being returned.

[BagritsevichStepan]

Oh, I did not mean to change the return type based on the option. This is of course is not consistent.

I understand what you are saying. Let's keep the original single scalar being returned.

Sorry, maybe I misunderstood you. But HSETEX always returns an array of two values, including the case without the NX option

[BagritsevichStepan]

Remove the changes of the returned value

[adiholden]

Oh, I did not mean to change the return type based on the option. This is of course is not consistent.

I understand what you are saying. Let's keep the original single scalar being returned.

I did not say that we change the return type based on the option.. today we return integer, the suggested solution by Stepan says return array of 2 integers when using NX or not.
I am saying that from my understanding the change in the return type is redundant, as I dont see problem with returning the number of added items to the hset regardless of the option NX which implies how the command will affect elements in the set that were not added