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

Handling ordering and filtering in connections #20

Closed
thenikso opened this issue Aug 28, 2015 · 9 comments

Comments

Projects
None yet
5 participants
@thenikso
Copy link

commented Aug 28, 2015

The need come from displaying the user a list of nodes that she should be able to sort/filter.

I'd like to get a better grasp on the philosophy behind Connections and how would one go in building a backend to support the {before,after,first,last} pagination along with ordering/filtering.

From graphql/graphql-spec#4 I understand that GraphQL per se doesn't really care on how you handle pagination/filtering/ordering. Hence I'm writing here even if I realize that this is not strictly in the Relay scope.

My general question is: how do you go in handling filtering and especially sorting the Relay way (if at all) and why?

Lets say we could query with a Relay server like:

currentUser {
  friends(first:10, after:"opaque-cursor", order:AGE) {
    edges: [ ... ]
  }
}

I guess my questions are:

  • would that connection make sense (in that example AGE would be an Enum value) or would be querying for a different firendsOrderedByAge connection be more aligned with Relay philosophy? Or what else?
  • say you want to combine multiple ordering, how would you go about that? I'm currently trying with ie: friends(order:[AGE,POSTCOUNT], ...)
  • as the API I'm currently trying to proxy via a Relay server does not map well with the Connection model, how would you go in building a backend API that does?
  • I'm trying to cut all the way to the SQL to at least make this work. say you get back {after_age, after_id} from your opaque cursor in the after parameter; would a pseudo-SQL like this make sense? SELECT * FROM a_table WHERE (age, id) > (after_age, after_id) ORDER BY age ASC, id ASC LIMIT first

Thank you all!

@devknoll

This comment has been minimized.

Copy link

commented Aug 28, 2015

Ordering with an enum argument seems right. An array of enums sounds good too if it makes sense for your data.

The connection model is primarily for pagination. If you don't need it, you can get away with just using a list. Otherwise, you will likely need to find a way to embed data into the cursor to help you request the next set of data. One solution for an append only list would be to simply have the item index as the cursor. Then you know what to fetch from the API by taking the cursor + how the requests are paginated. I think most well designed APIs that support pagination will also provide some kind of opaque cursor that you can also use directly. Keep in mind that it's also not required to support both forward and backward paging. I think most APIs only support forward anyway, which would restrict what you're able to expose through GraphQL.

@dschafer dschafer added the question label Aug 28, 2015

@dschafer

This comment has been minimized.

Copy link
Contributor

commented Aug 28, 2015

would that connection make sense (in that example AGE would be an Enum value) or would be querying for a different firendsOrderedByAge connection be more aligned with Relay philosophy? Or what else?
say you want to combine multiple ordering, how would you go about that? I'm currently trying with ie: friends(order:[AGE,POSTCOUNT], ...)

Yep, I would add a new argument (we use orderBy as the name at Facebook) that accepts a List of NonNull Enums, where the enum is NAME, AGE, etc. That's definitely in line with the Relay/GraphQL philosophy.

I would definitely recommend using a list instead of a singular, because in the case where you only have one thing to orderBy, you can pass that as a singular; because of http://facebook.github.io/graphql/#sec-Lists, these two are the same if orderBy accepts a list:

friends(orderBy:AGE)
friends(orderBy:[AGE])

In fact, the decision to make list coercion work this way was made specifically to enable that kind of orderBy behavior, and to allow us to always have orderBy be a list even in cases where we only need to sort by one thing.

as the API I'm currently trying to proxy via a Relay server does not map well with the Connection model, how would you go in building a backend API that does?
I'm trying to cut all the way to the SQL to at least make this work. say you get back {after_age, after_id} from your opaque cursor in the after parameter; would a pseudo-SQL like this make sense? SELECT * FROM a_table WHERE (age, id) > (after_age, after_id) ORDER BY age ASC, id ASC LIMIT first

How you'd do pagination definitely varies depending on the backend; that approach seems reasonable to me. In general, I would figure out what data you would need to fetch the next page, and then throw all of that into the cursor (we base64 cursors at Facebook to make it more clear that they are opaque, then the server just unbase64s them and uses the data to get the next page.

One solution for an append only list would be to simply have the item index as the cursor. Then you know what to fetch from the API by taking the cursor + how the requests are paginated. I think most well designed APIs that support pagination will also provide some kind of opaque cursor that you can also use directly. Keep in mind that it's also not required to support both forward and backward paging. I think most APIs only support forward anyway, which would restrict what you're able to expose through GraphQL.

