graphql-spec icon indicating copy to clipboard operation
graphql-spec copied to clipboard

[RFC] exporting variables between queries

Open bbakerman opened this issue 7 years ago • 10 comments

RFC exporting variables between queries

This RFC calls for the ability to "export" variables from one query into another.

This allows a single server call to be made and outputs from one query to be fed into others as variables.

So given:

query A {
    hero 
    {
        id @export(as:"droidId")
        name 
        friends  @export(as:"r2d2Friends") 
        {
            name @export(as:"friendNames")
        }
    }
}
        
query B($droidId : String!) {
    droid (id : $droidId ) {
        name
    }
}

Output values from operation A would be exported as the variables named "droidId", "r2d2Friends", and "friendNames"

The operation B would be detected to be dependent on operation A and would be executed after operation A completes. It would receive the value of "droidId" as a variable from operation A

Multiple operation support

This RFC is dependent on https://github.com/facebook/graphql/issues/375, that is the ability to execute multiple operations in one server interaction

Variable arity

There are design questions around variable arity. In the example above the "droidId" exported variable can be thought of as a singleton value because the surrounding types are not lists types.

Where as "r2d2Friends" can be thought of as a list value because the surrounding types are list values and hence multiple exported variable values are possible.

One can imagine in certain cases some one might want map like semantics so that "the first / last" value of a list is considered a singleton value.

Naming could be used to indicate behaviour say, for example where plural export names imply a list while singlar variable names imply a singleton value. eg: "droidId" would always be considered a singleton value where as "friendNames" would always be considered a list value.

This is a great discussion point we think.

Consumer Supplied Variable Values

If the consumer caller supplies their own variable values, these will be passed to each operation. The exported variables will be placed into the variables map and will overwrite any existing variables with the same name key.

Exported Variable Value Validation

Validation of the variable values would follow the same rules as consumer supplied variable values.

Validation errors would be placed on each operation execution result

Current implementations

Sangria has this capability today https://sangria-graphql.org/learn/#batch-executor and graphql-java has a PR exploring the idea https://github.com/graphql-java/graphql-java/pull/808

These are based on the ideas presented in a Facebook presentation about capabilities used within Facebook today.

We would love to see the Facebook learnings represented back out to the greater graphql community.

bbakerman avatar Nov 08 '17 23:11 bbakerman

Thanks a lot for starting this discussion, @bbakerman! These are very good discussion points.

In my current implementation, I took a quite pragmatic approach. I realize that the decisions that I took might be suboptimal. So, the feature is marked as experimental ATM and my hope is to change semantics as soon as better options emerge:

  • Variable arity: For all @exports with the same name I merge the values in a list. When it comes time to give a value to the target query, I'm trying to adjust it to the type of the variable. So, for instance, if the exported variable is of type [Person] and query requires Person!, I will take the first element of the list.
  • Consumer Supplied Variable Values: if a name of the variable is the same (in @export and in user-supplied variable), I decided to merge them into a list and then make the same adjustments as described above
  • Exported Variable Value Validation: I rely on the standard execution mechanism. Every query executed in isolation and all variable-related errors are handled in the same way as during the normal query execution (so some query results might have something in errors array).

I also would like to add some point that I found quite interesting as I was working on it. In the following query:

query q1 {
  person(id: 1) {
    email @export(as: "emails") 
  }
}

query q2 {
  person(id: 2) {
    email  @export(as: "emails") 
  }
}
           
mutation SendEmails {
  notify(emails: $emails, message: "🍪 Free cookies! 🍪")
}

There are several things to consider and define semantics for:

  • $emails is exported twice. Should something like this be allowed? ATM I do allow it and merge variable values in a list. When it comes time to execute the SendEmails mutation, I will adjust the value of the variable, if necessary (according to the variable type)
  • Variable inference - in the presentation, I haven't seen any explicit variable declarations. So I assumed that some kind of type inference takes place. I have implemented the variable type inference in the current version, though it is:
    • strict - if the variable is used in multiple places, the type should match exactly. Inference takes place only if no explicit definition is present and the variable with the same name is exported somewhere.
    • optional - it is possible to completely disable it. In this case, variables must be always be defined explicitly
  • Variables are exported from 2 queries and used in the 3rd mutation. I wonder what you think about this pattern and whether it should be allowed. (I do allow it ATM)

@leebyron In your presentation you haven't touched on the implementation details. At this point, it would be very interesting to learn how you defined the discussed semantics in your implementation. It would be great if you could share this and your thoughts on these points :)

OlegIlyenko avatar Nov 09 '17 00:11 OlegIlyenko

Wow, I never noticed that in the presentation, there were no explicit variable declarations (from the link above):

image

I wonder if this could be simpler if we used declarations in both cases, for example:

query NewsFeed($ids: [ID!]) {
  feed {
    stories {
      id @export(as: $ids)
    }
  }
}

query StoryComments($ids: [ID!]) {
  stories(ids: $ids) { 
    # ...
  }
}

In that way, both operations are already syntactically valid and we remove the need for maybe-ambiguous type inference.

But, are there shortcomings to that approach?

rmosolgo avatar Nov 09 '17 15:11 rmosolgo

Just wanted to revive this conversation, as it would be a huge win for my team right now.

Would it be helpful to make the @export directive be explicit about whether the exported variable is a list or not? For instance, @export(as: "id") would export a single variable $id and throw an exception if it is declared again, and @export(into: "ids") would allow multiple declarations (or a single declaration inside a list type), and just shovel successive results into the list.

alexgenco avatar Jan 07 '19 23:01 alexgenco

Another thought: My team's current requirements could probably be satisfied in a single operation with multiple mutations inside it, rather than multiple operations. This could potentially simplify the export variable type declarations, which could go in a directive next to the normal declared variables of the operation. Something like:

mutation CreateAndFollowUser($input: UserInput!) @exported($userId: String) {
  createUser(input: $input) {
    user {
      id @export(as: "userId")
    }  
  }

  followUser(userId: $userId) {
    success
  }
}

Here are a couple limitations right off the bat:

  • This potentially adds overhead since the second mutation has to wait for the first to complete before executing.
  • This doesn't allow passing variables across queries/mutations/subscriptions.

I think the first limitation could be remedied by using @defer. For instance, the query could change to

  followUser(userId: $userId) @defer {
    ...
  }

And then the followUser response could stream out after the createUser response.

I'm not sure I have a remedy for the second limitation yet, but maybe we could punt on that until multiple operation support makes it to spec? 🤷‍♂️

alexgenco avatar Jan 09 '19 23:01 alexgenco

I like the concept of the @export directive. One question that comes to mind, though: the examples within this issue all deal with simple variable passing, meaning not complex input types. @alexgenco, correct me if I'm wrong, most of the mutations we deal with only take complex input objects.

input ChargePaymentMethodInput {
  clientMutationId: String
  paymentMethodId: ID!
  transaction: TransactionInput!
}
mutation TokenizeAndChargePaymentMethod($tokenizeInput: TokenizeCreditCardInput!, $chargeInput: ChargePaymentMethodInput!) {
  tokenizeCreditCard(input: $tokenizeInput) {
    paymentMethod {
      id @export("paymentMethodId")
    }
   }
   chargePaymentMethod(input: $chargeInput) {
     transaction {
       id
       status
     }
   }
}

How were you thinking of passing the exported paymentMethodId variable to ChargePaymentMethodInput?

cmonty avatar Jan 10 '19 03:01 cmonty

@export(into: "ids") would allow multiple declarations (or a single declaration inside a list type), and just shovel successive results into the list.

Not sure how the syntax would work, but #517 deals with running a given query with a list of input parameters.

mathstuf avatar Jan 10 '19 14:01 mathstuf

@cmonty I think you basically have to inline all inputs down to the level of the exported variable, but anything deeper you could still include as an operation variable. So your example would be:

mutation TokenizeAndChargePaymentMethod(
  $tokenizeInput: TokenizeCreditCardInput!,
  $transactionInput: TransactionInput!
) @exported($paymentMethodId: String) {
  tokenizeCreditCard(input: $tokenizeInput) {
    paymentMethod {
      id @export(as: "paymentMethodId")
    }
  }
  chargePaymentMethod(input: {
    paymentMethodId: $paymentMethodId,
    transaction: $transactionInput
  }) {
    transaction {
      id
      status
    }
  }
}

alexgenco avatar Jan 10 '19 16:01 alexgenco

i wanted to share a couple of thoughts on this

Topic 1: Determining the type of an exported variable

I think it is better to statically determine type of an exported variable in the following way:

case 1

if the graph path to the exported field contains any list type field in it then the type should be a list of the exported field type.

For example, in

query A {
    users 
    {
        email @export(as:"emails")
    }
}

emails would have type [String] since the path to the exported field contains one or more list type in it: users:[User] -> email:String

case 2

otherwise, it is just has the type of the exported field.

For example, in

query A($userId: ID) {
    user(userId: $userId)
    {
        email @export(as:"email")
    }
}

email would have type String since the path to the exported field does not contain any list in it user:User -> email:String

case 3

if multiple @exports with the same id are to be supported, then the type of of the exported fields should be the same and it will be a list of the root type.

For example, in

query A($userId1: ID) {
    user(userId: $userId1)
    {
        email @export(as:"emails")
    }
}
query B($userId2: ID) {
    user(userId: $userId2)
    {
        email @export(as:"emails")
    }
}

emails would have type [String] because even though the path to the exported fields does not contain any list in it, it's user:User -> email:String in both cases, it is used more than once.

later when using this variable as an input to another operation strict type matching would be used rather than deciding the casting method here.

Note: I didn't put to many thoughts on nullablity but i think it can be inferred too. Note 2: I suggest the usage described in case 3 is forbidden in favor of the @collect(all: "email1 email2", as: "emails") usage directive suggested in topic 2. Note 3: the type could be also explicitly declared, specially if case 3 is eliminated.

Topic 2. Using the exported variable

The examples that export a list of ids i've seen so far force the mutation that takes the them as an input to be prepared for this, ie: to take a list of ids. And while that may be more efficient in some cases, it forces the implementor of the schema to provide such mutation, rather than letting the client make use of existing mutations that take a id. I believe it would be better if we benefit of @export more transparently, without application code changes. So I came up with this idea: a @expand directive that can expand the list values.

For example, if we have a query to collect emails but a mutation that takes as input only send one email at a time clients could use it like this:

query A {
    users 
    {
        email @export(as:"emails")
    }
}
mutation B {
    sendEmail(email : $email, subject: "hello") @expand(all: "emails", as: "email") {
        status
    }
}

In addition to this, other directives could be provided, such as:

  • @extract(from: "emails", single: "email") to get a single element (and fail if more than 1).
  • @extract(from: "emails", first: "email") to get the first element. (see note)
  • @extract(from: "emails", last: "email") to get the last element. (see note)
  • @extract(from: "emails", any: "email") to get any element. (see note)
  • @collect(all: "email", as: "emails") to make the element into a list (not very useful with just one item since auto-boxing from single inputs to lists already take place)
  • @collect(all: "email1 email2", as: "emails") to make multiple elements into a list.

Some uniqueness support could be provided:

  • @expand(unique: "emails", as: "email") using the "unique" attribute instead of "all"
  • @collect(unique: "email1 email2", as: "emails"), again, "unique" instead of "all"

Note: Strictly speaking the collected values will sometimes be a List and other times be a Bag (like an unordered Set with duplicates) depending on when the export is declared, for example:

  • if the export comes from two queries or two fields that could be resolved in parallel
  • if the are multiple List fields in the path to the exported field, which could also be spawned in parallel.

It may not be worth doing, but ideally whether a variable is a bag or a list would be tracked by the server executing the query (not necessarily in the type system), and it should prevent "first" and "last" to be extracted from bags, allowing only "any" and "single" for those cases.

Topic 3: Value pairs

Edit: Now I believe this is an overkill for a corner case, but sharing anyway.

Most of the time just a single or list of identifiers is exported but there are cases when identifier pairs may be needed. For example

query A {
    friendships(filter: { age: { gt: 10 }})
    {
        friend1 {
          id @export(as: "ids1")
        }
        friend2 {
          id @export(as: "ids2")
        }
    }
}
mutation B {
    upgradeFriendship(id1: $ids1, id1: $ids2) { # how should the separate lists be combined??? 
        status
    }
}

there is no clear to specify this way imo. To do this maybe it would be better to be able to export not only leaf types but also object types.

query A {
    friendships(filter: { age: { gt: 10 }}) @export(as: "oldFriendships")
    {
        friend1 {
          id @export(as: "ids1")
        }
        friend2 {
          id @export(as: "ids2")
        }
    }
}
mutation B {
    upgradeFriendship(id1: $oldFriendship.friend1.id, id1: $oldFriendship.friend2.id) { @expand(all: "oldFriendships", as: "oldFriendship")
        status
    }
}

although since dots are not allowed in variable names either that is changed or this will need something like:

mutation B {
    upgradeFriendship(id1: $id1, id1: $id2) { @expand(all: "oldFriendships", as: "oldFriendship") @unwrap(path: "oldFriendship.friend1.id", as:"id1") @unwrap(path: "oldFriendship.friend2.id", as:"id2")
        status
    }
}

The type of the $oldFriendships variable here is unclear to me, it would a synthetic type that matches the structure of selected fields of the object type. At least in this form, it introduces an order in which the directives must be resolved.

Topic 4: Mutations in any object type

I've seen comments from people already doing this. People create fields in their object types that are actually mutations (and act as object oriented design methods). Something like this:

query A {
    friendships(filter: { age: { gt: 10 }})
    {
        friend1 {
          id
        }
        friend2 {
          id
        }
        upgradeFriendship
    }
}

It seems somewhat elegant but from what I can tell it violates the spec, object fields should not have side effects. And it is also unclear by looking at the query. It doesn't solve every case that the @export directive solves, but it does solve it for some cases. So I can't help to wonder whether this is something worth considering for the spec. Some things to consider would be:

  • Do we really want the graphs to be OO models?
  • What is the correct execution order?
  • Those object fields that are actually mutations would need some syntax to indicate it (although a directive would do it too), eg:
type Friendship {
   friend1: User!
   friend2: User!
   mutation upgradeFriendship : UpgradeFriendshipResult!
   # or the uglier alternative:
   upgradeFriendship : UpgradeFriendshipResult! @mutation
}
  • The queries should indicate that clearly. Eg:
query A {
    friendships(filter: { age: { gt: 10 }})
    {
        friend1 {
          id
        }
        friend2 {
          id
        }
        mutation upgradeFriendship
        # or the uglier alternative:
        upgradeFriendship @mutation
    }
}

Topic 5: How much server side scripting do we want?

This is no specific proposal, just a reflection. The more these directives are added the more we lean towards the query language becoming a scripting language that is executed on the server side (although never turing complete). We can always come up with scenarios where we require a chain of query/mutation execution to be followed or skipped or modified based on an arbitrary logic. Having @export support and being able to combine that with @include or @skip seems ok. But what is a reasonable limit for their inclusion in the spec? Are the ideas behind my proposed @expand and @extract and @collect beyond that limit? Is everything under topic 3 and @unwrap beyond that limit?

enriquedacostacambio avatar Jan 17 '20 22:01 enriquedacostacambio

Any update on this?

Fintasys avatar Mar 17 '21 14:03 Fintasys

Hope this feature is prioritized as it has serious performance benefits. Bump!

elleryfamilia avatar May 06 '22 19:05 elleryfamilia