feat(GraphQL): Disallow DQL schema changes for predicates used in GraphQL schema (DGRAPH-3245) #7742

abhimanyusinghgaur · 2021-04-21T13:21:47Z

See Discuss Issue for more details.

This change is

pawanrawal

nice changes

Reviewed 2 of 2 files at r1.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @abhimanyusinghgaur, @manishrjain, and @vvbalaji-dgraph)

edgraph/server.go, line 563 at r1 (raw file):

	// convert the existing GraphQL schema to a DQL schema
	handler, err := gqlSchema.NewHandler(existingGQLSch, false)

We can't get all of this from the schema that we have in memory?

edgraph/server.go, line 579 at r1 (raw file):

	gqlReservedPreds := make(map[string]*pb.SchemaUpdate)
	gqlReservedTypes := make(map[string]*pb.TypeUpdate)
	for _, pred := range gqlReservedDgSch.Preds {

I suppose this uses the @dgraph directive pred name where one is available?

edgraph/server.go, line 605 at r1 (raw file):

				dgType = "[" + dgType + "]"
			}
			return errors.Errorf("type mismatch for GraphQL predicate %s. want: %s, got: %s",

What's the exact message that's returned to the user? Is it clear that this predicate is being used by the GraphQL API and the schema change is incompatible with the GraphQL def? We should make sure that the error is clear to the user here.

edgraph/server.go, line 622 at r1 (raw file):

			}
		}
		// if gqlSchema had @reverse, then dqlSchema must have it. dqlSchema can't remove @reverse.

Can gqlSchema even have reverse? Would that be for predicates added before a GraphQL API was formed?

edgraph/server.go, line 629 at r1 (raw file):

				x.ParseAttr(gqlPred.Predicate))
		}
		// if gqlSchema had @count, then dqlSchema must have it. dqlSchema can't remove @count.

I thought that dqlSchema could change @count and reverse but not other indexes?

edgraph/server.go, line 662 at r1 (raw file):

		}

		// check that all the fields of the gqlType must be present in the dqlType

Interesting, this is useful for delete I guess?

edgraph/server.go, line 665 at r1 (raw file):

		for _, f := range gqlType.Fields {
			if !dqlFields[f.Predicate] {
				return errors.Errorf("missing field %s in GraphQL type %s",

Just make sure the error messages returned in all the cases are easy to understand for the user.

graphql/e2e/schema/schema_test.go, line 739 at r1 (raw file):

		Person.relative: uid .
		`,
	}).Error(), "type mismatch for GraphQL predicate Person.age. want: int, got: float")

Cannot alter predicate Person.age, type definition is incompatible with what is expected by the GraphQL API for type: Person, field: age, want: int, got: float

Is the above possible?

abhimanyusinghgaur

Reviewable status: 0 of 2 files reviewed, 8 unresolved discussions (waiting on @manishrjain, @pawanrawal, and @vvbalaji-dgraph)

edgraph/server.go, line 563 at r1 (raw file):