Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GEOSEARCHSTORE - ASC | DESC options are meaningless #157

Open
shohamazon opened this issue Jun 16, 2024 · 2 comments
Open

GEOSEARCHSTORE - ASC | DESC options are meaningless #157

shohamazon opened this issue Jun 16, 2024 · 2 comments
Labels
documentation Improvements or additions to documentation

Comments

@shohamazon
Copy link

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 
@shohamazon shohamazon changed the title GEOSEARCHSTORE - ASC | DESC options are meaningless GEOSEARCHSTORE - ASC | DESC options are meaningless Jun 16, 2024
@shohamazon shohamazon changed the title GEOSEARCHSTORE - ASC | DESC options are meaningless GEOSEARCHSTORE - ASC | DESC options are meaningless Jun 16, 2024
@madolson
Copy link
Member

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
Copy link
Author

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! 🙂

@hwware hwware added the documentation Improvements or additions to documentation label Jun 19, 2024
@madolson madolson transferred this issue from valkey-io/valkey Jul 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

3 participants