Skip to content

fix(federation/query-planning): Prevent error for satisfiable @shareable mutation fields#8352

Merged
sachindshinde merged 4 commits intodevfrom
sachin/fix-shareable-mutation-fields-in-qp
Oct 16, 2025
Merged

fix(federation/query-planning): Prevent error for satisfiable @shareable mutation fields#8352
sachindshinde merged 4 commits intodevfrom
sachin/fix-shareable-mutation-fields-in-qp

Conversation

@sachindshinde
Copy link
Contributor

@sachindshinde sachindshinde commented Sep 29, 2025

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 is the equivalent of this JS repo PR.

Unrelatedly, this PR also fixes an error messaging issue when QP unexpectedly doesn't find a plan (this shouldn't happen, but if it did, it was previously using the wrong federation error enum and message).

@sachindshinde sachindshinde requested review from a team as code owners September 29, 2025 18:59
@apollo-librarian
Copy link

apollo-librarian bot commented Sep 29, 2025

✅ Docs preview ready

The preview is ready to be viewed. View the preview

File Changes

3 new, 10 changed, 4 removed
+ graphos/routing/(latest)/observability/router-telemetry-otel/apm-guides/datadog/observing-and-monitoring/dashboard-template.mdx
+ graphos/routing/(latest)/self-hosted/managed-hosting/railway.mdx
+ graphos/routing/(latest)/self-hosted/managed-hosting/render.mdx
* graphos/routing/(latest)/customization/rhai/index.mdx
* graphos/routing/(latest)/customization/coprocessor.mdx
* graphos/routing/(latest)/observability/graphos/graphos-reporting.mdx
* graphos/routing/(latest)/observability/router-telemetry-otel/apm-guides/datadog/connecting-to-datadog/datadog-agent/datadog-agent-metrics.mdx
* graphos/routing/(latest)/observability/router-telemetry-otel/apm-guides/datadog/connecting-to-datadog/datadog-agent/datadog-agent-traces.mdx
* graphos/routing/(latest)/observability/router-telemetry-otel/apm-guides/prometheus/otel-traces-to-prometheus.mdx
* graphos/routing/(latest)/observability/router-telemetry-otel/enabling-telemetry/selectors.mdx
* graphos/routing/(latest)/observability/router-telemetry-otel/enabling-telemetry/spans.mdx
* graphos/routing/(latest)/self-hosted/containerization/docker.mdx
* graphos/routing/(latest)/_sidebar.yaml
- graphos/routing/(latest)/observability/router-telemetry-otel/apm-guides/datadog/index.mdx
- graphos/routing/(latest)/observability/router-telemetry-otel/apm-guides/datadog/router-instrumentation.mdx
- graphos/routing/(latest)/observability/router-telemetry-otel/apm-guides/datadog/connecting-to-datadog/index.mdx
- graphos/routing/(latest)/observability/router-telemetry-otel/apm-guides/datadog/connecting-to-datadog/otel-collector.mdx

Build ID: 8be98f32a1d4924fb5228093
Build Logs: View logs

URL: https://www.apollographql.com/docs/deploy-preview/8be98f32a1d4924fb5228093

@github-actions

This comment has been minimized.

@sachindshinde sachindshinde force-pushed the sachin/fix-shareable-mutation-fields-in-qp branch from bb6a401 to 308f954 Compare September 29, 2025 19:10
@dariuszkuc
Copy link
Member

Corresponding JS fix

@sachindshinde sachindshinde changed the title fix(federation/query-planning): Prevent error for satisfiable @shareable mutation fields fix(federation/query-planning): Prevent error for satisfiable @shareable mutation fields Sep 29, 2025
@sachindshinde sachindshinde force-pushed the sachin/fix-shareable-mutation-fields-in-qp branch from 308f954 to fe269b1 Compare September 29, 2025 19:56
@sachindshinde sachindshinde force-pushed the sachin/fix-shareable-mutation-fields-in-qp branch from fe269b1 to ac12814 Compare September 29, 2025 22:37
@sachindshinde sachindshinde requested a review from a team as a code owner September 29, 2025 22:37
@sachindshinde sachindshinde force-pushed the sachin/fix-shareable-mutation-fields-in-qp branch from ac12814 to 2d9fa2f Compare September 29, 2025 23:11
@sachindshinde
Copy link
Contributor Author

@dariuszkuc
That PR has been split up, and this PR corresponds more closely to apollographql/federation#3304 now.

Copy link
Contributor

@duckki duckki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! The fix makes sense and I confirmed the test is now fixed.

@sachindshinde sachindshinde force-pushed the sachin/fix-shareable-mutation-fields-in-qp branch from 1badade to c39b11d Compare October 16, 2025 02:07
@sachindshinde
Copy link
Contributor Author

After further testing, there were two more changes needed:

  1. Top-level query planning was ignoring root type resolution edges, but not @key edges; this has been fixed now.
    • This normally wouldn't be an issue as @key on root operation types is inefficient compared to root type resolution edges, meaning query planning wouldn't normally choose such options. However, for supergraph schemas with unsatisfiable mutation operations, this could accidentally generate a plan instead of an error (when it should be erroring).
  2. Non-local selection estimation needed to be updated to accommodate initial subgraph constraints, otherwise it would systematically overestimate the number of non-local selections.
    • If not fixed, this leads to mutation query planning unexpectedly experiencing a QueryPlanComplexityExceeded error when there are many subgraphs with mutation types.

@sachindshinde sachindshinde merged commit b14c4a3 into dev Oct 16, 2025
15 checks passed
@sachindshinde sachindshinde deleted the sachin/fix-shareable-mutation-fields-in-qp branch October 16, 2025 04:59
@duckki
Copy link
Contributor

duckki commented Oct 16, 2025

The follow-up update also looks good. Nice work!

@abernix abernix mentioned this pull request Oct 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants