valkey-io / valkey-doc

Other
19 stars 29 forks source link

GEOSEARCHSTORE - ASC | DESC options are meaningless #157

Open shohamazon opened 2 months ago

shohamazon commented 2 months ago

The GEOSEARCHSTORE command in Valley currently supports the ASC and DESC options, which are used to specify the sorting order of the search results. However, in the context of GEOSEARCHSTORE, where the results are stored in a sorted set, these options become redundant.

When using GEOSEARCHSTORE, the results are stored in a sorted set where the order is determined by the score. Therefore, explicitly specifying the sorting order with ASC or DESC options is unnecessary and has no effect on the final stored results.

Example:

127.0.0.1:6379> GEOADD Sicily 13.361389 38.115556 "Palermo" 15.087269 37.502669 "Catania"
(integer) 2
127.0.0.1:6379> GEOSEARCHSTORE key2 Sicily FROMLONLAT 15 37 BYBOX 400 400 km ASC COUNT 3
(integer) 2
127.0.0.1:6379> ZRANGE key2 0 -1 WITHSCORES
1) 1) "Palermo"
   2) (double) 3479099956230698
2) 1) "Catania"
   2) (double) 3479447370796909
127.0.0.1:6379> GEOSEARCHSTORE key2 Sicily FROMLONLAT 15 37 BYBOX 400 400 km DESC COUNT 3
(integer) 2
127.0.0.1:6379> ZRANGE key2 0 -1 WITHSCORES
1) 1) "Palermo"
   2) (double) 3479099956230698
2) 1) "Catania"
   2) (double) 3479447370796909 
madolson commented 2 months ago

It's a breaking change to remove them, since someone might be calling them and seeing the intended result. Is there any concern with just leaving them as is and just documenting that they don't have any effect?

shohamazon commented 2 months ago

It's fine to update the documentation to clarify that ASC and DESC don't affect GEOSEARCHSTORE. There's no real concern with keeping them as they are.

Thanks for your response, and have a great day! 🙂