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 for panic in fillGroupedVars #3781

Merged
merged 3 commits into from Aug 9, 2019

Conversation

@pawanrawal
Copy link
Member

commented Aug 9, 2019

Fixes #3768

For a uid child, DestUIDs can be nil if there were no source UIDs. This check was already happening in formResult but has been missed in this function. I am working on adding more code comments and trying to reduce the code duplication for the groupby code if possible in another PR.


This change is Reviewable

@pawanrawal pawanrawal requested review from manishrjain and dgraph-io/team as code owners Aug 9, 2019
Copy link

left a comment

A review job has been created and sent to the PullRequest network.


@pawanrawal you can click here to see the review status or cancel the code review job.

Copy link
Member

left a comment

:lgtm: Got a comment.

Reviewed 2 of 2 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @pawanrawal)


query/groupby.go, line 298 at r1 (raw file):

			attr = child.Attr
		}
		if child.DestUIDs != nil && len(child.DestUIDs.Uids) != 0 {

Use child.DestUIDs.GetUids() which already takes care of DestUIDs being nil.

@pawanrawal pawanrawal merged commit 99652e6 into master Aug 9, 2019
4 of 5 checks passed
4 of 5 checks passed
code-review/reviewable 1 file, 1 discussion left (manishrjain, pawanrawal)
Details
Blockade (dgraph) TeamCity build finished
Details
CI (dgraph) TeamCity build finished
Details
GolangCI No issues found!
Details
license/cla Contributor License Agreement is signed.
Details
pawanrawal added a commit that referenced this pull request Aug 9, 2019
Use DestUIDs() function while accessing uids from child in GroupBy query. This fixes #3768.
pawanrawal added a commit that referenced this pull request Aug 9, 2019
Use DestUIDs() function while accessing uids from child in GroupBy query. This fixes #3768.
@pawanrawal pawanrawal deleted the pawan/fix-3768 branch Aug 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.