Enhancement: request required selection sets only for subschemas within a given operation

[gmac]

Similar to stitching resolvers, it’d be super handy if merged type config selectionSet could be defined as a function in addition to a string. Then, similar to Federation @requires, a dynamic mapping could be made of hints to select based on the request selection. This capability could even be turned into a standard helper like forwardArgsToSelectionSet.

[yaacovCR]

Are you able to spell out a more in-depth example of what you are trying to achieve?

[yaacovCR]

Actually, I think I get it, but possibly it might make more sense to pass th parameter to the merge resolver function

[yaacovCR]

Which is actually I think already available under info.fieldNodes which is guaranteed to have the same arguments

[yaacovCR]

Actually, I don't think that is what you are referring to.

Definitely a working example to help discuss would help.

One issue is that to optimize delegation planning, a lot of things are pre-calculated build time

[gmac]

To elaborate, for large models with lots of ID fields that crossover between services, currently the only option is to define a large selectionSet that contains every possible hint field that might be relevant to any other subservice. This is not ideal because the hint fields may have costs associated with them.

So, it would be useful if —like resolvers— the selectionSet could be a function that accepts a representation of the query as an argument, and would return a dynamic selectionSet with the specific hints it will need to send to other services.

This is more of a roadmap feature than a pressing need. It would offer a lot of flexibility if the feature existed. That said, it’s also fine if you have no intentions of supporting this and want to close it out.

[gmac]

Also if this is at odds with query planning, it’s totally fine to close this out. The general idea stems from the premise of federation where the set of model hints requested will be derived from the actual fields being queried. While tools doesn’t need anything remotely as sophisticated as the federation directive API, allowing selectionSet to be a function would provide a hook for doing some sophisticated logic there when/if needed.

[yaacovCR]

I think it's reasonable to work on that but I do not think that selection sets need to be functions for that.

Rather, that behavior should be integrated into improvements with query planning

[gmac]

That makes sense, and would be awesome! Honestly, the features that are there now are great, I don’t ever mean to imply otherwise. If my suggestions ever get annoying by all means feel free to say so :) Also, I should probably use a discussion thread for these sorts of conversations rather than a formal issue. Will do better about that!

…

On Mon, Aug 3, 2020 at 3:31 PM Yaacov Rydzinski ***@***.***> wrote: I think it's reasonable to work on that but I do not think that selection sets need to be functions for rather that that should be integrated into improvements with inquiry planning — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <https://github.com/ardatan/graphql-tools/issues/1874#issuecomment-668202098>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFRROFQCWKYAJI45QWEJETR64GAHANCNFSM4PTIFR5Q> .

[yaacovCR]

If you like the work we have done here so far please feel free to sponsor myself and @ardatan among others!

https://github.com/sponsors/yaacovCR
https://github.com/sponsors/ardatan

Please feel free to open up another issue or discussion in terms of the trade-offs between pre-calculating required selection sets and not over fetching...

[yaacovCR]

And thank you for your interest and comments, it is not annoying in the least, rather they are very helpful and encouraging.

[yaacovCR]

I think this is a simple as changing the merging selection sets to be by field rather than by type.

Each field is from only a subset of subschemas.

[gmac]

By-field would be pretty comprehensive. If there was a mapping such as

{
products: “{ productIds }”,
venues: “{ venueIds }”,
}

That would allow the granularity of something like the federation @require directive. That said, not every model would require field-level granularity, and when fields are used, all fields may need to select a common top-level key like id... so selectionSet still seems useful as the basic interface, with an option to enhance for field-level additions.

New streamlined stitching docs are in the works!

[yaacovCR]

Basically, I'm still not understanding exactly how this would work...

because of that, I am not yeah totally sure that I disagree, which is why I seem to be refusing to let this conversation die...

Returning to your example with shippingEstimate, I guess I see that those arguments are optional?

That seems like a poorly designed schema which requires optional arguments depending on the fields requested... Is that a common pattern?

[gmac]

Sorry, I've been mobile-only and haven't been able to put together good code snippets. So, a couple things up front...

1. That original federation demo from apollo is fairly contrived and not a great example.
2. The recreation test suite here doesn't really recreate it. Case of point: the original inventory service has no dedicated queries, while this example cheats and adds a dedicated inventory query rather than using virtual records (I'll explain what I mean by that below).

That said, type merging DOES do real federation (which is actually a very general-purpose GraphQL pattern outside of Apollo's specific distribution of it... technically "federation" is just Apollo's name for it). Here's a slightly better example:

const { makeExecutableSchema } = require('@graphql-tools/schema');
const { stitchSchemas } = require('@graphql-tools/stitch');
const { graphql } = require('graphql');

const products = [
  {
    upc: '1',
    name: 'Table',
    price: 899,
    weight: 500
  },
  {
    upc: '2',
    name: 'Couch',
    price: 1299,
    weight: 1000
  }
];

const productsSchema = makeExecutableSchema({
  typeDefs: `
    type Product {
      upc: String!
      name: String!
      price: Int!
      weight: Int!
    }
    type Query {
      _productsByUpc(upcs: [String!]!): [Product]!
    }
  `,
  resolvers: {
    Query: {
      _productsByUpc: (_root, { upcs }) => upcs.map(upc => products.find(product => product.upc === upc) || null),
    }
  },
});

const carriers = [
  { id: 1, name: 'USPS', maxWeight: 10, },
  { id: 2, name: 'FedEx', maxWight: 999 },
  { id: 3, name: 'Freight', maxWight: Infinity },
];

const inventorySchema = makeExecutableSchema({
  typeDefs: `
    type Product {
      shippingEstimate: Int!
      carrier: Carrier!
    }
    type Carrier {
      id: Int!
      name: String!
    }
    
    # implement the import part of the federation spec, following their strange naming:
    # https://www.apollographql.com/docs/apollo-server/federation/federation-spec/
    scalar _Any
    union _Entity = Product
    type Query {
      _entities(representations: [_Any!]!): [_Entity]!
    }
  `,
  resolvers: {
    Product: {
      shippingEstimate(product) {
        if (product.price > 1000) {
          return 0; // free for expensive items
        }
        return Math.round(product.weight * 0.5) || null; // estimate is based on weight
      },
      carrier(product) {
        return carriers.find(carrier => product.weight <= carrier.maxWight)
      }
    },
    Query: {
      _entities(_root, { representations }) {
        // each representation is a plain object with a "__typename"
        // and dependent fields selected from other subservices, ie:
        // { __typename: "Product", weight: 50, price: 500 }
        // these representations are resolved as virtual records within this service
        return representations;
      },
    },
  },
});

const stitchedSchema = stitchSchemas({
  subschemas: [
    {
      schema: inventorySchema,
      merge: {
        Product: {
          selectionSet: '{ weight price }',
          fieldName: '_entities',
          key: ({ weight, price }) => ({ weight, price, __typename: 'Product' }),
          args: (representations) => ({ representations }),
        }
      }
    },
    {
      schema: productsSchema,
      merge: {
        Product: {}
      }
    }
  ],
  mergeTypes: true,
});

graphql(stitchedSchema, `
  query {
    _productsByUpc(upcs: ["1", "2"]) {
      name
      shippingEstimate
      carrier {
        name
      }
    }
  }
`).then(({ data, errors }) => {
  console.log(JSON.stringify(data, null, 2));
});

The main thing to note here is that inventory service has no database concept of a Product – there's no way to lookup a Product by ID from inventory service because it has no product data (the apollo example unnecessarily adds one, which dilutes the example). Federation works by passing around virtual records: they're objects that have a __typename and key fields pulled from other services, for example:

{
  representations: [
    { __typename: "Product", weight: 100, price: 500 },
    { __typename: "Product", weight: 350, price: 100 },
  ]
}

The gateway builds that representations argument and sends it off to a subservice (via the federation spec), which can then resolve typed data using those stubs as virtual records. In this way, Inventory service can contribute fields to the Product type even though it has no database concept of a Product and thus no ability to query for one.

All told, this pattern is really powerful and it already works perfectly in type merging. The one tiny inconvenience at the moment is that the merge config selectionSet is a constant, so you always have to fetch every possible field hint rather than just the ones needed for a query. That was my original motive for pitching the ability to build a dynamic selection set there.

[yaacovCR]

Thanks for fleshing out the example. Powerful maybe, but not really well designed GraphQL (as I see it), as the inventory service is advertising data it cannot reliably deliver because the necessary arguments to the Product entity are optional via use of the Any scalar when they should be required by being typed or arguments to the shippingEstimate field.

Thanks again for adding color to the example and explaining more of your motivation. I am not eager to support this, because I think of it as an antipattern, but I suppose we have to because it is syntactically valid!

[yaacovCR]

Taking the other side of the argument, I suppose Apollo would argue that what prevents it from being an antipattern are the directives themselves which teach clients to send what is needed.

I suppose instead of inventing our own syntax, the best thing to do is to introspect the directives and support that spec.

[yaacovCR]

I guess there is still a trade-off that is worth pointing out at least between calculating the required fields and query time versus at build time, with the ladder faster at query time but requiring over fetching

[yaacovCR]

Meaning, a common pattern outside federation?

[gmac]

