Skip to content

fix: query planning for shared entities#1409

Closed
StarpTech wants to merge 1 commit intomasterfrom
dustin/eng-9037-shared-entity-with-external-key-field-fails-query-planning
Closed

fix: query planning for shared entities#1409
StarpTech wants to merge 1 commit intomasterfrom
dustin/eng-9037-shared-entity-with-external-key-field-fails-query-planning

Conversation

@StarpTech
Copy link
Copy Markdown
Collaborator

@StarpTech StarpTech commented Feb 26, 2026

Summary

Fixes query planning failure (500 error) when a federated query references a shared entity whose @key
field is marked @external in one subgraph.

Example: A CartBundleProduct entity exists in two subgraphs. The cart subgraph owns fields like name
and category but marks the key field code as @external. The products subgraph owns code. Querying
bundleProducts { code name } through the cart subgraph would fail with:

could not select the datasource to resolve CartBundleProduct.code

Root Cause

Newer federation composition puts @external key fields only in ExternalFieldNames (not in both
FieldNames and ExternalFieldNames like older composition did). This broke two assumptions in the
planner:

  1. Key source detection (key_fields_visitor.go): The existing fallback that un-marks external key
    fields relied on the field being in both FieldNames and ExternalFieldNames. With the field only in
    ExternalFieldNames, the key was incorrectly treated as external, preventing the cart subgraph from
    being a key "source." This broke the jump graph so no datasource could resolve the field.
  2. Field suggestion creation (datasource_filter_collect_nodes_visitor.go): Even after fixing #1, the
    cart subgraph still didn't suggest the code field. When the visitor converted an external field to
    non-external (via notExternalKeyPaths), it cleared isExternal but the field had no other qualifying
    flag (hasRootNode, hasChildNode, isProvided were all false). So no suggestion was created and the
    planner routed code to the products subgraph via entity resolution — generating a malformed upstream
    query.

Fix

Three changes:

  • datasource_configuration.go: Add HasNonExternalFieldsForType() to check whether a datasource
    actively resolves a type (has regular, non-external fields for it) vs. being a stub with only external
    key fields.
  • key_fields_visitor.go: When a key field is external but the datasource has non-external fields for
    that entity type, treat the key as non-external. The datasource actively resolves this entity, so it
    can provide the key field in its response.
  • datasource_filter_collect_nodes_visitor.go: When an external field is reclassified as non-external
    (because it's a key field), promote its external root/child node status to regular root/child status.
    This ensures a field suggestion is actually created, so the planner can route the field to the correct
    datasource.

Test plan

  • New test TestGraphQLDataSourceFederation_SharedEntityWithExternalKeyField with 4 sub-tests:
    • bundleProducts with code field — the main bug (was failing, now passes)
    • bundleProducts with only code field — variant of the bug (was failing, now passes)
    • cart only fields without bundleProducts — control (always passed)
    • bundleProducts without code field — control (always passed)

CodeRabbit

Summary by CodeRabbit

Release Notes

  • Tests

    • Added comprehensive test coverage for GraphQL federation with shared entities, including scenarios with external key fields and complex entity relationships.
  • Improvements

    • Enhanced federation support for key fields defined in external field names across subgraphs.
    • Improved query planning and data source resolution in federated GraphQL schemas with shared entities.

Checklist

  • I have discussed my proposed changes in an issue and have received approval to proceed.
  • I have followed the coding standards of the project.
  • Tests or benchmarks have been added or updated.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Feb 26, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 78ea8ff and e1da1c7.

📒 Files selected for processing (4)
  • v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_shared_entity_test.go
  • v2/pkg/engine/plan/datasource_configuration.go
  • v2/pkg/engine/plan/datasource_filter_collect_nodes_visitor.go
  • v2/pkg/engine/plan/key_fields_visitor.go

📝 Walkthrough

Walkthrough

This PR introduces federation support for shared entities with external key fields. It adds a new test suite validating federation scenarios where key fields exist only in ExternalFieldNames rather than regular FieldNames, implements a new interface method to check for non-external fields in datasources, and updates key-field visitor logic to correctly treat keys as non-external when datasources actively resolve entities.

Changes

Cohort / File(s) Summary
Federation Key-Field Handling
v2/pkg/engine/plan/key_fields_visitor.go, v2/pkg/engine/plan/datasource_filter_collect_nodes_visitor.go
Added logic to treat keys as non-external when datasource reports non-external fields for the type; promotes node statuses when external fields are disabled during key-field handling to preserve query suggestions.
Federation Metadata Configuration
v2/pkg/engine/plan/datasource_configuration.go
Introduced HasNonExternalFieldsForType interface method on NodesInfo and implementation on DataSourceMetadata to check existence of non-external fields for a given type.
Federation Shared Entity Tests
v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_shared_entity_test.go
Added comprehensive test TestGraphQLDataSourceFederation_SharedEntityWithExternalKeyField with multiple subtests covering federation scenarios where entity key fields exist exclusively in ExternalFieldNames.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: query planning for shared entities' is concise and directly addresses the main objective of the PR, which is fixing query planning failures for shared entities with external key fields.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch dustin/eng-9037-shared-entity-with-external-key-field-fails-query-planning

Comment @coderabbitai help to get the list of available commands and usage tips.

@StarpTech StarpTech closed this Feb 26, 2026
@StarpTech
Copy link
Copy Markdown
Collaborator Author

StarpTech commented Feb 26, 2026

Close it because it's a composition error.

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