fix(query): Do not execute filters if there are no source uids (#7962… #8452
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
all-seeing-code
requested review from
akon-dey,
skrdgraph and
darkn3rd
as code owners
|
|
all-seeing-code
added
the
cherry-pick-22.0.2
MichelDiz
approved these changes
Nov 23, 2022
| @@ -230,25 +230,27 @@ type Function struct { | |||
| // SubGraph is the way to represent data. It contains both the request parameters and the response. | |||
| // Once generated, this can then be encoded to other client convenient formats, like GraphQL / JSON. | |||
| // SubGraphs are recursively nested. Each SubGraph contain the following: | |||
| // * SrcUIDS: A list of UIDs that were sent to this query. If this subgraph is a child graph, then the | |||
There was a problem hiding this comment.
Are we changing anything significant in the comments?
There was a problem hiding this comment.
No, we are not. But these are go fmt changes which can be safely accepted.
| @@ -2210,6 +2212,10 @@ func ProcessGraph(ctx context.Context, sg, parent *SubGraph, rch chan error) { | |||
| } | |||
|
|
|||
| filter.SrcUIDs = sg.DestUIDs | |||
| if len(filter.SrcUIDs.Uids) == 0 { | |||
There was a problem hiding this comment.
Is there an existing test for this scenario?
There was a problem hiding this comment.
I added a test TestFilterWithNoSrcUid here.
all-seeing-code
force-pushed
the
cherry-pick-7962
branch
from
91db90e to
cce6ab7
Compare
nswamy
approved these changes
Nov 28, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No need to execute filter subgraph if there are no source UIDs.
(cherry picked from commit 849b587)
Description
The original PR adds a fail fast methodology, where it doesn't expand on a filter subgraph if the srcUid is nil. The current cherry-pick PR brings that change in and adds a test case that scenario.
Test
TestFilterWithNoSrcUid