Your criticism about the weak typing in the federation pattern is fair. It is definitely designed around scale and automation. In the context of type merging when you’re declaring a service per type, you could certainly build a typed service rather than generic “_entities“ service... That’d make much more sense in the context of that inventory service, which has only one type (if I was building that for real, I most definitely wouldn’t use an abstract there; that was very much demonstrating federation). On the flip side, that federation spec now has an official community with tools available that auto-create subservices that implement its spec, so I might still choose to use the _entities service pattern depending on what other apps I was integrating with and what they supported. Either way, IMHO, Tools does not need any official capacity to read/understand federation directives from a schema. I wouldn’t expect Tools to be a feature-complete alternative to an Apollo gateway — I can just use Apollo for that. Honestly, I think it’s present capabilities to work with federation are perfect: it can do it but it doesn’t promise anything about it. The original “selectionSet as a function” idea that this whole conversation stems from was just another generic hook that would give developers some options if they needed them without really promising/supporting anything official with regard to federation. Anyway, good talk here. :)

…

On Wed, Aug 5, 2020 at 5:25 AM Yaacov Rydzinski ***@***.***> wrote: Taking the other side of the argument, I suppose Apollo would argue that what prevents it from being an antipattern are the directives themselves which teach clients to send what is needed. I suppose instead of inventing our own syntax, the best thing to do is to introspect the directives and support that spec. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1874 (reply in thread)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFRROBXQGT6FTUWFRLTD23R7EQPVANCNFSM4PUPK75Q> .

[yaacovCR]

I have to set up an example, but seeing as we already support selection sets by field on gateway, is this as simple as leaving upc and weight off of the key field for that subschema, and adding it to the global gateway selection set hints, even though there is no stitching resolver.

Putting aside my ill-informed ideas about federation, we have to support something like this, because even if the field was shippingEstimate(weight: Int, upc: String) as I am advocating, it wouldn't make sense to have the merged field weight be both a field for product and an argument.

[gmac]

Look forward to seeing your example. I can’t quite picture what you’re describing there. Regarding field-level resolvers in the gateway, I will say that is a stitching pattern my organization is actively moving away from due to the maintenance of implementations being spread across services and the gateway. Federation puts _everything_ in subservices and nothing in the gateway (full automation), which is good for separation of concerns but really difficult to orchestrate the complex directive API across apps of different languages. Type merging is offering a really interesting new midpoint that declares some global service config in the gateway but otherwise can keep type resolvers in their respective services of concern. The thing I’m finding most attractive is that merging lets you use both stitching and federation-like patterns when and where they each make sense based on circumstance.

…

On Wed, Aug 5, 2020 at 11:48 AM Yaacov Rydzinski ***@***.***> wrote: I have to set up an example, but seeing as we already support selection sets by field on gateway, is this as simple as leaving upc and weight off of the key field for that subschema, and adding it to the global gateway selection set hints, even though there is no stitching resolver. Putting aside my ill-informed ideas about federation, we have to support something like this, because even if the field was shippingEstimate(weight: Int, upc: String) as I am advocating, it wouldn't make sense to have the merged field weight be both a field for product and an argument. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1874 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFRROD37OLJFGGQTFRG22DR7F5ORANCNFSM4PUPK75Q> .

[yaacovCR]

#1888

[yaacovCR]

Well, that didn't work. Wishful thinking I guess!

[yaacovCR]

Ahah. The selection sets are added by field but because the "proxiability" from one schema to the next is preprocessed from the smaller key, delegation to inventorySchema is fired off prior to those fields arriving. So we are back to the question of larger, more expensive keys or dynamically determining proxiability in terms of overall performance.

Not sure I have a great handle on a real world example, but makes sense in general to not cause wasted cycles in the backing services in favor of more cycles on the gateway...

[gmac]

I have several real-world examples, although in those cases the keys happen to be extremely cheap cached values where over selecting would not be a problem. All that’s to say — this is more of a principled design discussion than anything. What exists is very, very good. This probably isn’t a big enough deal that it requires tearing things apart to accommodate; but, if it fits into the long-term roadmap, it would be a generally valuable aspect of practical design. To the question of cycles in the gateway versus work in the services, could it split the difference? If there was a function hook somewhere that allowed a developer to build their own key in the cases where they needed to be judicious (at the cost of more time in the gateway), that seems totally sufficient. Otherwise, the existing behavior definitely seems like the 99% use case.

…

On Thu, Aug 6, 2020 at 7:20 AM Yaacov Rydzinski ***@***.***> wrote: Ahah. The selection sets are added by field but because the "proxiability" from one schema to the next is preprocessed from the smaller key, delegation to inventorySchema is fired off prior to those fields arriving. So we are back to the question of larger, more expensive keys or dynamically determining proxiability in terms of overall performance. Not sure I have a great handle on a real world example, but makes sense in general to not cause wasted cycles in the backing services in favor of more cycles on the gateway... — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1874 (reply in thread)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFRRODX3D42EM23TSIVEEDR7KGZLANCNFSM4PUPK75Q> .

[yaacovCR]

#1888 i abandoned some build-time optimizations for other reasons, see #1888 (comment)

This seems -- to me -- along the lines of what you had in mind, but please tell me otherwise as necessary.

Not sure whether necessary at this point to add option for field's selectionSet property beyond string of a function taking a fieldNode and returning a selectionSet for even more advanced options.

Can also add for the type itself, beyond string selectionSet, option of a function taking an array of fieldNodes and returning a selectionSet.

Does this work for now, or are those advanced options really necessary?

[gmac]

I did a fairly comprehensive breakdown of Federation compared to Type Merging here: #1888 (comment). The short of it is that general feature parity with Federation would have some sort of concept along these lines:

Product: {
  selectionSet: selectionSetByField({
    shippingEstimate: '{ price weight }',
    carrier: '{ weight }'
  }),
  fieldName: '_entities',
  key: ({ weight, price }) => ({ weight, price, __typename: 'Product' }),
  args: (representations) => ({ representations }),
}

That selectionSetByField method could be an official GraphQL Tools helper method, or just a note in documentation as to how a developer would build a dynamic selection set. Also, worth noting – I'm not personally in need of this feature. I've raised it mainly as a discussion of feature parity with Federation (having a general-purpose Federation alternative exist is extremely valuable to the GraphQL ecosystem given that Federation has some good features, but comes with some distastefully strong opinions).

[gmac]

RE:

Not sure whether necessary at this point to add option for field's selectionSet property beyond string of a function taking a fieldNode and returning a selectionSet for even more advanced options.

Yes, if selectionSet could be a function that accepts a fieldNode and gives back a selection set, that'd probably be the most appropriately generic implementation that could handle any conceivable usecase. If that existed, I'd probably pursue a contribution along the lines of #1802 which would give developers something simple they could use directly out of the box for the 95% use case.

[yaacovCR]

Why is the function necessary? We already have:

graphql-tools/packages/batch-delegate/tests/typeMerging.example.test.ts

Lines 232 to 246 in 5f128f0

	{
	schema: inventorySchema,
	merge: {
	Product: {
	selectionSet: '{ upc }',
	fields: {
	shippingEstimate: {
	selectionSet: '{ price weight }',
	},
	},
	fieldName: '_productByRepresentation',
	args: ({ upc, weight, price }) => ({ product: { upc, weight, price } }),
	}
	}
	},

More verbose, but preserves extensibility, maybe.

Functions might be nice down the road, but I want to see a use case first.

[gmac]

Geeze dude, I can't keep up with you... I missed that test in your PR apparently. That's a pretty nice interface. 👍

[yaacovCR]

Note that this still works but has essentially been superseded by computed fields...

[gmac]

FWIW, we could probably axe the “fields” API in v8. It’s not documented and the inconsistency really strikes against its usefulness.

[gmac]

@yaacovCR – was writing some merging docs and got looking at args versus argsFromKeys... they appear to be interchangeable aliases as far as merged type config is concerned: https://github.com/gmac/graphql-tools/blob/master/packages/stitch/src/stitchingInfo.ts#L170.

In experimenting, there appears to be no functional difference between this...

merge: {
  User: {
    selectionSet: '{ id }',
    fieldName: 'usersByIds',
    key: ({ id }) => id,
    args: (ids) => ({ ids }),
  }
}

and this...

merge: {
  User: {
    selectionSet: '{ id }',
    fieldName: 'usersByIds',
    key: ({ id }) => id,
    argsFromKeys: (ids) => ({ ids }),
  }
}

That said, any reason documentation can't just always use args for consistency (rather than trying to explain that argsFromKeys is used with key?)

[yaacovCR]

I started off thinking args would do something different when key is enabled, but then I realized that if you make it a separate argument you can give better typings with type script, but I retain the backwards compatibility.

In a future major version, args might no longer work when key is specified.

[gmac]

Documentation PR is open here: #1910.

[gmac]

Thanks! Will bake that into docs.

…

On Thu, Aug 13, 2020 at 8:44 AM Yaacov Rydzinski ***@***.***> wrote: I started off thinking args would do something different when key is enabled, but then I realized that if you make it a separate argument you can give better typings with type script, but I retain the backwards compatibility. In a future major version, args might no longer work when key is specified. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1874 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFRROHY6EN62YAV7PVKJN3SAPN4RANCNFSM4PUPK75Q> .