Missing WITHHASH option in GEOxxx commands

[zhongxinghong]

see https://redis.io/docs/latest/commands/georadius/

[nihohit]

hi, PRs are welcome :)

[zhongxinghong]

I try to add hash field to RadiusSearchResult, but I found out there might be some ambiguity when parsing dist & hash. The dist with f64 type could be parsed from Value::Int and the hash with u64 could alse be parsed from Value::Int too. Should we limit the type of dist to Value::XXXString and the type of hash to Value::Int? :(

[nihohit]

Should we limit the type of dist to Value::XXXString and the type of hash to Value::Int?

Would that help? AFAIS the hash could still be parsed as string if no dist was provided.

I don't think that there's a way to disambiguate a response to a request that has WITHHASH without coords or dist, from a response to a request that has WITHDIST without coords or hash. According to the docs, WITHHASH is a niche feature, so IMO the simplest solution is to require that WITHHASH be set only on requests that also set WITHDIST, so there'll be no ambiguity - the first value is always dist, the second is always hash.

[zhongxinghong]

Just test these commands in redis-cli 6.2.5:

127.0.0.1:6379> GEOADD Sicily 13.361389 38.115556 "Palermo" 15.087269 37.502669 "Catania"
(integer) 2
127.0.0.1:6379> GEORADIUS Sicily 15 37 200 km WITHDIST WITHHASH WITHCOORD
1) 1) "Palermo"
   2) "190.4424"
   3) (integer) 3479099956230698
   4) 1) "13.36138933897018433"
      2) "38.11555639549629859"
2) 1) "Catania"
   2) "56.4413"
   3) (integer) 3479447370796909
   4) 1) "15.08726745843887329"
      2) "37.50266842333162032"
127.0.0.1:6379> GEORADIUS Sicily 15 37 200 km WITHHASH
1) 1) "Palermo"
   2) (integer) 3479099956230698
2) 1) "Catania"
   2) (integer) 3479447370796909
127.0.0.1:6379>

A GEORADIUS search can only have single WITHHASH option and the second field of each Array element has an (integer) type. Currently a rust f64 value can be parsed from Value::Int / Value::SimpleString / Value::BulkString / Value::Double based on from_redis_value_for_num!(f64);, so there will be ambiguous: we don't know the second field in reply is dist or hash without knowing the request if the server send an (interger). In https://github.com/redis/go-redis/blob/master/command.go (cmd *GeoLocationCmd) readReply(rd *proto.Reader) method, go-redis parse dist / hash based on the request so it's very clear: only parse dist if cmd.q.WithDist has been set. But in redis-rs the RadiusSearchResult doesn't known the request. I agree. the WITHHASH is just a niche feature, so it's reasonable to just ignore this corner case, but it's not a perfect solution. For users who encounter this corner case It will be tricky because there will not be any error but just an mis-parsed result.