fix(query-planner): @requires fetch group optimization selects invalid key

[ryan01010111]

Notes

Fixes #3113 and #2987.
FWIW, this fix has held up in production systems for 6+ months.

Problem

Fetch group optimization/handling for @requires conditions "assumes" existence of a key in the parent group's schema. This leads to failed query planning in cases where the parent group's schema does not define the same keys for an entity as that of the child group.

Fix Summary

• Added logic that resolves a select-able key field from a fetch group's parent, and incorporated it into the fetch group optimization flow.

[apollo-cla]

@ryan01010111: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/

[changeset-bot]

🦋 Changeset detected

Latest commit: 5b4b389

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

This PR includes changesets to release 7 packages

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

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

[svc-apollo-docs]

✅ Docs preview has no changes

The preview was not built because there were no changes.

Build ID: 902e5499d2d799b68cbce00f

[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.

[ryan01010111]

Could someone confirm requirements met for having this reviewed? @duckki @lennyburdette

[duckki]

Could someone confirm requirements met for having this reviewed? @duckki @lennyburdette

Thank you for creating this PR. The federation team will review.

[sachindshinde]

Thanks for filing the PR and including a test. The issue more specifically is that canSatisfyConditions() guarantees that the subgraph has a locally satisfiable key, but it doesn't guarantee that any upstream parent groups will have that entire key.

This means that when createPostRequiresGroup() tries to fetch that key in parent.group, it might fail. You've updated the code in addPostRequireInputs()/downstream functions to look for a different key in the parent group, but that may not find one, in which case query planning will still error. I've filed a separate PR that effectively falls back to group if the locally satisfiable key can't be found in parent.group, and simplifies your test case a bit (there are some extra fields/recursion that aren't needed).

[ryan01010111]

@sachindshinde Thank you very much for the review, and explanation! Do you happen to have an ETA on whatever release you expect the fix to go into?