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

Make queryPlanStoreKey a hash of both the query and Operation name #2310

Merged

Conversation

AneethAnand
Copy link
Contributor

@AneethAnand AneethAnand commented Jan 3, 2023

…he query and Operation name

Fixes #2309

@apollo-cla
Copy link

@AneethAnand: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/

@netlify
Copy link

netlify bot commented Jan 3, 2023

👷 Deploy request for apollo-federation-docs pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 224e18e

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jan 3, 2023

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.

@korinne
Copy link
Contributor

korinne commented Feb 1, 2023

Thanks for submitting a pull request! Linking this to the original issue, so that we can review more thoroughly.

@AneethAnand AneethAnand force-pushed the federation/MakeInitQPStorePublic branch from d7d5e69 to fcb10d2 Compare February 13, 2023 13:39
@changeset-bot
Copy link

changeset-bot bot commented Feb 13, 2023

🦋 Changeset detected

Latest commit: 224e18e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 7 packages
Name Type
@apollo/gateway Patch
@apollo/federation-internals Patch
@apollo/composition Patch
@apollo/query-planner 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

@AneethAnand AneethAnand changed the title Make initQueryPlanStore public and queryPlanStoreKey a hash of both t… Make queryPlanStoreKey a hash of both the query and Operation name Feb 13, 2023
@korinne korinne added the P2 An issue that needs to be addressed on a reasonable timescale. label Feb 22, 2023
@trevor-scheer
Copy link
Member

trevor-scheer commented Feb 23, 2023

@AneethAnand can you please implement the change that Sylvain requested and add a patch changeset?

@AneethAnand
Copy link
Contributor Author

@AneethAnand can you please implement the change that Sylvain requested and add a patch changeset?

Incorporated your review comments! Thanks again for your inputs

Copy link
Member

@trevor-scheer trevor-scheer left a comment

Choose a reason for hiding this comment

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

Thanks @AneethAnand!

@trevor-scheer trevor-scheer merged commit ccbf459 into apollographql:main Feb 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 An issue that needs to be addressed on a reasonable timescale. status/needs-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unwarranted large QPcache sizes because of not hashing the operation name for queryPlanStoreKey
5 participants