👍

@thenikso

This comment has been minimized.

Copy link
Author

commented Aug 29, 2015

Thank you all very much! For me this is clear now.

To sum it up for new readers:

  • If you want to use Connection pagination with your own filtering/ordering, accept additional args such as orderBy for your connection field
  • Consider using a non nullable enum list for your orderBy arg
  • You might want to encode all filters and order fields in your Connection cursors to reproduce the correct edges before slicing where requested
  • Do not feel forced to support both after and before, choose based on how you plan your Relay server to be used
  • The way you fetch data from you backend is up to you and not related with Relay/GraphQL. Just make sure that you can retrieve all the data you need to perform the appropriate fetch from a Connection's cursor. You might find ROW_NUMBER useful if you are trying to hook up your Relay server directly to an SQL server that supports it.

Example connection field args setting:

...
args: Object.assign({
  orderBy: {
    type: new GraphQLList(new GraphQLNonNull(myOrderByEnum)),
  },
}, connectionArgs),
...
@mnpenner

This comment has been minimized.

Copy link

commented May 19, 2016

How about descending order? What's the consensus/best practice?

Maybe create another enum for ASC/DESC?

friends(order:[[AGE,ASC],[POSTCOUNT,DESC]], ...)

Is it possible then to make ASC the default so that we can write something like:

friends(order:[AGE,[POSTCOUNT,DESC]], ...)

?

@dschafer

This comment has been minimized.

Copy link
Contributor

commented May 20, 2016

We actually rarely run into this case at FB, and when we do we just define another ordering, so I don't necessarily have a suggestion based on experience here.

Your suggestion seems good, though; specifically, I would have orderBy take a [Ordering!], where Ordering is an InputObject:

input Ordering {
  sort: Sort!
  direction: Direction! = ASC
}

enum Sort { AGE, POSTCOUNT }
enum Direction { ASC, DESC }

seems right to me. You'd then do:

{
  friends(orderBy: [{sort: AGE}, {sort: POSTCOUNT, direction: DESC}])
}
@taion

This comment has been minimized.

Copy link
Contributor

commented Aug 29, 2018

@dschafer Is it intentional that the coercion above is not applied to variables? While the shorthand above is convenient when working with literals, it's a bit odd that the equivalent doesn't work when using variables – e.g. one can't use a String in place of a [String!].

@dschafer

This comment has been minimized.

Copy link
Contributor

commented Aug 29, 2018

@dschafer Is it intentional that the coercion above is not applied to variables? While the shorthand above is convenient when working with literals, it's a bit odd that the equivalent doesn't work when using variables – e.g. one can't use a String in place of a [String!].

Hm, interesting; you can't use a String in place of a [String], but if you pass a string literal in as the value of a [String], it seems like it should work (and testing on a couple of servers, it appears to):

Facebook internal Graphiql

(n.b. FriendsOrdering is a string enum with the valid sorts)

query Test(
  $name:String!
  $sort:[FriendsOrdering]
) {
  username(username:$name) {
    ... on User {
      friends(orderby:$sort first:2) {
        nodes {
          name
        }
      }
    }
  }
}

with variables

{
  "name": "dschafer",
  "sort": "name"
}

yields

{
  "data": {
    "username": {
      "friends": {
        "nodes": [
          {
            "name": "Aaron G"
          },
          {
            "name": "Aaron R"
          }
        ]
      }
    }
  }
}

Github Graphiql

query($ids:[ID!]!) { 
  nodes(ids:$ids) { 
    id
  }
}

with

{
  "id": "MDQ6VXNlcjI3NjAwMDU="
}

yields

{
  "data": {
    "nodes": [
      {
        "id": "MDQ6VXNlcjI3NjAwMDU="
      }
    ]
  }
}

(this also demonstrates another use of this: if you have a root field that takes a list of IDs in, you can use it for the single ID case as well (though of course the response will still be an array).

@taion

This comment has been minimized.

Copy link
Contributor

commented Aug 29, 2018

Yup – my assertion here is that, for convenience and flexibility, it should be permissible to do $ids: ID! above.

This also gives an easier migration path to move these things from scalars to lists.

@taion

This comment has been minimized.

Copy link
Contributor

commented Aug 29, 2018

BTW, the behavior as above for values is actually defined by the spec in http://facebook.github.io/graphql/June2018/#sec-Type-System.List.

I just posted an RFC to make the same allowance for variable types. I believe this adds a nice symmetry. graphql/graphql-spec#509

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.