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

Question: Non-Unique Cursors #93

Closed
jwdotjs opened this issue Aug 9, 2016 · 5 comments
Closed

Question: Non-Unique Cursors #93

jwdotjs opened this issue Aug 9, 2016 · 5 comments

Comments

@jwdotjs
Copy link

jwdotjs commented Aug 9, 2016

If a user wants to specify first and after to paginate through results, but orderBy is on a field that is non-unique, is there currently a solution or a proposed solution for handling that?

Example: users(first: 5, orderBy: FIRST_NAME)

If the first five users' names were Adam, the cursor would be "Mg==" for all of them I believe, so if I then specified after: "Mg==" it would grab a new set, but maybe not correctly paginating. If the next set is all "Adam" as well, it won't be able to paginate further since the after cursor would still be "Mg=="

Thank you for building this library!

@calebmer
Copy link
Collaborator

Yeah, this is a known bug and has a fairly simple solution. I'll try to do a hot fix sometime soon. I'm working on a large upgrade of PostGraphQL that fixes this and much more, but this bug really deserves a hot fix 👍

For more context on the issue: in order to implement cursor pagination correctly, PostGraphQL uses > and <. So first_name > "Adam" in this case. This fails when, as you mention, you have many "Adam"s 😊

The solution to this is adding the primary key implicitly to the sort/cursors and rewriting our comparison like this: (first_name, id) > ("Adam", 243).

@jwdotjs
Copy link
Author

jwdotjs commented Aug 10, 2016

That works. It might be good to allow the user to specify an array of orderBy's and inject id as the last in the set always (or whatever the primary key is).

Thanks so much for your work on the library! I have been doing a lot of research for a presentation on graphql and relay for a local meetup and the source code from postgraphql was of huge aid, whereas many other graphql libraries or tutorials only talked about concepts of the relay specifications at a high level.

@calebmer
Copy link
Collaborator

No problem man, glad I could help 😊

But yeah, as for the array of orderBys this is where union input types would be nice, graphql/graphql-spec#202. I’m not sure how well an array of orderBys would play with Relay though? I may do this in the next version. We’ll see…

@calebmer
Copy link
Collaborator

Ok, see #95

I’ll leave it for a bit to collect code review and then publish a patch.

@calebmer
Copy link
Collaborator

Released in 1.8.2 🎉

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

3 participants