Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix QP bug querying some fields with @interfaceObject #2362

Merged
merged 2 commits into from Feb 2, 2023

Conversation

pcmanus
Copy link
Contributor

@pcmanus pcmanus commented Feb 1, 2023

When an interface field is requested only for some implementation, and the query "transit" through a subgraph using @interfaceObject, then the query planning code was not recognizing early enough that the field was queried on an implementation type and not the interface itself, and this lead to a later assertion error.

This commit adds a reproduction test, and fix the issue by simply making sure, when we build the query plan "paths", to not use an interface field when it's an implementation field that is queried.

@netlify
Copy link

netlify bot commented Feb 1, 2023

👷 Deploy request for apollo-federation-docs pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 914ded6

@changeset-bot
Copy link

changeset-bot bot commented Feb 1, 2023

🦋 Changeset detected

Latest commit: 914ded6

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-graphs Patch
@apollo/query-planner Patch
@apollo/composition Patch
@apollo/gateway Patch
@apollo/federation-internals 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

@codesandbox-ci
Copy link

codesandbox-ci bot commented Feb 1, 2023

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.

@clenfest
Copy link
Contributor

clenfest commented Feb 2, 2023

Just one misspelling in the changelog. Other than that, it looks like we need to update the graphql type check thingy (is there an actual reason for having this be a part of our pipeline?) but I'll approve assuming you'll do that.

Sylvain Lebresne added 2 commits February 2, 2023 10:32
When an interface field is requested only for some implementation, and
the query "transit" through a subgraph using `@interfaceObject`, then
the query planning code was not recognizing early enough that the field
was queried on an implementation type and not the interface itself, and
this lead to a later assertion error.

This commit adds a reproduction test, and fix the issue by simply making
sure, when we build the query plan "paths", to not use an interface field
when it's an implementation field that is queried.
@pcmanus pcmanus merged commit 7e2ca46 into apollographql:main Feb 2, 2023
@korinne korinne added this to the 2.3.1 milestone Feb 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants