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

Improve error of empty block queries #3015

Merged
merged 2 commits into from
Feb 15, 2019

Conversation

srfrog
Copy link
Contributor

@srfrog srfrog commented Feb 14, 2019

Closes #3006


This change is Reviewable

@srfrog srfrog requested a review from a team February 14, 2019 03:02
Copy link
Contributor

@danielmai danielmai left a comment

Choose a reason for hiding this comment

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

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


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

			gq.Alias != "shortest" && !gq.IsEmpty) {
			return x.Errorf("Invalid query, no function used at root and no aggregation" +
				" or math variables found in the body.")

This looks good. I'd make this two sentences. "Invalid query. No function ... "

IIRC, there was a change that shows the line/column number of the error in question. This won't have that, right?

Copy link
Contributor Author

@srfrog srfrog left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @danielmai)


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

Previously, danielmai (Daniel Mai) wrote…

This looks good. I'd make this two sentences. "Invalid query. No function ... "

IIRC, there was a change that shows the line/column number of the error in question. This won't have that, right?

No it wont

Copy link
Contributor

@manishrjain manishrjain left a comment

Choose a reason for hiding this comment

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

Good to go from my side, once @danielmai 's comment is addressed and he gives an lgtm.

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @danielmai)

Copy link
Contributor

@danielmai danielmai left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved


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

Previously, srfrog (Gus) wrote…

No it wont

Ok. Make it two sentences and then :lgtm:.

@srfrog srfrog merged commit 684bec9 into master Feb 15, 2019
@srfrog srfrog deleted the srfrog/issue-3006_improve_error_message branch February 15, 2019 03:32
dna2github pushed a commit to dna2fork/dgraph that referenced this pull request Jul 19, 2019
* query/query.go: improve error of empty block queries

* query/query.go: Changed error into two sentences
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants