apollographql / federation

🌐  Build and scale a single data graph across multiple services with Apollo's federation gateway.
https://apollographql.com/docs/federation/
Other
661 stars 249 forks source link

Bug: Query Planner Fails to Build Query for Nested Entity Fields Decorated With `@requires` #3113

Open ryan01010111 opened 1 month ago

ryan01010111 commented 1 month ago

Issue Description

When querying for nested entity fields decorated with @requires, a gateway for a federated graph fails to build a query plan, resulting in error "Cannot add selection of field \"X\" to selection set of parent type \"Y\"". This is a critical issue for my org.

All @apollo/* packages at latest, and composition done with latest (v2.8.4) rules.

Link to Reproduction

CodeSandbox

Reproduction Steps

  1. Start all services (if not already running) with:
    npm run dev
  2. Run test query to reproduce the error with:
    npm run test:query
duckki commented 1 month ago

I can reproduce this issue. The query planner is trying to overly optimize the fetch dependency graph (here), when

image

The QP fails because it attempts to add an edge from the parent node to the new node (bypassing the key fetch node in the middle), assuming the key fetch node and its parent node share the same key field.

The reasoning for this optimization seems ok, but we also need to use the correct key isbn, not id. Or, we can check this situation and fall back to the normal process (the else-branch).

ryan01010111 commented 3 weeks ago

Thanks, @duckki for confirming, and identifying the root cause! I just want to check in to see if there are any updates/plans regarding this issue. cc @clenfest

ryan01010111 commented 2 days ago

I've gone ahead and created a fork with a patch, but in accordance with the project contribution guidelines, would like to consult on the design before opening a PR. I agree with @duckki that the edge optimization is reasonable, and so we should just check for the cross-subgraph key condition, and look into the appropriate subgraph schema for a suitable key field to select. Please see my changes here.