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

fix(Query): Do an error check before bubbling up nil error #8769

Merged
merged 5 commits into from
Mar 28, 2023

Conversation

all-seeing-code
Copy link
Contributor

@all-seeing-code all-seeing-code commented Mar 25, 2023

Problem

Without the check, the code erraneously bubbles up nil error even when there's been an error in one of the child subgraph. This later causes a panic because the downstream code expects no error.

Reproduce this:

DQL Query:

{
    var(func: eq(name, "Alicey")){
        friend(orderdesc: id) {
            f as count(uid)
        }
    }
    q(func: uid(f)){
        name
    } 
}

Fixes: https://github.com/dgraph-io/projects-internal/issues/89

Solution

Add an error check.

@dgraph-bot dgraph-bot added area/core internal mechanisms go Pull requests that update Go code labels Mar 25, 2023
Copy link
Contributor

@skrdgraph skrdgraph left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@all-seeing-code all-seeing-code merged commit b256f9c into main Mar 28, 2023
@all-seeing-code all-seeing-code deleted the anurag/query-crash branch March 28, 2023 17:08
@@ -2363,7 +2363,7 @@ func ProcessGraph(ctx context.Context, sg, parent *SubGraph, rch chan error) {
}
}

if sg.DestUIDs == nil || len(sg.DestUIDs.Uids) == 0 {
if (sg.DestUIDs == nil || len(sg.DestUIDs.Uids) == 0) && childErr == nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a comment explaining the isseu?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/core internal mechanisms go Pull requests that update Go code
Development

Successfully merging this pull request may close these issues.

5 participants