Skip to content

fix(composition,query-planner): Fix @shareable mutation field behavior#3303

Closed
sachindshinde wants to merge 2 commits intomainfrom
sachin/fix-shareable-mutation-fields
Closed

fix(composition,query-planner): Fix @shareable mutation field behavior#3303
sachindshinde wants to merge 2 commits intomainfrom
sachin/fix-shareable-mutation-fields

Conversation

@sachindshinde
Copy link
Contributor

@sachindshinde sachindshinde commented Sep 25, 2025

The current behavior for @shareable mutation fields has some bugs. At a high level:

  1. Query planning may unexpectedly error while planning an operation with a @shareable mutation field.
  2. Composition may erroneously declare a schema with a @shareable mutation field satisfiable when it shouldn't be.

This PR fixes both of these issues. It specifically:

  1. Updates composition satisfiability to ensure that all query paths under a @shareable mutation field can be satisfied starting at the same subgraph. (I.e., that no matter what selection set is on the @shareable mutation field, the query planner will not need to make multiple calls to that mutation field across subgraphs.)
    • This is done by partitioning subgraph paths in ValidationState by the initial subgraph when a shared mutation field is encountered, and keeping track of separate error stacks as well. If all these partitions experience satisfiability errors, then there's no way to satisfy the selection set containing the witness operation from each partition using a single @shareable mutation field call, and the user is asked to fix (at least) one of the partition's satisfiability errors.
  2. Updates query planning on mutation fields to only consider one initial subgraph at a time for a traversal, and returns the plan with lowest cost.
    • Previously, this was mixing up options that started in different subgraphs in the same traversal, which would sometimes generate query planning errors if those differing options were chosen. There were a few ways to fix this, but the least invasive/simplest was just to do multiple traversals, each with a limited initial subgraph.
  3. Updates a particular query planning optimization to avoid discarding an option if another option with less subgraph jumps/cost is found, if that option starts in a different subgraph and the option is for a mutation operation.

Note this PR should be fine to release in a patch release from the gateway/router perspective, since it restricts composition and relaxes QP (from the composition perspective, the actual rejection of some compositions isn't great, but the incidence of broken @shareable mutation fields is rare and the alternative is runtime errors).

@sachindshinde sachindshinde requested a review from a team as a code owner September 25, 2025 19:01
@changeset-bot
Copy link

changeset-bot bot commented Sep 25, 2025

⚠️ No Changeset found

Latest commit: eb578df

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@apollo-librarian
Copy link

apollo-librarian bot commented Sep 25, 2025

❌ Docs preview failed

The preview failed to build.

Build ID: b0f2a56fabc1ca25acac30d8
Build Logs: View logs

Errors

General: ENOSPC: no space left on device, mkdir '/tmp/librarian/remote-sources/apollographql/rover/main'

@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 25, 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.

@sachindshinde
Copy link
Contributor Author

After further analysis, the occurrence of compositions that were mistakenly considered satisfiable due to this bug may be higher than expected. Accordingly, this PR has been split it in a query planning PR #3304 (which can merge in the next patch release) and a composition PR #3305 (which can merge in the next minor release).

sachindshinde added a commit that referenced this pull request Oct 21, 2025
…ion fields (#3304)

This PR fixes a bug where query planning may unexpectedly error while
planning an operation with a satisfiable `@shareable` mutation field.

Specifically, this PR:
1. Updates query planning on mutation fields to only consider one
initial subgraph at a time for a traversal, and returns the plan with
lowest cost.
- Previously, this was mixing up options that started in different
subgraphs in the same traversal, which would sometimes generate query
planning errors if those differing options were chosen. There were a few
ways to fix this, but the least invasive/simplest was just to do
multiple traversals, each with a limited initial subgraph.
- Non-local selection estimation was also appropriately updated to
account for this (if it weren't, it would systemically overestimate the
number of non-local selections).
2. Updates a particular query planning optimization to avoid discarding
an option if another option with less subgraph jumps/cost is found, if
that option starts in a different subgraph and the option is for a
mutation operation.
3. Updates query planning to ignore `@key` edges at top-level
(previously only root type resolution edges were ignored).

This PR was originally filed as part of
#3303, but was split off
so it could be merged faster in the next patch release.
sachindshinde added a commit that referenced this pull request Oct 24, 2025
…3305)

This PR fixes a bug where composition may erroneously declare a schema
with a `@shareable` mutation field satisfiable when it shouldn't be.

Specifically, this PR updates composition satisfiability to ensure that
all query paths under a `@shareable` mutation field can be satisfied
starting at the same subgraph. (I.e., that no matter what selection set
is on the `@shareable` mutation field, the query planner will not need
to make multiple calls to that mutation field across subgraphs.)

This is done by partitioning subgraph paths in `ValidationState` by the
initial subgraph when a shared mutation field is encountered, and
keeping track of separate error stacks as well. If all these partitions
experience satisfiability errors, then there's no way to satisfy the
selection set containing the witness operation from each partition using
a single `@shareable` mutation field call, and the user is asked to fix
(at least) one of the partition's satisfiability errors.

This PR may cause composition errors for a previously successful
composition if that composition was mistakenly satisfiable due to this
bug. We've determined that this may not be a rare occurrence, and
accordingly this fix will land in the next minor release (which is why
this PR was split off from
#3303, where it was
originally filed). This PR is currently stacked on top of
#3304 for tests to pass,
but once that PR merges and is pulled into `next`, this PR's base will
change to `next`.
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.

1 participant