fix(federation/qp): suppress the type condition at root-level inline spread in subgraph fetch operations

[duckki]

Background

We allow renaming root types (like MyQuery, instead of Query) in subgraph schemas. But, the supergraph schema does not record that renaming information. Thus, if an inline spread at root level like ... on Query { ... } is present in the subgraph operation, it can cause runtime errors at the subgraph server since it will be invalid in the (original) subgraph schema.

Proposed fix

We can simply drop any root type condition by changing ... on Query to .... The unconditioned inline spread still may be necessary for directives.

Discussion

Potential additional optimizations:

• Root-level inline spreads could be eliminated completely, if no directives are present. But, this PR did not attempt to do that.
• @defer may add an extra root-level inline spread, which could be avoided preemptively. But, this PR didn't implement that.

---

Checklist

Complete the checklist (and note appropriate exceptions) before the PR is marked ready-for-review.

• Changes are compatible[^1]
• Documentation[^2] completed
• Performance impact assessed and acceptable
• Tests added and passing[^3]
  • Unit Tests
  • Integration Tests
  • Manual Tests

Closes #7573

[github-actions]

@duckki, please consider creating a changeset entry in /.changesets/. These instructions describe the process and tooling.

[svc-apollo-docs]

✅ Docs preview ready

The preview is ready to be viewed. View the preview

File Changes

0 new, 6 changed, 0 removed

* graphos/routing/(latest)/configuration/yaml.mdx
* graphos/routing/(latest)/customization/custom-binary.mdx
* graphos/routing/(latest)/operations/subscriptions/configuration.mdx
* graphos/routing/(latest)/performance/caching/distributed.mdx
* graphos/routing/(latest)/performance/caching/in-memory.mdx
* graphos/routing/(latest)/security/demand-control.mdx

Build ID: e8bdebe6e64ad22cee30c462

URL: https://www.apollographql.com/docs/deploy-preview/e8bdebe6e64ad22cee30c462

[dariuszkuc]

this seems redundant? guessing its a leftover from debugging?

[duckki]

It was a necessary change. After can_rebase_on change, some tests failed at .or_insert call below because ele.key() and element.key() became different in some cases (... on Query vs ...).

In general, ele and element may be different and it makes sense to look up element.key(), instead of ele.key(). It appears that resulting in a different key via rebase here never happened before.

BTW, to_owned_key/as_borrowed_key calls had to be added, since element itself is going to move into the closure below. So, the key had to be cloned using to_owned_key (the normal clone() would only do a shallow cloning).