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

Allow the use of database offsets when getting connections #20

Closed
solher opened this issue Feb 12, 2016 · 4 comments
Closed

Allow the use of database offsets when getting connections #20

solher opened this issue Feb 12, 2016 · 4 comments

Comments

@solher
Copy link
Contributor

solher commented Feb 12, 2016

Currently the relay.ConnectionFromArray(data, args) method only allows "software" offsets/cursor.

If the client asks for the first two elements, we potentially query 10000 documents from the database before throwing away the 9998 we don't want.

It would be a good feature to allow the use of database offset methods like LIMITand OFFSET in SQL.

Something like that:

func(p graphql.ResolveParams) (interface{}, error) {
    // convert args map[string]interface into ConnectionArguments
    args := relay.NewConnectionArguments(p.Args)
    offset, limit := relay.ComputeOffset(args)

    ships := []interface{}{}
    if faction, ok := p.Source.(*Faction); ok {
        // get ship objects from current faction
        ships = FindRelatedShips(faction.ID, offset, limit)
    }

    // let relay library figure out the result given the list of ships for this faction
    return relay.ConnectionFromArray(ships, args), nil
}
@solher
Copy link
Contributor Author

solher commented Feb 12, 2016

There is an issue as a lot of databases doesn't support the backward pagination.

The simplest solution would be to sort the results backward before applying the limit/offset.
We would maybe need a third return boolean parameter reverse.
Or maybe use negative offset and limit values.

@sogko
Copy link
Member

sogko commented Feb 12, 2016

hi @solher thanks for taking your time to look into this project, appreciate it very much 👍🏻

If the client asks for the first two elements, we potentially query 10000 documents from the database before throwing away the 9998 we don't want.

So the example solution you gave seems about right and it would address your issue, except that current relay-go library is missing a couple of pieces that would help to make it work

  • relay.computeOffset: Actually there is already relay.getOffset().
    It translates a connection cursor string to an offset value that you can use to construct your query for your data source (SQL, Mongo etc)
    This is currently a private method. This should be exposed and make it available.
  • relay.ConnectionFromArray() assume that the input data is static or representative of the whole data collection. Thats why currently your implementation might have to get every data for it to return correct cursor Connection value. And you are right to think that this is not ideal.

The missing piece is actually relay.ConnectionFromSliceArray() introduced a couple of months back in relay-js.
This has not been ported to relay-go

{ConnectionFromSliceArray} effectively allows you to fetch the data for a subset of a connection, and pass that in with some metadata indicating where the subsequence starts and how long the total array would be if materialized, and get a connection object back.

I can't personally commit when these changes would be made available but PRs are greatly welcomed!

Hope this answers your question!

TODO:

  • Expose relay.GetOffset()
  • Port and expose relay.ConnectionFromSliceArray()
  • Secondary todo: Port latest changes from relay-js to relay-go (From 0.3.2 to 0.3.6)

@solher
Copy link
Contributor Author

solher commented Feb 12, 2016

I'm going to work on it ;)

@sogko
Copy link
Member

sogko commented Feb 13, 2016

Merged PR #22
Thanks @solher 👍🏻

@sogko sogko closed this as completed Feb 13, 2016
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

2 participants