subscriptions support in the query planner

[clenfest]

Created a couple of new node types:

• SubscriptionPlanNode which is a top level node indicating a subscription
• SubscriptionNode which is a subscription version of a FetchNode

[changeset-bot]

🦋 Changeset detected

Latest commit: 8405657

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 7 packages

Name	Type
@apollo/composition	Minor
@apollo/query-planner	Minor
@apollo/gateway	Minor
@apollo/federation-internals	Minor
@apollo/query-graphs	Minor
@apollo/subgraph	Minor
apollo-federation-integration-testsuite	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[netlify]

✅ Deploy Preview for apollo-federation-docs canceled.

Name	Link
🔨 Latest commit	dbf4660
🔍 Latest deploy log	https://app.netlify.com/sites/apollo-federation-docs/deploys/63e5c1e7ac1ff10008f63578

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

[bnjjj]

@clenfest ping me when you think it's ready to merge if you want me to try it in the router directly before merging.

[pcmanus]

A couple remarks:

• have we given though about what @provides and @requires within the "primary" fetch of a subscription? I mean, those "work" in the sense that the query plan will handle them in the "normal" way, but are we sure this is what we want for subscription? That is, my understanding of our design is that what is in the primary fetch will not be "refreshed" automatically on events unless users manually include those fields in the events themselves, but it feels like @requires and @provides makes it quite a bit harder for a user to understand what is "primary" (and need to be manually included in events if a refresh is needed) and what isn't (which fwiw, strikes me as the one thing maybe a bit complex UX-wise in the design even when @requires and @provides are not involved).
• the patch expects that subscriptions have no @defer, but there doesn't seem to be any validation that this is not the case. The current validation does reject @defer on subscription root fields, but only on root fields while I think we want to exclude them from "anywhere in a subscription operation" instead.

[clenfest]

A couple remarks:

• have we given though about what @provides and @requires within the "primary" fetch of a subscription? I mean, those "work" in the sense that the query plan will handle them in the "normal" way, but are we sure this is what we want for subscription? That is, my understanding of our design is that what is in the primary fetch will not be "refreshed" automatically on events unless users manually include those fields in the events themselves, but it feels like @requires and @provides makes it quite a bit harder for a user to understand what is "primary" (and need to be manually included in events if a refresh is needed) and what isn't (which fwiw, strikes me as the one thing maybe a bit complex UX-wise in the design even when @requires and @provides are not involved).
• the patch expects that subscriptions have no @defer, but there doesn't seem to be any validation that this is not the case. The current validation does reject @defer on subscription root fields, but only on root fields while I think we want to exclude them from "anywhere in a subscription operation" instead.

Correct me if I'm wrong, but I believe that the call to operation.withoutDefer(); on line 2515 should strip out all the defers.

[pcmanus]

Asides from a few nitpicks that would be nice but are unimportant, I'd appreciate:

• renaming SubscriptionPlanNode into SubscriptionNode now. It's genuinely feel cleaner to me.
• the point about @defer below.
  But none are blockers per-se, so pre-emptively approving.

Correct me if I'm wrong, but I believe that the call to operation.withoutDefer(); on line 2515 should strip out all the defers.

Yes, it will. But my remark was more a UX suggestion. What I mean is that afaict, the behaviour with the current patch (in the router) would be:

1. for non-subscription operation, a @defer always does something (even when subgraph fetches can't be quite deferred, we still at least defer the response proc❯
2. for subscription operation, if the @defer is on a root field, then we reject it (as pointed in my previous comment).
3. for subscription operation, if the @defer is not on a root field, then this patch ignores it.

Now, point 3 is technically ok from a spec perspective, but from a consistency POV, merging 2 and 3 by always rejecting @defer within subscription operations wo❯

Note that rejecting @defer instead of ignoring it is, I believe, pretty trivial code wise: instead of skipping this check for subscription, we could let it run but then error if hasDefers && isSubscription.

Anyway, I'd personally prefer rejecting @defer instead of ignoring it, but certainly not a huge point.