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

[Feature Request] A way to combine several identical queries instead of looping #45

Open
adrianocr opened this issue May 18, 2021 · 4 comments
Labels
enhancement New feature or request needs design Issues where we don't know exactly what we need yet; discuss on the issue before implementing

Comments

@adrianocr
Copy link

adrianocr commented May 18, 2021

I understand that this isn't production ready yet nor is it complete but I'd like to request a feature if you guys are taking those requests.

I have a small app that hits a graphql endpoint and returns ~50-60 product handles. I then need to individually query data about those products through their handle. The query for each individual product looks like this:

productByHandle(handle:$handle) { 
    variants(first: 35) { 
        edges { 
            node { 
                id 
                price 
            } 
        } 
    } 
}

If I were to loop through each one and run the query individually I would hit a rate limit unless I specified something like time.Sleep(500 * time.Millisecond) after each query. But that adds up and takes time.

So what I ended up doing was writing a function that combines all the queries into named/aliased queries like this:

query {
  product1: productByHandle(handle:"ABC") { 
      variants(first: 35) { 
          edges { 
              node { 
                  id 
                  price 
              } 
          } 
      } 
  }
  product2: productByHandle(handle:"ZYX") { 
      variants(first: 35) { 
          edges { 
              node { 
                  id 
                  price 
              } 
          } 
      } 
  }
  product3: productByHandle(handle:"XYZ") { 
      variants(first: 35) { 
          edges { 
              node { 
                  id 
                  price 
              } 
          } 
      } 
  }
  product4: productByHandle(handle:"CBA") { 
      variants(first: 35) { 
          edges { 
              node { 
                  id 
                  price 
              } 
          } 
      } 
  }
  ...
  ...
  ...
}

which allows me to send it as one query instead of 50-60 individual ones.

So essentially what I am proposing is a method for doing essentially the same with genqlient; to be able to combine several identical queries into one large one. Thoughts?

@benjaminjkraft
Copy link
Collaborator

benjaminjkraft commented May 18, 2021

Interesting idea. This is definitely something we've run into as well. We usually solve it by making a field something like productsByHandles(handles: [String!]!): [Product], but of course that requires server changes.

There are actually two ways to do this in GraphQL: one is what you describe, but you can also do "batch queries" which just send all the queries separately in the same HTTP request. I think the way Apollo does it is the de-facto spec, which some other libraries support (I'm not sure about specific ones). genqlient doesn't support this yet, but perhaps it could (added #46).

Then the question is whether genqlient can do something for you here. One thing you can do without special support (once we support fragments at all, see #8) is

fragment productFields on Product {
  variants(...) { ... }
}

query {
  product1: productByHandle(handle:"ABC") { ...productFields }
  product2: productByHandle(handle:"DEF") { ...productFields }
  product3: productByHandle(handle:"GHI") { ...productFields }
  ...
}

So then the question is: should genqlient be able to generate that for you? (Another way you could do it today is to autogenerate that file -- you could just write a little shell script or something.) In general I'm pretty hesitant to have genqlient manipulate the given query, or add special query syntax, if we can avoid it (see also #27), but this may point to the need for an extension API where one of the things it could do is to modify the queries before generating code.

In any case I'll leave this open to think about as we go -- thanks for the suggestion!

@benjaminjkraft
Copy link
Collaborator

benjaminjkraft commented May 28, 2021

I was thinking about this a bit more, because it is a problem we have as well at Khan. We have mostly solved it by the productsByHandles approach, but I've never felt it's ideal.

On the one hand, I think batch queries are philosophically a much better solution; and I think that homogeneous batch queries should be easy for geqnlient to support (that's not a promise, just a guess; see #46).

On the other hand, I think the way Apollo did them is inefficient in several ways: first of all you have to pass the operation-document several times (so the backend has to parse it several times, etc.); and then additionally the backend's resolvers are called several times instead of a get-many, which is especially bad if you use federation -- I believe the gateway will in that case not batch the requests to the backends. So it's fine for where you want to avoid several HTTP roundtrips, but doesn't really help if you want to make your backends more efficient.

That said, I think probably the right solution to this is to fix the problems of batching on the Apollo side, then use that. I'll start a discussion somewhere when I get a chance to write up the context.

@adrianocr
Copy link
Author

adrianocr commented May 28, 2021

On the topic of batch queries I think they could potentially be a solution but I think they present an extra bit of learning curve (if that even makes sense grammatically) and adds complexity. I'm not an expert on the subject so I'm not sure if there are other ways of doing batch queries but here's how Shopify does it: https://shopify.dev/tutorials/perform-bulk-operations-with-admin-api.

The query isn't even a query, it's a mutation. But on top of that you also need to then write code into your handlers to keep polling a specific address to check on the status of the operation. Only when the endpoint gives you a completed status can you actually access the data you were interested in to begin with.

So when you say genqlient may be able to add batch query functionality do you mean a similar approach to Shopify's and would genqlient also do the heavy lifting of polling the required endpoint for a completed status?

@benjaminjkraft
Copy link
Collaborator

benjaminjkraft commented May 28, 2021

It looks like the way Shopify does it is a bit different. The Apollo way is a bit simpler for the application code, it's basically just you stick a bunch of queries in a single HTTP request. You can read more in their documentation here: https://www.apollographql.com/docs/react/api/link/apollo-link-batch-http/. That's what I would plan to follow, although I think they do auto-batching and, at least to start, I would probably make it explicit.

@benjaminjkraft benjaminjkraft added enhancement New feature or request needs design Issues where we don't know exactly what we need yet; discuss on the issue before implementing labels Sep 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs design Issues where we don't know exactly what we need yet; discuss on the issue before implementing
Projects
None yet
Development

No branches or pull requests

2 participants