Skip to content

fix(composition): Handle output type covariance properly in satisfiability#3277

Merged
sachindshinde merged 2 commits intomainfrom
sachin/fix-output-type-covariance-in-satisfiability
Jun 24, 2025
Merged

fix(composition): Handle output type covariance properly in satisfiability#3277
sachindshinde merged 2 commits intomainfrom
sachin/fix-output-type-covariance-in-satisfiability

Conversation

@sachindshinde
Copy link
Contributor

@sachindshinde sachindshinde commented Jun 16, 2025

This PR fixes two bugs in the satisfiability code when the subgraph type differs from the supergraph type due to output type covariance:

  1. It now handles no-op inline fragments when advancing a subgraph path with a graph transition.
    • Previously, the code assumed this was impossible, but it can happen in the case of output type covariance where the subgraph type matches the inline fragment type, which doesn't match the supergraph type. Since we don't store no-op edges in the query graph, this led to no edge being found, making satisfiability believe the transition was mistakenly unsatisfiable.
  2. The data of previous vertex visits now additionally includes the tail's type name for a given subgraph path.
    • Previously, the code assumed the tail's type name always matched the type name of the vertex in the key of the previous vertex visit state store. This is not the case when there's output type covariance, which led to satisfiability mistakenly believing a previous vertex visit was matching the current one (i.e. a loop's been detected) when it wasn't.

@sachindshinde sachindshinde requested a review from a team as a code owner June 16, 2025 19:04
@changeset-bot
Copy link

changeset-bot bot commented Jun 16, 2025

🦋 Changeset detected

Latest commit: fdecbd1

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 Patch
@apollo/query-graphs Patch
@apollo/gateway Patch
@apollo/query-planner 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 Jun 16, 2025

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.

@dariuszkuc dariuszkuc force-pushed the sachin/fix-output-type-covariance-in-satisfiability branch from 5171b0f to 790844d Compare June 24, 2025 17:19
@apollo-librarian
Copy link

apollo-librarian bot commented Jun 24, 2025

✅ Docs preview ready

The preview is ready to be viewed. View the preview

File Changes

0 new, 7 changed, 0 removed
* (developer-tools)/apollo-mcp-server/(latest)/command-reference.mdx
* (developer-tools)/apollo-mcp-server/(latest)/guides/index.mdx
* (developer-tools)/kotlin/v5/testing/android-studio-plugin.mdx
* (developer-tools)/rover/ci-cd.mdx
* (developer-tools)/rover/getting-started.mdx
* graphos/routing/(latest)/configuration/cli.mdx
* graphos/routing/(latest)/self-hosted/containerization/docker.mdx

Build ID: 4f17a68972dd49935c987bcc

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

@sachindshinde sachindshinde merged commit 51bed5b into main Jun 24, 2025
17 checks passed
@sachindshinde sachindshinde deleted the sachin/fix-output-type-covariance-in-satisfiability branch June 24, 2025 18:07
dariuszkuc pushed a commit that referenced this pull request Jun 24, 2025
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## @apollo/composition@2.11.1

### Patch Changes

- Adding new CompositionOption `maxValidationSubgraphPaths`. This value
represents the maximum number of SubgraphPathInfo objects that may exist
in a ValidationTraversal when checking for satisfiability. Setting this
value can help composition error before running out of memory. Default
is 1,000,000.
([#3254](#3254))

- Fix bug in composition where, when a field's type in a subgraph is a
subtype of the field's type in the supergraph, the satisfiability
validation spuriously succeeds/errors.
([#3277](#3277))

- Updated dependencies
\[[`7799ad1717becf15fb0e82f89619f2ec8a24b4d4`](7799ad1),
[`b26794c5724ef23d1f0fd45a40aee3d301557489`](b26794c),
[`51bed5be49d8e87adae59f568315c9e3488a91e0`](51bed5b)]:
    -   @apollo/federation-internals@2.11.1
    -   @apollo/query-graphs@2.11.1

## @apollo/gateway@2.11.1

### Patch Changes

- Updated dependencies
\[[`7799ad1717becf15fb0e82f89619f2ec8a24b4d4`](7799ad1),
[`b26794c5724ef23d1f0fd45a40aee3d301557489`](b26794c),
[`51bed5be49d8e87adae59f568315c9e3488a91e0`](51bed5b)]:
    -   @apollo/federation-internals@2.11.1
    -   @apollo/composition@2.11.1
    -   @apollo/query-planner@2.11.1

## @apollo/federation-internals@2.11.1

### Patch Changes

- fix: make composeDirective argument non-nullable.
([#3278](#3278))

Per our
[docs](https://www.apollographql.com/docs/graphos/schema-design/federated-schemas/reference/directives#composedirective),
`@composeDirective` requires non-nullable value to be passed as an
argument.

Our
[validations](https://github.com/apollographql/federation/blob/main/composition-js/src/composeDirectiveManager.ts#L250-L255)
were checking for valid values (it has to be a string that starts with
`@`),
but the generated schema was incorrectly specifying that the argument
was nullable.

- Adding new CompositionOption `maxValidationSubgraphPaths`. This value
represents the maximum number of SubgraphPathInfo objects that may exist
in a ValidationTraversal when checking for satisfiability. Setting this
value can help composition error before running out of memory. Default
is 1,000,000.
([#3254](#3254))

## @apollo/query-graphs@2.11.1

### Patch Changes

- Fix bug in composition where, when a field's type in a subgraph is a
subtype of the field's type in the supergraph, the satisfiability
validation spuriously succeeds/errors.
([#3277](#3277))

- Updated dependencies
\[[`7799ad1717becf15fb0e82f89619f2ec8a24b4d4`](7799ad1),
[`b26794c5724ef23d1f0fd45a40aee3d301557489`](b26794c)]:
    -   @apollo/federation-internals@2.11.1

## @apollo/query-planner@2.11.1

### Patch Changes

- Updated dependencies
\[[`7799ad1717becf15fb0e82f89619f2ec8a24b4d4`](7799ad1),
[`b26794c5724ef23d1f0fd45a40aee3d301557489`](b26794c),
[`51bed5be49d8e87adae59f568315c9e3488a91e0`](51bed5b)]:
    -   @apollo/federation-internals@2.11.1
    -   @apollo/query-graphs@2.11.1

## @apollo/subgraph@2.11.1

### Patch Changes

- Updated dependencies
\[[`7799ad1717becf15fb0e82f89619f2ec8a24b4d4`](7799ad1),
[`b26794c5724ef23d1f0fd45a40aee3d301557489`](b26794c)]:
    -   @apollo/federation-internals@2.11.1

## apollo-federation-integration-testsuite@2.11.1

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
sachindshinde added a commit to apollographql/router that referenced this pull request Jun 26, 2025
…eTransition()` (#7740)

This PR ports `ValidationState()` from the JS codebase, except `validateTransition()`. Note that:
1. We've skipped `currentSubgraphs()`, as Rust composition doesn't currently at least give node information.
2. The subgraph context keys were strings in JS, but that was just because they needed to be used in JS `Map`s efficiently. We don't have that restriction in Rust, so we use actual data structures for the keys.
    - As part of this, we don't need actually need a map from subgraph names to subgraph enum values (that was used in JS to avoid issues with separator characters in the keys potentially being in the subgraph names). 
3. We did not port any fixes from apollographql/federation#3277 yet to this code.
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.

2 participants