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

No way to get requested fields of the object inside `resolve` #19

Closed
xslim opened this Issue Jul 3, 2015 · 26 comments

Comments

Projects
None yet
@xslim

xslim commented Jul 3, 2015

Let's say I have a Database with User entity with a lot of fields:

var Sequelize = require('sequelize');
var sequelize = new Sequelize('sqlite://database.db');

var User = sequelize.define('User', {
  name: Sequelize.STRING,
  email: Sequelize.STRING,
  otherField: Sequelize.STRING,
});

And I have a GraphQL:

var queryType = new GraphQLObjectType({
  name: 'Query',
  fields: () => ({
    user: {
      type: userType,
      args: { id: { type: GraphQLID } },
      resolve: (_, {id}) => User.findById(id)
    },
    users: {
      type: new GraphQLList(userType),
      resolve: (source, args) => {
        User.all(); // How to get the requested fields?
        //User.all({ attributes: args._requestedFields });
      }
    },
  })
});

And I do a GraphQL query to get name of all users

 curl -s -X POST http://localhost:8080/api/graph -d '{ users { name } }' | jq '.'

I don't want to fetch all the fields from the database. I want to know what fields to fetch...

Is there any way to get a list of fields that were requested in resolve method?

@devknoll

This comment has been minimized.

Show comment
Hide comment
@devknoll

devknoll Jul 4, 2015

Here's one idea... The 4th argument to resolve is the AST for the field. In your simple example, you could get the fields with something like:

resolve: (source, args, root, ast) => {
  var args = ast.selectionSet.selections.map(selection => selection.name.value);
  User.all({ attributes: args });
}

That would work, as long as you didn't have any fragments/spreads/etc. For more advanced cases, you could probably use the AST visitor utils. There's also a curious // TODO: provide all fieldASTs, not just the first field for that param too...

I believe that I heard that Facebook optimizes things like this internally, so hopefully Lee will show up and drop some wisdom 😄

devknoll commented Jul 4, 2015

Here's one idea... The 4th argument to resolve is the AST for the field. In your simple example, you could get the fields with something like:

resolve: (source, args, root, ast) => {
  var args = ast.selectionSet.selections.map(selection => selection.name.value);
  User.all({ attributes: args });
}

That would work, as long as you didn't have any fragments/spreads/etc. For more advanced cases, you could probably use the AST visitor utils. There's also a curious // TODO: provide all fieldASTs, not just the first field for that param too...

I believe that I heard that Facebook optimizes things like this internally, so hopefully Lee will show up and drop some wisdom 😄

@xslim

This comment has been minimized.

Show comment
Hide comment
@xslim

xslim Jul 4, 2015

Thanks for thd tip! I'll give it a try!

I believe getting requested fields in resolve will be needed not only for me)

xslim commented Jul 4, 2015

Thanks for thd tip! I'll give it a try!

I believe getting requested fields in resolve will be needed not only for me)

@leebyron leebyron added the enhancement label Jul 4, 2015

@leebyron

This comment has been minimized.

Show comment
Hide comment
@leebyron

leebyron Jul 4, 2015

Collaborator

@devknoll is on the money with this. This is an area that is not yet complete, hence the TODO in the source. There is a more complex case that we don't yet support, and I plan to investigate ways of doing so.

Here's a contrived, but valid query: we want the names of our friends and the birthdates of our friends.

{
  me {
    ...friendNames
    ...friendBirthdays
  }
}

fragment friendNames on User {
  friends { name }
}

fragment friendBirthdays on User {
  friends { birthdate }
}

GraphQL's execution engine will notice that these fragments overlap, and will fetch friends one time, and will fetch { name, birthdate } afterwards. However we're not yet sharing this information wholly to the resolve function.

Collaborator

leebyron commented Jul 4, 2015

@devknoll is on the money with this. This is an area that is not yet complete, hence the TODO in the source. There is a more complex case that we don't yet support, and I plan to investigate ways of doing so.

Here's a contrived, but valid query: we want the names of our friends and the birthdates of our friends.

{
  me {
    ...friendNames
    ...friendBirthdays
  }
}

fragment friendNames on User {
  friends { name }
}

fragment friendBirthdays on User {
  friends { birthdate }
}

GraphQL's execution engine will notice that these fragments overlap, and will fetch friends one time, and will fetch { name, birthdate } afterwards. However we're not yet sharing this information wholly to the resolve function.

@devknoll

This comment has been minimized.

Show comment
Hide comment
@devknoll

devknoll Jul 4, 2015

I wonder if the problem could be generalized by adding some metadata to fields to provide hints.

For a SQL backend, maybe you'd say if there's an access to first_name, include that field in the query. name would prefetch both first_name and last_name. friends would generate another query, using the hints beneath it.

Then at the start, you would recursively go through the tree of requested fields, generate all the queries that you'll need, execute them in parallel, and then turn the results into your business objects. And finally, run resolve on the root object (-- no idea how this would work with fragments).

I would love to know if you have a more elegant solution at Facebook, @leebyron 😄

devknoll commented Jul 4, 2015

I wonder if the problem could be generalized by adding some metadata to fields to provide hints.

For a SQL backend, maybe you'd say if there's an access to first_name, include that field in the query. name would prefetch both first_name and last_name. friends would generate another query, using the hints beneath it.

Then at the start, you would recursively go through the tree of requested fields, generate all the queries that you'll need, execute them in parallel, and then turn the results into your business objects. And finally, run resolve on the root object (-- no idea how this would work with fragments).

I would love to know if you have a more elegant solution at Facebook, @leebyron 😄

@leebyron

This comment has been minimized.

Show comment
Hide comment
@leebyron

leebyron Jul 7, 2015

Collaborator

@devknoll we have never tackled this problem of writing SQL queries from GraphQL queries at Facebook - we never write SQL queries directly in the product anyhow, so this has never come up, so it's fresh research for those trying to do this.

What you described of keeping a mapping between GraphQL fields and SQL fields is what @schrockn was considering doing as he's been toying with building this in the past. Given a GraphQL query you should be able to determine which fields need to be queried from which tables based on those mappings and assemble the right query.

Collaborator

leebyron commented Jul 7, 2015

@devknoll we have never tackled this problem of writing SQL queries from GraphQL queries at Facebook - we never write SQL queries directly in the product anyhow, so this has never come up, so it's fresh research for those trying to do this.

What you described of keeping a mapping between GraphQL fields and SQL fields is what @schrockn was considering doing as he's been toying with building this in the past. Given a GraphQL query you should be able to determine which fields need to be queried from which tables based on those mappings and assemble the right query.

@devknoll

This comment has been minimized.

Show comment
Hide comment
@devknoll

devknoll Jul 7, 2015

@leebyron Ah, I meant the more general problem of avoiding calling out to the database multiple times. For some reason I thought this was already optimized. Thanks for the detailed response though!

devknoll commented Jul 7, 2015

@leebyron Ah, I meant the more general problem of avoiding calling out to the database multiple times. For some reason I thought this was already optimized. Thanks for the detailed response though!

@leebyron

This comment has been minimized.

Show comment
Hide comment
@leebyron

leebyron Jul 7, 2015

Collaborator

One thing we have figured out at Facebook is a debounced query dispatcher with memoized caching.

When we issue a query from our application logic tier, we actually wait for the frame of execution to unwind before dispatching that query to a service tier. This way if any other queries are issued during the same frame of execution, all can be dispatched in a single go.

If later in the same process we issue a query for the same object, we can quickly fulfill that query by returning an from the in-memory cache.

Between these two optimizations, we have seen really efficient query dispatch and fulfillment between our app layer and service layer without requiring any change in how people (or more recently, GraphQL) actually perform the queries.

Does that help answer?

Collaborator

leebyron commented Jul 7, 2015

One thing we have figured out at Facebook is a debounced query dispatcher with memoized caching.

When we issue a query from our application logic tier, we actually wait for the frame of execution to unwind before dispatching that query to a service tier. This way if any other queries are issued during the same frame of execution, all can be dispatched in a single go.

If later in the same process we issue a query for the same object, we can quickly fulfill that query by returning an from the in-memory cache.

Between these two optimizations, we have seen really efficient query dispatch and fulfillment between our app layer and service layer without requiring any change in how people (or more recently, GraphQL) actually perform the queries.

Does that help answer?

@devknoll

This comment has been minimized.

Show comment
Hide comment
@devknoll

devknoll Jul 7, 2015

Absolutely, thank you.

devknoll commented Jul 7, 2015

Absolutely, thank you.

@leebyron

This comment has been minimized.

Show comment
Hide comment
@leebyron

leebyron Jul 7, 2015

Collaborator

Implementing this in JS is really easy as well. Something along the lines of (warning: coding directly in this editor):

export function sendQuery(str) {
  return new Promise((resolve, reject) => {
    if (queue.length === 0) {
      process.nextTick(dispatchQueue);
    }
    queue.push([str, resolve, reject]);
  });
}

var queue = [];

function dispatchQueue() {
  var toDispatch = queue;
  queue = [];
  yourBatchQueryMethodHere(toDispatch.map(([str]) => str)).then(results => {
    results.forEach((result, index) => {
      var [,resolve] = toDispatch[index];
      resolve(result);
    });
  });
}
Collaborator

leebyron commented Jul 7, 2015

Implementing this in JS is really easy as well. Something along the lines of (warning: coding directly in this editor):

export function sendQuery(str) {
  return new Promise((resolve, reject) => {
    if (queue.length === 0) {
      process.nextTick(dispatchQueue);
    }
    queue.push([str, resolve, reject]);
  });
}

var queue = [];

function dispatchQueue() {
  var toDispatch = queue;
  queue = [];
  yourBatchQueryMethodHere(toDispatch.map(([str]) => str)).then(results => {
    results.forEach((result, index) => {
      var [,resolve] = toDispatch[index];
      resolve(result);
    });
  });
}
@gyzerok

This comment has been minimized.

Show comment
Hide comment
@gyzerok

gyzerok Jul 14, 2015

@leebyron What you mean by yourBatchQueryMethodHere? Can you share an example of such method?

I'm currently want to batch GraphQL client to my server.

gyzerok commented Jul 14, 2015

@leebyron What you mean by yourBatchQueryMethodHere? Can you share an example of such method?

I'm currently want to batch GraphQL client to my server.

@mnylen

This comment has been minimized.

Show comment
Hide comment
@mnylen

mnylen Jul 14, 2015

@gyzerok, I implemented this in our api aggregation layer. See https://gist.github.com/mnylen/e944cd4e3255f4421a0b for example. Hopefully it helps.

mnylen commented Jul 14, 2015

@gyzerok, I implemented this in our api aggregation layer. See https://gist.github.com/mnylen/e944cd4e3255f4421a0b for example. Hopefully it helps.

@gyzerok

This comment has been minimized.

Show comment
Hide comment
@gyzerok

gyzerok commented Jul 14, 2015

@mnylen Thank you!

@leebyron

This comment has been minimized.

Show comment
Hide comment
@leebyron

leebyron Jul 16, 2015

Collaborator

@mnylen that's a great and terse example! @gyzerok the idea is that function is going to be very different depending on what backend storage system you're using :)

Collaborator

leebyron commented Jul 16, 2015

@mnylen that's a great and terse example! @gyzerok the idea is that function is going to be very different depending on what backend storage system you're using :)

@gyzerok

This comment has been minimized.

Show comment
Hide comment
@gyzerok

gyzerok Jul 16, 2015

@leebyron The point is I do not want batching for backend, but for frontend. The reason is I'm currently trying to make tool for Redux to achieve Relay-like functionality. All because I'm highly interested in Relay and already bored to wait its public release. Here is a dirty proof of concept.

gyzerok commented Jul 16, 2015

@leebyron The point is I do not want batching for backend, but for frontend. The reason is I'm currently trying to make tool for Redux to achieve Relay-like functionality. All because I'm highly interested in Relay and already bored to wait its public release. Here is a dirty proof of concept.

@leebyron

This comment has been minimized.

Show comment
Hide comment
@leebyron

leebyron Jul 16, 2015

Collaborator

Ah, then I misunderstood. GraphQL is the tool we use for batching requests from the frontend. Relay helps us build single GraphQL queries to represent all the data we need at any one point.

The example I explained above and @mnylen and @devknoll were interested in was how to debounce round trips to a backing storage from GraphQL type implementations.

Collaborator

leebyron commented Jul 16, 2015

Ah, then I misunderstood. GraphQL is the tool we use for batching requests from the frontend. Relay helps us build single GraphQL queries to represent all the data we need at any one point.

The example I explained above and @mnylen and @devknoll were interested in was how to debounce round trips to a backing storage from GraphQL type implementations.

@leebyron

This comment has been minimized.

Show comment
Hide comment
@leebyron

leebyron Aug 12, 2015

Collaborator

Closing this now since v0.3.0 will include access to all fieldASTs as well as fragments which makes this one step easier. Specifically is is now possible.

Collaborator

leebyron commented Aug 12, 2015

Closing this now since v0.3.0 will include access to all fieldASTs as well as fragments which makes this one step easier. Specifically is is now possible.

@leebyron leebyron closed this Aug 12, 2015

@mnpenner

This comment has been minimized.

Show comment
Hide comment
@mnpenner

mnpenner Aug 22, 2015

Sorry to jump in on a closed ticket, but I'm running 0.4.2 now. Is there a newer example of how we would get a list of all the requested fields?

mnpenner commented Aug 22, 2015

Sorry to jump in on a closed ticket, but I'm running 0.4.2 now. Is there a newer example of how we would get a list of all the requested fields?

@fson

This comment has been minimized.

Show comment
Hide comment
@fson

fson Aug 22, 2015

Contributor

@mnpenner The third argument to resolve is a GraphQLResolveInfo object now and it includes the fieldASTs property. So you can do

resolve(source, args, { fieldASTs }) {
  // access fieldASTs
}
Contributor

fson commented Aug 22, 2015

@mnpenner The third argument to resolve is a GraphQLResolveInfo object now and it includes the fieldASTs property. So you can do

resolve(source, args, { fieldASTs }) {
  // access fieldASTs
}
@Nexi

This comment has been minimized.

Show comment
Hide comment
@Nexi

Nexi Aug 26, 2015

Sorry to write on a closed ticket too, but @leebyron I can't understand how exactly this debounced query dispatcher aggregates the queries on the next processor tick.

if I have the following graphQL:

post(id: 3000) {
  author {
    name
  }
}

In the current graphql implementation, first "post" needs to be resolved and then, after the post's data is available (promise has been resolved), executor goes inside "author" type end resolves it. In other words, aggregation layer doesn't know about "author", before post has been resolved. In this case, how aggregation layer can combine "post" and "author" query in one query, when "post" needs to be fetched first in order to aggregation layer figures out we need "author" too.

Do I miss something here?

Nexi commented Aug 26, 2015

Sorry to write on a closed ticket too, but @leebyron I can't understand how exactly this debounced query dispatcher aggregates the queries on the next processor tick.

if I have the following graphQL:

post(id: 3000) {
  author {
    name
  }
}

In the current graphql implementation, first "post" needs to be resolved and then, after the post's data is available (promise has been resolved), executor goes inside "author" type end resolves it. In other words, aggregation layer doesn't know about "author", before post has been resolved. In this case, how aggregation layer can combine "post" and "author" query in one query, when "post" needs to be fetched first in order to aggregation layer figures out we need "author" too.

Do I miss something here?

@leebyron

This comment has been minimized.

Show comment
Hide comment
@leebyron

leebyron Aug 26, 2015

Collaborator

@Nexi The query debouncing technique only works if your backends supports a batch fetching (e.g. SQL select from myTable where id in ...), and only when there is inherent parallelism in your query. In your example, there is no inherent parallelism because as you rightly point out, you must first request the "post" before you can request the "author".

Collaborator

leebyron commented Aug 26, 2015

@Nexi The query debouncing technique only works if your backends supports a batch fetching (e.g. SQL select from myTable where id in ...), and only when there is inherent parallelism in your query. In your example, there is no inherent parallelism because as you rightly point out, you must first request the "post" before you can request the "author".

@Nexi

This comment has been minimized.

Show comment
Hide comment
@Nexi

Nexi Aug 26, 2015

@leebyron Got it! Thank you for your quick response :)

Nexi commented Aug 26, 2015

@leebyron Got it! Thank you for your quick response :)

@leebyron

This comment has been minimized.

Show comment
Hide comment
@leebyron

leebyron Sep 14, 2015

Collaborator

I should mention that https://github.com/facebook/dataloader was released today which expands upon my example code above. In fact, this issue was the original inspiration for writing this new repo!

Collaborator

leebyron commented Sep 14, 2015

I should mention that https://github.com/facebook/dataloader was released today which expands upon my example code above. In fact, this issue was the original inspiration for writing this new repo!

@bigblind

This comment has been minimized.

Show comment
Hide comment
@bigblind

bigblind Jan 1, 2016

I have a possibly stupid question, because I'm new to graphql, but it seems that any query with fragments and other bells and whistles could be rewritten to a query with just fields and subfields, so

{
  me {
    ...friendNames
    ...friendBirthdays
  }
}

fragment friendNames on User {
  friends { name }
}

fragment friendBirthdays on User {
  friends { birthdate }
}

could be rewritten as
{
me {
friends {
name
birthdate
}
}
}

It looks like the executor needs to think about queries in this way anyway, so could it modify the ast to look like this before passing it to resolve functions? I can see no reason why a resolve function should care about whether some field is in a fragment anyway.

bigblind commented Jan 1, 2016

I have a possibly stupid question, because I'm new to graphql, but it seems that any query with fragments and other bells and whistles could be rewritten to a query with just fields and subfields, so

{
  me {
    ...friendNames
    ...friendBirthdays
  }
}

fragment friendNames on User {
  friends { name }
}

fragment friendBirthdays on User {
  friends { birthdate }
}

could be rewritten as
{
me {
friends {
name
birthdate
}
}
}

It looks like the executor needs to think about queries in this way anyway, so could it modify the ast to look like this before passing it to resolve functions? I can see no reason why a resolve function should care about whether some field is in a fragment anyway.

@clintwood

This comment has been minimized.

Show comment
Hide comment
@clintwood

clintwood Jan 2, 2016

@bigblind ATM I use fragments to represent the fields from a specific backend store (i.e. fragment fields <=> store collection fields). That way when I compose my GraphQL query I know which store collections/tables need to be queried to compose the final set of fields for final output. Of course this is not mandatory, but useful...

clintwood commented Jan 2, 2016

@bigblind ATM I use fragments to represent the fields from a specific backend store (i.e. fragment fields <=> store collection fields). That way when I compose my GraphQL query I know which store collections/tables need to be queried to compose the final set of fields for final output. Of course this is not mandatory, but useful...

@jakepusateri

This comment has been minimized.

Show comment
Hide comment
@jakepusateri

jakepusateri Oct 16, 2016

@clintwood I made a library that handles fields, fragments, inline fragments, skip and include directives that may solve your problem: graphql-list-fields

jakepusateri commented Oct 16, 2016

@clintwood I made a library that handles fields, fragments, inline fragments, skip and include directives that may solve your problem: graphql-list-fields

@benjie

This comment has been minimized.

Show comment
Hide comment
@benjie

benjie Jan 16, 2017

For anyone else who lands here; fieldASTs was renamed to fieldNodes in v0.8.0

benjie commented Jan 16, 2017

For anyone else who lands here; fieldASTs was renamed to fieldNodes in v0.8.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment