127.0.0.1:6379> XADD x 1-0 f v
"1-0"
127.0.0.1:6379> XADD x 2-0 f v
"2-0"
127.0.0.1:6379> HELLO 3
1# "server" => "redis"
2# "version" => "255.255.255"
3# "proto" => (integer) 3
4# "id" => (integer) 4
5# "mode" => "standalone"
6# "role" => "master"
7# "modules" => (empty array)
127.0.0.1:6379> XREAD STREAMS x x 0 0
1# "x" => 
   1) 1) "1-0"
      2) 1) "f"
         2) "v"
   2) 1) "2-0"
      2) 1) "f"
         2) "v"
2# "x" => 
   1) 1) "1-0"
      2) 1) "f"
         2) "v"
   2) 1) "2-0"
      2) 1) "f"
         2) "v"

in RESP2 it is returned as an array of tuples (keyname, entries-array)

we have four options, ordered in descending order, from the most breaking one to the least: 1. change the protocol so that the reply in RESP3 is the same as the reply in RESP2 2. return an error in case the user gave dup keys as input 3. do nothing (may result in an error in the client library) 4. change the code to unite the results (i.e. take the minimal id for all dup keys in input)

Comment From: guybe7

@redis/core-team please vote

Comment From: oranagra

these are sorted according to the breakage they cause 1. major breaking change for all users (who use RESP3) 2. breaking change only to whoever uses dup keys, both for RESP3 and RESP2 (will need to issue separate commands instead) 3. will only break whoever uses dup keys with RESP3 and a client that has an issue with that, in which case only these users will have to adjust their code. 4. considering the response is in any way not coupled with the input (you can ask for 3 keys and get 2), i think this is a valid approach.

i vote for option 4

Comment From: guybe7

i vote for option 2

Comment From: itamarhaber

  • Vote: option 2
  • Question: backport?
  • CC: @michael-grunder @chayim @leibale

Comment From: chayim

Vote option 2

Comment From: michael-grunder

Options 2 and 4 both seem fine.

I lean toward option 2 since many clients are probably sending keys and IDs via an associative array anyway

Comment From: oranagra

note that the returned array doesn't necessarily match the input keys anyway (you can ask for 3 keys and get output for just two)

Comment From: michael-grunder

This is probably moot for many clients, that do something like this:

$redis->xRead(['s1' => '0-1', 's2' => '0-2']);

The only trepidation I'd have about option 4 is whether it will obscure what are effectively client-side logic bugs.

Comment From: soloestoy

TBH, I wanna vote for 1, I think it's a bug that we return a map in RESP3... if we can notify all client developers to fix it synchronously, I believe this is the most correct option, and seems very few users are using RESP3.

Second best, partially vote for 2, if we wanna minimize breaking change, I think we can just disallow dup keys only in RESP3, I don't think block users who are using dup keys in RESP2 is good idea, it's a valid usage I think, and up to now we didn't receive any real issues in this case from RESP3 users.

Comment From: oranagra

i now understand what i was missing and why option 4 isn't suitable. if the client uses COUNT, then merging the two requests using minimum, isn't really valid. but then it also means that the RESP2 response isn't really valid since the caller shouldn't map the index of the requested key to the index of the response (some keys can be missing).

in that case, i vote for 2. next question would be in which version to release that breaking change (i assume not many will be affected)

Comment From: oranagra

solution 2 approved in a core-team meeting for redis 8.0

Comment From: guybe7

will PR soon

Comment From: oranagra

note, i now realize that solution 2 (means some performance overhead), maybe we should re-consider solution 1. specifically if we also handle #12014 or #11812

Comment From: leibale

@oranagra @guybe7 ~I think GEODIST was overlooked as well #12361~

edit: ignore... :P