-
Notifications
You must be signed in to change notification settings - Fork 272
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
fix(federation): fix lifting selections when flattening inline fragments #6082
Conversation
@goto-bus-stop, please consider creating a changeset entry in |
CI performance tests
|
@@ -311,16 +305,18 @@ impl InlineFragmentSelection { | |||
// applied recursively. This could be worth investigating. | |||
let rebased_inline_fragment = | |||
self.inline_fragment.rebase_on(parent_type, schema)?; | |||
let mut mutable_selections = self.selection_set.selections.clone(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was the cause: when lifting selections, we were looking at the un-flattened original selection set, instead of the flattened one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Also, I like the clean up work 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see a reduction of mismatches from various graphs. Nice!
Some of our customers have a query shape like this:
Here, we know that the second
MySpecialNode
fragment is redundant, as the first instance of the fragment will always already select thenodes.value
field. We currently do not remove the redundant fragment.This PR fixes
flatten_unnecessary_fragments
so it matches JS QP and removes the second instance of the fragment. The actual fix is 7e9adbd, the other changes in simplify.rs are to add a test and to remove some unnecessary clones.