Python: adds GEOSEARCHSTORE command - #1581
Conversation
fdb8bfc to
5f0c379
Compare
| See https://valkey.io/commands/geosearchstore/ for more details. | ||
|
|
||
| Note: | ||
| When in cluster mode, both `source` and `destination` must map to the same hash slot. |
| When in cluster mode, both `source` and `destination` must map to the same hash slot. | ||
|
|
||
| Args: | ||
| destination (str): TThe key to store the search results. |
There was a problem hiding this comment.
| destination (str): TThe key to store the search results. | |
| destination (str): The key to store the search results. |
| If not specified, stores all results. | ||
| store_dist (bool): Whether to store the distance from the center as the sorted set score. Defaults to False. | ||
| - The distance is from the center of the circle or box, as a floating-point number, in the same unit specified for that shape. | ||
| - If set to False, the geohash of the location will be stored as the sorted set score. |
There was a problem hiding this comment.
Is geohash stored if store_dist set to True?
There was a problem hiding this comment.
If store_dist is false - the geohash is stored as score
If it's true- the distance is stored as score
There was a problem hiding this comment.
Can you please update the doc? It was unclear for me.
| store_dist: bool = False, | ||
| ) -> List[str]: | ||
| args = [key] | ||
| args = [*keys] |
There was a problem hiding this comment.
Optional:
| args = [*keys] | |
| args = keys |
| See https://valkey.io/commands/geosearchstore/ for more details. | ||
|
|
||
| Args: | ||
| destination (str): TThe key to store the search results. |
There was a problem hiding this comment.
| destination (str): TThe key to store the search results. | |
| destination (str): The key to store the search results. |
| "abc", "zxy", ListDirection.LEFT, ListDirection.LEFT, 1 | ||
| ), | ||
| redis_client.msetnx({"abc": "abc", "zxy": "zyx"}), | ||
| redis_client.geosearchstore( |
There was a problem hiding this comment.
nit: add a version check (see line 4804 for example)
| store_dist: bool = False, | ||
| ) -> int: | ||
| """ | ||
| Stores the results of a geospatial search into a destination sorted set. |
There was a problem hiding this comment.
we could be a bit more descriptive with the operation.
Example: zunionsstore:
https://github.com/aws/glide-for-redis/pull/1550/files#diff-37602a66f00251738190a1e0ebddd5d027022433028ab20ba728852a05abab7eR1978-R1980
Maybe:
Searches for members in a sorted set stored at `key` representing geospatial data within a circular or rectangular area and stores the result in `destination`. If `destination` already exists, it is overwritten. Otherwise, a new sorted set will be created.
To get the result directly, see `geosearch`.
There was a problem hiding this comment.
I agree, thanks!
| - If set to False, the geohash of the location will be stored as the sorted set score. | ||
|
|
||
| Returns: | ||
| int: The number of elements in the resulting sorted set. |
There was a problem hiding this comment.
| int: The number of elements in the resulting sorted set. | |
| int: The number of elements in the resulting sorted set stored at `destination`. |
| store_dist: bool = False, | ||
| ) -> TTransaction: | ||
| """ | ||
| Stores the results of a geospatial search into a destination sorted set. |
| - If set to False, the geohash of the location will be stored as the sorted set score. | ||
|
|
||
| Commands response: | ||
| int: The number of elements in the resulting sorted set. |
|
|
||
| @pytest.mark.parametrize("cluster_mode", [True, False]) | ||
| @pytest.mark.parametrize("protocol", [ProtocolVersion.RESP2, ProtocolVersion.RESP3]) | ||
| async def test_geosearchstore_by_box(self, redis_client: TRedisClient): |
There was a problem hiding this comment.
is it worthwhile adding tests to compare beyond the dateline? Or up near the poles.
Right now, our tests are focused around Europe, which isn't anywhere close to the Pacific coast datelines, and it close to the equator. I'm not sure how much we want to stress the system by doing geosearch around something like Tonga Island (-21.178986, -175.198242). If you do a large enough search, you should get islands on the other side of the meridian, like Fiji (-17.713371, 178.065033).
Of course, this kind of test is more testing Redis, rather than our API. So this is optional.
There was a problem hiding this comment.
maybe we can add it as a post GA small task
5f0c379 to
ae616a1
Compare
ae616a1 to
efc5f39
Compare
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.