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

Will the GEORADIUS support pagination? #3019

Closed
cvincent00 opened this issue Jan 18, 2016 · 25 comments
Closed

Will the GEORADIUS support pagination? #3019

cvincent00 opened this issue Jan 18, 2016 · 25 comments

Comments

@cvincent00
Copy link

I wonder if GEORADIUS would support pagination?

something like providing skip parameter:

redis> GEORADIUS Sicily 15 37 200 km COUNT 2 ASC
1) "Palermo"
2) "Catania"

redis> GEORADIUS Sicily 15 37 400 km COUNT 4 SKIP 2 ASC
1) "Vibo Valentia"
2) "Crotone"

If I use the geo commands in a wrong way, let me know and please don't mind.
Look forward to your kind reply.

@vkpandey82
Copy link

Pagination would be a critical feature for it to be used in "store locator" like applications

@antirez
Copy link
Contributor

antirez commented Feb 15, 2016

Hello, using a SKIP feature the command would need to compute the result set again and again every time. This is potentially very CPU intensive, so I wonder if we should instead support pagination via a STORE option that creates, as result, a sorted set indexed by distance (for the score).

This way one could model something like:

MULTI
GEORADIUS Sicily 15 37 200 km COUNT 2 ASC STORE geo_result_39204234
EXPIRE geo_result_39204234 1000
EXEC

The next calls will use ZRANGE or ZRANGEBYSCORE in order to paginate. Makes sense?

@bpo
Copy link
Contributor

bpo commented Feb 16, 2016

The STORE option here would be very useful to compose more complex operations. One could answer questions like "show me restaurants nearest the subway stations on the way home" (union of multiple stored georadius results).

Some new AGGREGATE options for ZUNIONINTERSTORE might also go well with this (e.g. to find ideal meeting locations between points).

@antirez
Copy link
Contributor

antirez commented Feb 16, 2016

Makes sense @bpo. However while trying to design this better I stepped into an issue. GEORADIUS can take options in order to report different things: distance, coordinates and so forth. If we use the STORE option we can actually only store the original score of the sorted set, so we would basically store the lat/long coordinate as the score.

The good thing about this, is that the result of STORE is at this point a valid geo index, which looks like is the way to go. However the problem with this approach, is that we cannot use ZRANGE* commands and get distance / coordinates. On the other hand, if we use GEORADIUS again against the subset of results we obtained, we can't iterate incrementally over them. Even if we change ZRANGE* in order to cope with this use case and do the decoding (or better, add a new command like GEORANGE), the scores would be randomly sorted being a mix of latitude and longitude. So looks like we have an issue...

Otherwise the STORE command should take options to use the distance as score, and encode the resulting elements as, Json or alike?

@bpo
Copy link
Contributor

bpo commented Feb 16, 2016

Otherwise the STORE command should take options to use the distance as score, and encode the resulting elements as, Json or alike?

What if GEORADIUS accepted multiple STORE options - i.e. potentially scoring the set by geo index and distance, etc..? In the common case where set members are IDs I wouldn't expect this to take much more space than json-encoding, and it feels a bit more in accord with the rest of the Redis API.

In many cases I think people would be ok with discarding the geo index in this operation, as they will still retain the original geo-encoded zset to look up element locations with - when iterating based on proximity, for example.

@antirez
Copy link
Contributor

antirez commented Feb 16, 2016

@bpo the problem is that we have just a score and an element, how to encode distance and other stuff? Or do you mean we could output multiple keys, one for STORE?

Btw your observations made me think that, usually, when you paginate you don't need location (you need that when plotting on a map), but element name, so the fact you can't paginate while having the lat/long info is not terrible perhaps.

However in order to start with a general API, I would probably support STORE directly with the ability to tell what to store, like:

STORE distance <key>
STORE geoindex <key>

And so forth. We could start with just a single store, and improve this over time.

@bpo
Copy link
Contributor

bpo commented Feb 16, 2016

@antirez that sounds great to me. I meant to suggest multiple outputs, something like

GEORADIUS ... STORE <index-key> <distance-key>

or

GEORADIUS ... STOREDIST <key> STOREIDX <key>

It's clearer to start with a single option as you have suggested. One could always use MULTI/EXEC to snapshot both if needed.

@antirez
Copy link
Contributor

antirez commented Feb 16, 2016

Thanks @bpo, totally makes sense, while writing the above comment I removed / retyped it multiple times since can't decide about STORE type VS STORETYPE as you suggested. Maybe it's better what you propose after all:

  • STORE stores and generates a valid geo index.
  • STOREDIST stores using distance as score.

Looks more Redis-ish?

@bpo
Copy link
Contributor

bpo commented Feb 16, 2016

I would be very happy with those options for storage. Reviewing the whole command - what do the WITHCOORD, WITHDIST, etc options mean when you add STORE ?

@antirez
Copy link
Contributor

antirez commented Feb 16, 2016

@bpo they would syntaxerr. Are you suggesting to overload those options to switch output? I thought about this, the problem I see is that WITHCOORD would be the default when STORE is used, but is not the default when STORE is not used, so may be kinda of confusing.

Alternatively, it is possible to implement it so that when no WITHCOORD nor WITHDIST are given, the resulting sorted set has scores all set to 0, in order to allow lexical pagination. This way the default will match.

@bpo
Copy link
Contributor

bpo commented Feb 16, 2016

@antirez A syntax error there makes sense to me - I think it gets too confusing to use WITHCOORD both to indicate "include coordinates in output" and "store members scored by coordinates". Alternately adding GEORADIUSSTORE would allow the storage-specific options to develop independently of the results returned to the client like ZUNIONSTORE, ZINTERSTORE, etc. I'm 👍 on any of these approaches.

@CloudMarc
Copy link

This is fabulous! I like the STORE and STOREDIST approach. In my use case, I'm doing an intersection of a GEORADIUS call with an unordered set. Might it be possible to also add STORESET ?

@antirez
Copy link
Contributor

antirez commented Feb 17, 2016

@CloudMarc I think you have a solution already, fortunately ZINTERSTORE works already with mixed data types, you can use both sets and sorted sets as input keys, so you do the intersection storing the result out to some tmpkey and ZRANGE tmpkey 0 -1. To make all this safe without leaving spurious keys:

MULTI
ZINTERSTORE tmpkey 2 myset myzset
ZRANGE tmpkey 0 -1
DEL tmpkey
EXEC

@cvincent00
Copy link
Author

Thanks for your focus. It is a awesome solutions for me. It will simplify my implement actually.

By the way, in my use case, I need certain quantity of members.
My current implement is that I query many times with increasing radius. And then I drop the items which have been returned by former requests.

I am wondering how to determine radius appropriately for ensuring performance due to population distribution.
Maybe I need to know how many members around here in order to determining radius.

antirez added a commit that referenced this issue Feb 18, 2016
@antirez
Copy link
Contributor

antirez commented Feb 18, 2016

We have an implementation into unstable if you want to check it. The new options are STORE and STOREDIST. They can be used with COUNT and the two orderings in order to store only a limited number of elements. Feedbacks welcomed. I'm testing it a bit more before merging into 3.2, and I'm writing unit tests of course.

antirez added a commit that referenced this issue Feb 18, 2016
@mp911de
Copy link

mp911de commented Feb 23, 2016

You can perform either STORE or STOREDIST with one operation, but not both with one command? If so, then the server should respond with ERR that both options can't be set at the same time.

@antirez
Copy link
Contributor

antirez commented Feb 23, 2016

You right! Good idea to trow an error.

@CloudMarc
Copy link

on unstable as of today, GEORADIUS with STORE followed by ZINTERSTORE works great, saves me lots of round trips - Thanks!

@l854434716
Copy link

redis 3.2.0 version has been released geographic indexing function, have This feature been out of beta ? can i use it in a production environment?

@itamarhaber
Copy link
Member

Yes.
On May 9, 2016 7:46 PM, "luozhi" notifications@github.com wrote:

redis 3.2.0 version has been released geographic indexing function, have
This feature been out of beta ? can i use it in a production environment?


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#3019 (comment)

@Danieth
Copy link

Danieth commented Jun 2, 2016

I noticed this issue was still open. I had a few questions:

  1. Is it still a possibility for GEORADIUS (or a very similar command) to store the resulting members into a Set instead of a Sorted Set? I'm not trying to be too picky about performance but I noticed ZINTERSTORE is much more expensive than SINTERSTORE. I also noticed that it's more difficult and potentially expensive to iterate over the members of a Sorted Set than a Set. My situation is similar to @CloudMarc - I'd just much prefer to avoid using a Sorted Set unless it makes sense, and semantically, it doesn't make sense for me to created a Sorted Set if I am going to just iterate over the members and completely ignore the scores. Perhaps this is just the Redis way of doing things?
  2. 0b6daf5#diff-c677ad7f1f09c412e2afe0355f0801ddL541 This section has the sort option causing sorting regardless of if sorting the data will have an effect on the output. If the STORE or STOREDIST options are used the sorted order doesn't matter at all. Perhaps the sort option is invalid if using STORE or STOREDIST? I might be misunderstanding this piece of code...
  3. The Redis documentation doesn't cover GEORADIUS STORE and STOREDIST options besides showing that they are possible options - could we document/formalize the result of using STORE and STOREDIST? I get the gist of it from these comments but documentation would be awesome.

I've used Redis before but never had the need to dive into the code/write more advanced queries. I'm not good at C++ but i'd be willing to try to do a pull-request to add some of this stuff.

@l854434716
Copy link

Which type of coordinate system is used by redis? wgs84? or someting else?

@antonioparisi
Copy link

any news on this?

JackieXie168 pushed a commit to JackieXie168/redis that referenced this issue Aug 29, 2016
@yoav-steinberg
Copy link
Contributor

yoav-steinberg commented Sep 14, 2021

Closing as this was already handled.

@yoav-steinberg
Copy link
Contributor

yoav-steinberg commented Sep 14, 2021

@Danieth

  1. Is it still a possibility for GEORADIUS (or a very similar command) to store the resulting members into a Set instead of a Sorted Set?

It doesn't make much of a difference, especially if you don't use STOREDIST (since it won't need to calculate anything special). Then you can use convert the sorted set to a regular set (see https://stackoverflow.com/questions/14297671/copy-a-redis-sorted-set-to-a-set/14299162 for example). Other than that, your suggest makes some sense, but I'm not sure it's worth the added complexity to the command.

  1. 0b6daf5#diff-c677ad7f1f09c412e2afe0355f0801ddL541 This section has the sort option causing sorting regardless of if sorting the data will have an effect on the output. If the STORE or STOREDIST options are used the sorted order doesn't matter at all. Perhaps the sort option is invalid if using STORE or STOREDIST? I might be misunderstanding this piece of code...

If COUNT is specified you'll need to sort anyway, buy I think you are right there's a potential optimization here. A PR would be welcome.

  1. The Redis documentation doesn't cover GEORADIUS STORE and STOREDIST options besides showing that they are possible options - could we document/formalize the result of using STORE and STOREDIST? I get the gist of it from these comments but documentation would be awesome.

Good point. I think this has since been done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests