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

Type conditioned fetching #2949

Merged
merged 37 commits into from
Apr 12, 2024
Merged

Type conditioned fetching #2949

merged 37 commits into from
Apr 12, 2024

Conversation

o0Ignition0o
Copy link
Contributor

@o0Ignition0o o0Ignition0o commented Mar 4, 2024

Type conditioned fetching

Fixes #2938
When querying a field that is in a path of 2 or more unions, the query planner was not able to handle different selections and would aggressively collapse selections in fetches yielding an incorrect plan.

This change introduces new syntax to express type conditions in (key and flatten) paths. Type conditioned fetching can be enabled through a flag, and execution is supported in the router only.

Copy link

changeset-bot bot commented Mar 4, 2024

🦋 Changeset detected

Latest commit: da43eab

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

Copy link

netlify bot commented Mar 4, 2024

Deploy Preview for apollo-federation-docs ready!

Name Link
🔨 Latest commit da43eab
🔍 Latest deploy log https://app.netlify.com/sites/apollo-federation-docs/deploys/6618e59a16c286000831fdad
😎 Deploy Preview https://deploy-preview-2949--apollo-federation-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

codesandbox-ci bot commented Mar 4, 2024

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.

query-planner-js/src/buildPlan.ts Outdated Show resolved Hide resolved
query-planner-js/src/buildPlan.ts Outdated Show resolved Hide resolved
query-planner-js/src/buildPlan.ts Outdated Show resolved Hide resolved
@o0Ignition0o o0Ignition0o changed the title WIP: type conditioned fetching Type conditioned fetching Mar 29, 2024
@o0Ignition0o o0Ignition0o marked this pull request as ready for review March 29, 2024 09:56
@o0Ignition0o o0Ignition0o requested a review from a team as a code owner March 29, 2024 09:56
Copy link
Member

@dariuszkuc dariuszkuc left a comment

Choose a reason for hiding this comment

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

Lets update gateway code to fail on startup vs at runtime when new config is used.

Was also wondering whether we should update the tests to more generic types such as s1/s2 => subgraphs, U => union, T1/T2 => types, etc (thats the pattern we have in some other tests)

gateway-js/src/executeQueryPlan.ts Outdated Show resolved Hide resolved
query-planner-js/src/buildPlan.ts Outdated Show resolved Hide resolved
query-planner-js/src/buildPlan.ts Outdated Show resolved Hide resolved
query-planner-js/src/buildPlan.ts Show resolved Hide resolved
query-planner-js/src/buildPlan.ts Outdated Show resolved Hide resolved
@o0Ignition0o
Copy link
Contributor Author

Renamed test elements in eb0b6e4!

@o0Ignition0o o0Ignition0o dismissed sachindshinde’s stale review April 12, 2024 12:14

Changes have been adressed.

@o0Ignition0o o0Ignition0o merged commit 3e2c845 into main Apr 12, 2024
19 checks passed
@o0Ignition0o o0Ignition0o deleted the igni/type_conditioned_fetching branch April 12, 2024 12:15
o0Ignition0o pushed a commit that referenced this pull request Apr 16, 2024
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.7.3

### Patch Changes

- Fix a query planning bug where invalid subgraph queries are generated
with `reuseQueryFragments` set true.
([#2952](#2952))
([#2963](#2963))

- Updated dependencies
\[[`ec04c50b4fb832bfd281ecf9c0c2dd7656431b96`](ec04c50),
[`a494631918156f0431ceace74281c076cf1d5d51`](a494631)]:
    -   @apollo/federation-internals@2.7.3
    -   @apollo/query-graphs@2.7.3

## @apollo/gateway@2.7.3

### Patch Changes

- Updated dependencies
\[[`ec04c50b4fb832bfd281ecf9c0c2dd7656431b96`](ec04c50),
[`3e2c845c74407a136b9e0066e44c1ad1467d3013`](3e2c845),
[`a494631918156f0431ceace74281c076cf1d5d51`](a494631)]:
    -   @apollo/query-planner@2.7.3
    -   @apollo/composition@2.7.3
    -   @apollo/federation-internals@2.7.3

## @apollo/federation-internals@2.7.3

### Patch Changes

- Fix a query planning bug where invalid subgraph queries are generated
with `reuseQueryFragments` set true.
([#2952](#2952))
([#2963](#2963))

- Fixed query planner to pass the directives from original query to
subgraph operations (#2961)
([#2967](#2967))

## @apollo/query-graphs@2.7.3

### Patch Changes

- Updated dependencies
\[[`ec04c50b4fb832bfd281ecf9c0c2dd7656431b96`](ec04c50),
[`a494631918156f0431ceace74281c076cf1d5d51`](a494631)]:
    -   @apollo/federation-internals@2.7.3

## @apollo/query-planner@2.7.3

### Patch Changes

- Fix a query planning bug where invalid subgraph queries are generated
with `reuseQueryFragments` set true.
([#2952](#2952))
([#2963](#2963))

- Type conditioned fetching
([#2949](#2949))

When querying a field that is in a path of 2 or more unions, the query
planner was not able to handle different selections and would
aggressively collapse selections in fetches yielding an incorrect plan.

This change introduces new syntax to express type conditions in (key and
flatten) paths. Type conditioned fetching can be enabled through a flag,
and execution is supported in the router only. (#2938)

- Fixed query planner to pass the directives from original query to
subgraph operations (#2961)
([#2967](#2967))

- Updated dependencies
\[[`ec04c50b4fb832bfd281ecf9c0c2dd7656431b96`](ec04c50),
[`a494631918156f0431ceace74281c076cf1d5d51`](a494631)]:
    -   @apollo/federation-internals@2.7.3
    -   @apollo/query-graphs@2.7.3

## @apollo/subgraph@2.7.3

### Patch Changes

- Updated dependencies
\[[`ec04c50b4fb832bfd281ecf9c0c2dd7656431b96`](ec04c50),
[`a494631918156f0431ceace74281c076cf1d5d51`](a494631)]:
    -   @apollo/federation-internals@2.7.3

## apollo-federation-integration-testsuite@2.7.3

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
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.

Query planner bug with queries fetching fields nested in multiple unions
4 participants