-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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(GraphQL): Add support for repeatitive fields in aggregate query #7094
Conversation
✔️ Deploy preview for dgraph-docs ready! 🔨 Explore the source changes: a87eff4 🔍 Inspect the deploy logs: https://app.netlify.com/sites/dgraph-docs/deploys/5fd0d2e32ceb35000764cecb 😎 Browse the preview: https://deploy-preview-7094--dgraph-docs.netlify.app |
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.
Reviewed 4 of 4 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @abhimanyusinghgaur, @MichaelJCompton, @minhaj-shakeel, and @vmrajas)
graphql/e2e/common/query.go, line 2929 at r1 (raw file):
tmin : titleMin tmin_again : titleMin tmax: titleMax
Also, request this twice
graphql/resolve/query_rewriter.go, line 193 at r1 (raw file):
// Add selection set to mainQuery and finalMainQuery. isAggregateFieldVisited := make(map[string]bool) // isAggregateFunctionVisited stores if the aggregate function has been added or not.
function for a field has been added or not. So the map entries would contain keys as nameMin, ageMin, nameName, etc.
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.
Reviewable status: 2 of 4 files reviewed, 2 unresolved discussions (waiting on @abhimanyusinghgaur, @MichaelJCompton, @minhaj-shakeel, and @pawanrawal)
graphql/e2e/common/query.go, line 2929 at r1 (raw file):
Previously, pawanrawal (Pawan Rawal) wrote…
Also, request this twice
Done.
graphql/resolve/query_rewriter.go, line 193 at r1 (raw file):
Previously, pawanrawal (Pawan Rawal) wrote…
function for a field has been added or not. So the map entries would contain keys as nameMin, ageMin, nameName, etc.
Done.
Motivation:
Recently, support for Aggregation Queries was added to GraphQL. It was discovered later on that there existed a bug in Aggregate Queries. The bug is that adding a repetitive aggregate field generates an incorrect DQL query.
Example:
generated a wrong DQL query and gave a malformed DQL query error.
This PR fixes this.
Testing:
Fixes GRAPHQL-882
This change is