-
Notifications
You must be signed in to change notification settings - Fork 1
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
Solve N+1 issue for remote relationships to connector #7
Comments
Compromise proposal: We implement the foreach capability for queries. We generate queries that look like this: query (n1_where: table_bool_expr, n2_where: table_bool_expr) {
n1: table(where: $n1_where) { foo bar }
n2: table(where: $n2_where) { foo bar }
} Where each variable set gets a root field. I believe this is how v2 works for remote schemas. The above proposal is admittedly a compromise, but GraphQL is poorly suited to solve this issue. V2 hasura does expose an api with collections where you could build giant where clauses and then split up the results, but that means we would have to
This doesn't get into the specifics of relationships and other issues. To me this feels like a lot of added complexity for minimal benefit, plus the added cost of making the entire thing more brittle. |
@sordina I'm not sure I understand the proposal. But I'll dive in anyway. If, by custom directive, you mean to require the remote GraphQL schema to provide details on the remote relationship capability, then I think that's not a strong idea. Adding constraints or requirements on the remote GraphQL endpoint would reduce adoption. (Ignore if I misunderstood the proposal.) In addition, It might be better to make it a more general solution that could work across REST/OpenAPI, GraphQL, gRPC, etc. Adding the details in the connector config (which defines the pattern for the Meaning...
The pattern required to do that has to be defined in the config, and it would be in some new, expanded properties in the existing remote relationship definition. |
When a remote relationship to this connector the classic N+1 issue arrises where N queries need to be performed for N records in a relationship.
For example if you distributed the chinook dataset across two postgres instances with one fronted by the PG connector and one fronted by a HasuraV2 instance and the GraphQL connector:
Then querying across the relationship like so, if there were 100 albums:
would query the graphQL connector 100 times.
The solution to this issue for other connectors has been to implement the
ForEach
capability which then allows a query against the full set of records in the relationship at once, deferring the choice of what to do to the connector.The current implementation of the GraphQL Connector does not support the ForEach capability and associated variables. However we can't simply add this and implement an optimised query because unlike SQL databases, upstream GraphQL schemas don't share singular and plural records behind the same interface (tables), and don't indicate relationships involved between fields.
In this example, imagine that the NDC query was sent to the connector:
We would hope that the upstream query would be something like:
but how does the connector know that
artists
corresponds toartist
and that theartist_id
variable set should be put into thewhere: {artist_id: {in:
filter.In addition to this, once the query is performed it has to be decomposed into a resultset rather than just forward the results of the naive query.
So,
There are many options for this, however my preference is:
See Also
The text was updated successfully, but these errors were encountered: