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

Update gateway to build & use new Query Plan format #4410

Merged
merged 19 commits into from
Aug 31, 2020

Conversation

JakeDawkins
Copy link
Contributor

@JakeDawkins JakeDawkins commented Jul 23, 2020

resolves #4265

This PR aims to rebuild the query planner and executor for apollo-gateway to use the new query plan format, as defined in #4265

#4448 was merged into this PR and were already reviewed, so I'm going to leave some comments on this PR as to what files actually are unreviewed and need reviewing.

It's probably best not to review this PR by commit. Since these changes were made prior to the query plan builder, there was some translation work that had to be put in place to actually produce the new query plans for tests of the executor. Those changes were ripped out in #4448.

Changes:

  • most of the changes made to the executor were focused around referencing selections instead of selectionSet.selections since we flattened that structure.
  • other changes were to reference operation instead of source on fetchNodes

* move new getResponseName to be type safe with qp builder
* update query planner to produce new fetchNodes
* replace query plan types entirely
* swap over build/execute qp to new types
* update snapshots for manual checking
* update QP serializer so we can pretty print
* update qp serializer to remove _entities printing
* adjust function name to make more sense
* fix imports/exports for QP types
* fix cucumber & other serialization for tests
* fix whitespace diffs in snapshots
* remove old qp transform in tests
* remove unused serializer function
* address reviews,  add comments

- add new flattenEntitiesfield in place of old function which did a fuzzy check of a
  query's signature to eliminate the _entities field from a pretty-printed query plan.
  - this new function should be much safer, and harder to fool by accident
  - the rename also makes more sense for what it's doing -- it flattens an _entities operation
    two levels—getting rid of the `query {` and `_entities` and flattens other query definitions
    just to remove `query {`
- fix added newlines from the pretty-printed query plan serializer
- format files
- add plenty of comments and descriptions around utility functions

* update variable declaration to use const
@JakeDawkins JakeDawkins changed the title Update query plan executor to use new Query Plan format Update gateway to build & use new Query Plan format Aug 25, 2020
Copy link
Member

@abernix abernix left a comment

Choose a reason for hiding this comment

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

Nice!

packages/apollo-gateway/src/buildQueryPlan.ts Outdated Show resolved Hide resolved
@JakeDawkins JakeDawkins merged commit f515e89 into main Aug 31, 2020
@JakeDawkins JakeDawkins deleted the jake/planner-executor-updates branch August 31, 2020 12:57
abernix pushed a commit to apollographql/federation that referenced this pull request Sep 4, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change query planner AST nodes into well-defined structures.
3 participants