graphile / crystal

🔮 Graphile's Crystal Monorepo; home to Grafast, PostGraphile, pg-introspection, pg-sql2 and much more!
https://graphile.org/
Other
12.63k stars 572 forks source link

Feature Request: Batch query support #634

Closed zopf closed 6 years ago

zopf commented 7 years ago

I'm submitting a ...

Some other GraphQL server implementations support a batch query protocol, whereby multiple GraphQL requests can be assembled into an array, the array is sent to the server, each of the requests is separately executed on the server, and their results are returned in one array to the client.

Example usage can be found here for Apollo: https://blog.graph.cool/improving-performance-with-apollo-query-batching-66455ea9d8bc

And an example implementation in Apollo server can be found here: https://github.com/apollographql/apollo-server/blob/3fecfcd3218c2f7f2c5ed59a351d71022944bc35/packages/apollo-server-core/src/runHttpQuery.ts#L65

This feature request proposes that PostGraphQL support a similar scheme, in order to allow clients to optionally make GraphQL requests to the PostGraphQL server in a batch mode, enabling multiple operations to be performed with a single HTTP request.

An example use case would be a client that needs to perform ten separate mutations (say, updating ten separate DB objects with new data). With batching, instead of sending ten separate HTTP requests to the PostGraphQL server, the client could send one larger HTTP request that contains an array of those ten mutations, and receive and array of ten results.

benjie commented 7 years ago

I definitely want to support this and it should be fairly easy to add. PRs welcome.

(In the mean time you can work around this by performing multiple mutations in the same GraphQL query:

mutation MultiCreate {
  post1: createPost(...) { ...Payload }
  post2: createPost(...) { ...Payload }
  post3: createPost(...) { ...Payload }
  post4: createPost(...) { ...Payload }
  post5: createPost(...) { ...Payload }
}
fragment Payload on CreatePostPayload {
  ...
}

)

benjie commented 7 years ago

Query batching

w9 commented 6 years ago

@benjie Does this workaround guarantee atomicity (my guess is not)? And if not, is there a work around that allows me to have the same atomicity as

INSERT INTO table (c1, c2) VALUES (1,2),(3,4),(5,6);

? Thanks!

w9 commented 6 years ago

To answer myself---I found the following solution works:

PostgresQL setup:

create type add_many_people_input as (
  name text,
  age numeric
);

create function add_many_people(
  inputs add_many_people_input[]
)
  returns void
language plpgsql security definer as $$
begin
  insert into person (name, age)
    select * from unnest(inputs);
end;

GraphQL query:

mutation {
  addManyPeople(
    input: {
      inputs: [
        {
          name: "John",
          age: 24
        },
        {
          name: "Jane",
          age: 23
        }
      ]
    }
  ) {
    clientMutationId
  }
}
wenzowski commented 6 years ago

Not sure if this is the right place to consider @export(as: "foo") support like Sangria has. Would be lovely to execute multiple dependent mutations in one go, without having to write volatile functions for every mutation chain.

benjie commented 6 years ago

Query batching support is available now via --enable-query-batching

barbalex commented 6 years ago

@benjie Sounds great. Where is that documented?

benjie commented 6 years ago

It's just that option for the CLI; or for the library enableQueryBatching: true.

As for the client side, for Apollo, you can use the BatchHttpLink link - something like:

import { ApolloClient } from 'apollo-client';
import { BatchHttpLink } from "apollo-link-batch-http";

const link = new BatchHttpLink({ uri: "/graphql" });

const client = new ApolloClient({
  link,
  // other options like cache
});

I've no experience using batching with other GraphQL clients.

barbalex commented 6 years ago

works like a charm, thanks @benjie

bgotthold-usgs commented 6 years ago

Works great with the example above, however, as soon as I provide some custom headers, postgraphile is returning a 'Must provide a query string' error. The same headers are working fine with httpLink on the Apollo side. Any insight?

benjie commented 6 years ago

Can you send details of what the exact network request is that's being sent? If you're not sure how to do so, follow the instructions in Step 1 here:

https://www.graphile.org/postgraphile/debugging/#step-1-check-youre-requesting-what-you-think-youre-requesting

Please send screenshots containing as much detail as possible.

bgotthold-usgs commented 6 years ago

Thanks for the quick response! screen shot 2018-10-15 at 9 03 15 am

the same request works fine without custom headers

screen shot 2018-10-15 at 9 07 38 am

sjmcdowall commented 6 years ago

Where is your Authorization header in the 2nd example ... that's not really a custom header..??

If you have turned on JWT processing, etc. somehow or in someway .. the lack or in this case, maybe the inclusion of the Auth header is causing the issue?

Anyway, it seems suspect.. :)

On Oct 15, 2018, at 10:09 AM, bgotthold-usgs notifications@github.com wrote:

Thanks for the quick response! https://user-images.githubusercontent.com/42218449/46959563-9416f780-d059-11e8-8f0e-e52904c2c1bd.png the same request works fine without custom headers

https://user-images.githubusercontent.com/42218449/46959744-ff60c980-d059-11e8-82d2-16074a93d2d9.png — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/graphile/postgraphile/issues/634#issuecomment-429892684, or mute the thread https://github.com/notifications/unsubscribe-auth/AB8M7eiYWAHrJ69tZAKWcq8h7Z_glfihks5ulKUpgaJpZM4Qklji.

benjie commented 6 years ago

The issue seems to be one of CORS:

screenshot 2018-10-15 18 13 58

Specifically the email-address header (which is not a standard header, and thus should be prefixed with an x-, but that's irrelevant) is not in the list of allowed headers, so the browser is preventing the CORS request taking place.

I suggest you disable PostGraphile's built in CORS support, and instead add your own cors middleware that you can configure however you need. This would involve using PostGraphile as a library. More details in these links:

https://expressjs.com/en/resources/middleware/cors.html

https://www.graphile.org/postgraphile/usage-library/#http-middleware

bgotthold-usgs commented 6 years ago

If you have turned on JWT processing, etc. somehow or in someway .. the lack or in this case, maybe the inclusion of the Auth header is causing the issue?

JWT processing is not turned on. Postgraphile was added after the fact. we are using a different service to verify our requests then pass them on to postgraphile.

bgotthold-usgs commented 6 years ago

@benjie Thanks for the response. It turns out the issue was defiantly one of CORS. I was sending access-control-allow-headers instead of request-headers. Weird that this did not raise any issues in non query-batching mode. For reference we are running PostGraphile as a library and using CORS middleware. Anyway batch mode is awesome!

alexisrolland commented 5 years ago

Hi @benjie, I've got one more question on Query Batching. Would you have an example of payload PostGraphile expects for batch requests? Or alternatively, would you have an example of payload sent by Appollo when sending batch requests? I'd like to try to reproduce that with in VueJS with vue-resource component. Thank you Alexis

benjie commented 5 years ago

Somehow missed your question. Batching is super simple, the input is an array of the normal JSON inputs, and the output is an array of the normal JSON outputs (and the input/output indexes match). That’s all there is to it really.

alexisrolland commented 5 years ago

I got the answer from you on Discord, no worry... I'm writing examples here. I'll try to do a PR on the documentation as discussed (if I find where query batching is documented in the repo).

Payload structure: [{query: "...", variables: {...}}, {query: "...", variables: {...}}, ...] Response body: [{"data": [...]}, {"data": [...]}, {"errors": [...]}, ...]

benjie commented 5 years ago

Exactly; you may want to add operationName to some of the inputs (that's the standard trifecta: query, variables, operationName).

Thanks @alexisrolland! 🙏

korenzerah commented 5 years ago

Hello, Does it apply to mutations too?

alexisrolland commented 5 years ago

Hello, Does it apply to mutations too?

Yes

korenzerah commented 5 years ago

I couldn't find an example of batched mutations or to run it on graphiQL.

alexisrolland commented 5 years ago

@korenzerah the payload structure of your http request is exactly the same for batches of queries or mutation. Cf. Previous example above:

Payload structure: [{query: "...", variables: {...}}, {query: "...", variables: {...}}, ...]

The only difference is in the query attributes of the dictionaries above, you should write a GraphQL mutation rather than a GraphQL query. Example:

[{query: "myMutation{createBlaBlaBla(input:{...}){...}}", variables: {...}}, ...]
korenzerah commented 5 years ago

Thanks for your replies. Is query batching less recommended than query merging? Because query batching does N requests to the database while query merging makes it in 1 request.

alexisrolland commented 5 years ago

Sorry I don’t know what you mean by query merging?

korenzerah commented 5 years ago

As @benjie mentioned:

mutation MultiCreate {
  post1: createPost(...) { ...Payload }
  post2: createPost(...) { ...Payload }
  post3: createPost(...) { ...Payload }
  post4: createPost(...) { ...Payload }
  post5: createPost(...) { ...Payload }
}
fragment Payload on CreatePostPayload {
  ...
}

That's query merging.

alexisrolland commented 5 years ago

Ok thanks I missed that previous post. I do not know what’s recommended between query merging or query batching. I have not tried query merging and I don’t know how it works but one of the characteristics of query batching is that each query is a single database transaction. PostGraphile would return a response (success or failure) for each query in the batch. Might be useful depending on what you try to achieve.

benjie commented 5 years ago

Every mutation field uses one query to do the mutation and then one separate query to render the results of the mutation; so the merging you showed would need at least 10 queries (not one).

I'd recommend query batching over query merging because static queries are always preferred over dynamic queries. With static queries we can cache the parsing and validation, whereas with dynamic queries (as in query merging) we have to parse and validate each time.