Feat(GraphQL): Add count queries feature at non-root levels #6834

vmrajas · 2020-11-03T13:38:20Z

Fixes GRAPHQL-746
Motivation:
Relate RFC: https://discuss.dgraph.io/t/count-queries-in-graphql/10714/9
The PR adds the following things:

Adds aggregate_ fields to graphql output schema.
Adds functionality to do count queries at non-root levels in GraphQL
Adds tests in graphql/resolve/auth_query_test.yaml and graphql/resolve/query_test.yaml
Adds e2e tests in graphql/e2e/common/

This change is

Docs Preview:

pawanrawal

Reviewed 14 of 40 files at r1.
Reviewable status: 14 of 40 files reviewed, 12 unresolved discussions (waiting on @MichaelJCompton, @pawanrawal, and @vmrajas)

graphql/e2e/common/query.go, line 2452 at r1 (raw file):

				name
				ag : aggregate_states {
                	count

Some extra spaces at the end here and also below.

graphql/e2e/common/query.go, line 2465 at r1 (raw file):

				"name": "India",
				"ag": { 
					"count" : 3

I thought India has more states 😅

graphql/resolve/auth_query_test.yaml, line 1286 at r1 (raw file):

    }

- name: "Count at child and other child level fields with Auth deep filter and field filter"

This test could probably be clubbed with the second test above.

graphql/resolve/query_rewriter.go, line 736 at r1 (raw file):

		}
	}
	rbac := auth.evaluateStaticRules(constructedForType)

Is there no way to reuse some of the code below from another function?

graphql/resolve/query_test.yaml, line 2561 at r1 (raw file):

      queryCountry {
        nm : name
        ag : aggregate_states{

good to have space before {

graphql/schema/gqlschema.go, line 859 at r1 (raw file):

		addTypeHasFilter(sch, defn)
		// We need to call this at last as aggregateFields
		// should not be part of hasfilter of updatepayloadtype etc.

of HasFilter or UpdatePayloadType etc.

graphql/schema/gqlschema.go, line 1139 at r1 (raw file):

		// list types of kind Object or Interface
		// (not scalar lists or not singleton types or lists of other kinds).
		// TODO: Add aggregateField generation for fields of type list[Union]

Create a JIRA ticket for this and remove the TODO from here.

graphql/schema/gqlschema.go, line 1144 at r1 (raw file):

				schema.Types[fld.Type.Name()].Kind == ast.Interface) {
			aggregateField := &ast.FieldDefinition{
				Name: "aggregate_" + fld.Name,

I think this should be in camelcase like aggregateUser as that's the convention for graphql field names. Also, do we have a check somewhere that disallows these names for other fields?

graphql/schema/gqlschema.go, line 1150 at r1 (raw file):

			}
			addFilterArgumentForField(schema, aggregateField, fld.Type.Name())
			aggregateFields = append(aggregateFields, aggregateField)

Why not just add the aggregateField to defn.Fields directly here? You won't need aggregateFields then.

graphql/schema/wrappers.go, line 874 at r1 (raw file):

func (f *field) IsAggregateField() bool {
	return strings.HasPrefix(f.DgraphAlias(), "aggregate_") &&

Do we need to check both or could we just check one of the conditions?

graphql/schema/wrappers.go, line 1421 at r1 (raw file):

}

func (f *field) ConstructedFor() Type {

Add some comments about what this function does on a field.

graphql/schema/wrappers.go, line 1463 at r1 (raw file):

func (f *field) ConstructedForDgraphPredicate() string {
	if !f.IsAggregateField() {

Add some comments about what this function returns here please and what does 10 below signify.

vmrajas

Reviewable status: 14 of 40 files reviewed, 12 unresolved discussions (waiting on @MichaelJCompton and @pawanrawal)

graphql/e2e/common/query.go, line 2452 at r1 (raw file):