From 42a3acc8138a72b5f993c1f0e7a40fbfd716306b Mon Sep 17 00:00:00 2001
From: Abhimanyu Singh Gaur
 <12651351+abhimanyusinghgaur@users.noreply.github.com>
Date: Tue, 24 Mar 2020 21:04:36 +0530
Subject: [PATCH 1/8] fix @dgraph working with @search

---
 graphql/schema/schemagen.go | 129 ++++++++++++++++++++++++++++--------
 1 file changed, 100 insertions(+), 29 deletions(-)

diff --git a/graphql/schema/schemagen.go b/graphql/schema/schemagen.go
index 722be274d75..b2c7a645115 100644
--- a/graphql/schema/schemagen.go
+++ b/graphql/schema/schemagen.go
@@ -132,7 +132,11 @@ func NewHandler(input string) (Handler, error) {
 		return nil, gqlErrList
 	}
 
-	dgSchema := genDgSchema(sch, defns)
+	dgSchema, gqlErrList := genDgSchema(sch, defns)
+	if gqlErrList != nil {
+		return nil, gqlErrList
+	}
+
 	completeSchema(sch, defns)
 
 	return &handler{
@@ -182,13 +186,21 @@ func fieldName(def *ast.FieldDefinition, typName string) string {
 	return predArg.Value.Raw
 }
 
+func getDgraphTypeError(f *ast.FieldDefinition, defName, typStr string) *gqlerror.Error {
+	return gqlerror.ErrorPosf(f.Position,
+		"Type: %s; Field: %s has its dgraph Type: %s; which is different from a previous field"+
+			" with same dgraph predicate name.", defName, f.Name, typStr)
+}
+
 // genDgSchema generates Dgraph schema from a valid graphql schema.
-func genDgSchema(gqlSch *ast.Schema, definitions []string) string {
+func genDgSchema(gqlSch *ast.Schema, definitions []string) (string, gqlerror.List) {
 	var typeStrings []string
+	var errs []*gqlerror.Error
 
 	type dgPred struct {
 		typ     string
-		index   string
+		gqlType string
+		indexes map[string]bool
 		upsert  string
 		reverse string
 	}
@@ -215,8 +227,6 @@ func genDgSchema(gqlSch *ast.Schema, definitions []string) string {
 			typName := typeName(def)
 
 			typ := dgType{name: typName}
-			var typeDef strings.Builder
-			fmt.Fprintf(&typeDef, "type %s {\n", typName)
 			fd := getPasswordField(def)
 
 			for _, f := range def.Fields {
@@ -234,6 +244,15 @@ func genDgSchema(gqlSch *ast.Schema, definitions []string) string {
 				}
 				fname := fieldName(f, typName)
 
+				if edge, ok := dgPreds[fname]; ok && edge.gqlType != f.Type.Name() && edge.
+					reverse == "" {
+					errs = append(errs, gqlerror.ErrorPosf(f.Position,
+						"Type: %s; Field: %s has its GraphQL Type: %s; which is different from a"+
+							" previous field with same dgraph predicate name.", def.Name, f.Name,
+						f.Type.Name()))
+					continue
+				}
+
 				var prefix, suffix string
 				if f.Type.Elem != nil {
 					prefix = "["
@@ -253,9 +272,15 @@ func genDgSchema(gqlSch *ast.Schema, definitions []string) string {
 							forwardPred.reverse = "@reverse "
 							dgPreds[forwardEdge] = forwardPred
 						} else {
-							edge := dgPreds[fname]
-							edge.typ = typStr
-							dgPreds[fname] = edge
+							edge, ok := dgPreds[fname]
+							if ok && edge.typ != "" && edge.typ != typStr {
+								errs = append(errs, getDgraphTypeError(f, def.Name, typStr))
+								continue
+							} else {
+								edge.typ = typStr
+								edge.gqlType = f.Type.Name()
+								dgPreds[fname] = edge
+							}
 						}
 					}
 					typ.fields = append(typ.fields, field{fname, parentInt != nil})
@@ -265,52 +290,72 @@ func genDgSchema(gqlSch *ast.Schema, definitions []string) string {
 						prefix, scalarToDgraph[f.Type.Name()], suffix,
 					)
 
-					indexStr := ""
+					indexes := make([]string, 0)
 					upsertStr := ""
 					search := f.Directives.ForName(searchDirective)
 					id := f.Directives.ForName(idDirective)
 					if id != nil {
 						upsertStr = "@upsert "
+						indexes = []string{"hash"}
 					}
 
 					if search != nil {
 						arg := search.Arguments.ForName(searchArgs)
 						if arg != nil {
-							indexes := getAllSearchIndexes(arg.Value)
-							indexes = addHashIfRequired(f, indexes)
-							indexStr = fmt.Sprintf(" @index(%s)", strings.Join(indexes, ", "))
+							indexes = append(getAllSearchIndexes(arg.Value), indexes...)
 						} else {
-							indexStr = fmt.Sprintf(" @index(%s)", defaultSearches[f.Type.Name()])
+							indexes = append(indexes, defaultSearches[f.Type.Name()])
 						}
-					} else if id != nil {
-						indexStr = fmt.Sprintf(" @index(hash)")
 					}
 
 					if parentInt == nil {
-						dgPreds[fname] = dgPred{
-							typ:    typStr,
-							index:  indexStr,
-							upsert: upsertStr,
+						edge, ok := dgPreds[fname]
+						if ok && edge.typ != typStr {
+							errs = append(errs, getDgraphTypeError(f, def.Name, typStr))
+							continue
+						} else {
+							edge = dgPred{
+								typ:     typStr,
+								gqlType: f.Type.Name(),
+								indexes: make(map[string]bool),
+							}
 						}
+						if edge.upsert == "" {
+							edge.upsert = upsertStr
+						}
+						for _, index := range indexes {
+							edge.indexes[index] = true
+						}
+						dgPreds[fname] = edge
 					}
 					typ.fields = append(typ.fields, field{fname, parentInt != nil})
 				case ast.Enum:
 					typStr = fmt.Sprintf("%s%s%s", prefix, "string", suffix)
 
-					indexStr := " @index(hash)"
+					indexes := []string{"hash"}
 					search := f.Directives.ForName(searchDirective)
 					if search != nil {
 						arg := search.Arguments.ForName(searchArgs)
 						if arg != nil {
-							indexes := getAllSearchIndexes(arg.Value)
-							indexStr = fmt.Sprintf(" @index(%s)", strings.Join(indexes, ", "))
+							indexes = getAllSearchIndexes(arg.Value)
 						}
 					}
 					if parentInt == nil {
-						dgPreds[fname] = dgPred{
-							typ:   typStr,
-							index: indexStr,
+						edge, ok := dgPreds[fname]
+						if ok && edge.typ != typStr {
+							errs = append(errs, getDgraphTypeError(f, def.Name, typStr))
+							continue
+						} else {
+							edge = dgPred{
+								typ:     typStr,
+								gqlType: f.Type.Name(),
+								indexes: make(map[string]bool),
+							}
 						}
+						for _, index := range indexes {
+							edge.indexes[index] = true
+						}
+						dgPreds[fname] = edge
 					}
 					typ.fields = append(typ.fields, field{fname, parentInt != nil})
 				}
@@ -321,9 +366,19 @@ func genDgSchema(gqlSch *ast.Schema, definitions []string) string {
 					typName = typeName(parentInt)
 				}
 				fname := fieldName(fd, typName)
+				typStr := "password"
 
 				if parentInt == nil {
-					dgPreds[fname] = dgPred{typ: "password"}
+					if edge, ok := dgPreds[fname]; ok && edge.typ != typStr {
+						errs = append(errs, getDgraphTypeError(fd, def.Name, typStr))
+						continue
+					} else {
+						edge = dgPred{
+							typ:     typStr,
+							gqlType: fd.Type.Name(),
+						}
+						dgPreds[fname] = edge
+					}
 				}
 
 				typ.fields = append(typ.fields, field{fname, parentInt != nil})
@@ -332,6 +387,7 @@ func genDgSchema(gqlSch *ast.Schema, definitions []string) string {
 		}
 	}
 
+	predWritten := make(map[string]bool, len(dgPreds))
 	for _, typ := range dgTypes {
 		var typeDef, preds strings.Builder
 		fmt.Fprintf(&typeDef, "type %s {\n", typ.name)
@@ -341,9 +397,24 @@ func genDgSchema(gqlSch *ast.Schema, definitions []string) string {
 				continue
 			}
 			fmt.Fprintf(&typeDef, "  %s\n", fld.name)
-			if !fld.inherited {
-				fmt.Fprintf(&preds, "%s: %s%s %s%s.\n", fld.name, f.typ, f.index, f.upsert,
+			if !fld.inherited && !predWritten[fld.name] {
+				var indexStr strings.Builder
+				indexCount := len(f.indexes)
+				if indexCount > 0 {
+					i := 1
+					indexStr.WriteString(" @index(")
+					for index, _ := range f.indexes {
+						indexStr.WriteString(index)
+						if i != indexCount {
+							indexStr.WriteString(", ")
+						}
+						i++
+					}
+					indexStr.WriteString(")")
+				}
+				fmt.Fprintf(&preds, "%s: %s%s %s%s.\n", fld.name, f.typ, indexStr.String(), f.upsert,
 					f.reverse)
+				predWritten[fld.name] = true
 			}
 		}
 		fmt.Fprintf(&typeDef, "}\n")
@@ -353,5 +424,5 @@ func genDgSchema(gqlSch *ast.Schema, definitions []string) string {
 		)
 	}
 
-	return strings.Join(typeStrings, "")
+	return strings.Join(typeStrings, ""), errs
 }

From 1962e19df5db0b8eabb54e18d61dee6fc08f9084 Mon Sep 17 00:00:00 2001
From: Abhimanyu Singh Gaur
 <12651351+abhimanyusinghgaur@users.noreply.github.com>
Date: Tue, 24 Mar 2020 23:38:09 +0530
Subject: [PATCH 2/8] add tests

---
 graphql/schema/gqlschema_test.yml | 35 ++++++++++++++++++++++++++
 graphql/schema/schemagen.go       | 28 +++++++++------------
 graphql/schema/schemagen_test.yml | 42 ++++++++++++++++++++++++++++++-
 3 files changed, 88 insertions(+), 17 deletions(-)

diff --git a/graphql/schema/gqlschema_test.yml b/graphql/schema/gqlschema_test.yml
index bbbe62dcbc1..d0f911a47d2 100644
--- a/graphql/schema/gqlschema_test.yml
+++ b/graphql/schema/gqlschema_test.yml
@@ -707,6 +707,41 @@ invalid_schemas:
       "locations":[{"line":1, "column":6}]},
     ]
 
+  -
+    name: "Conflicting GraphQL types for predicate"
+    input: |
+      type X {
+        f1: String! @dgraph(pred: "name")
+        f2: Y @dgraph(pred: "link")
+      }
+      type Y {
+        f1: Int @dgraph(pred: "name")
+        f2: X @dgraph(pred: "link")
+      }
+    errlist: [
+    {"message": "Type: Y; Field: f1 has its GraphQL Type: Int; which is different from a previous field with same dgraph predicate.",
+     "locations":[{"line":6, "column":3}]},
+    {"message": "Type: Y; Field: f2 has its GraphQL Type: X; which is different from a previous field with same dgraph predicate.",
+     "locations":[{"line":7, "column":3}]},
+    ]
+
+  -
+    name: "Conflicting dgraph types for predicate"
+    input: |
+      type X @secret(field: "f", pred: "pwd"){
+        f1: String @dgraph(pred: "name")
+      }
+      type Y {
+        f1: [String] @dgraph(pred: "name")
+        f2: String @dgraph(pred: "pwd")
+      }
+    errlist: [
+    {"message": "Type: Y; Field: f1 has its dgraph Type: [string]; which is different from a previous field with same dgraph predicate.",
+     "locations":[{"line":5, "column":3}]},
+    {"message": "Type: Y; Field: f2 has its dgraph Type: string; which is different from a previous field with same dgraph predicate.",
+     "locations":[{"line":6, "column":3}]},
+    ]
+
 
 
 valid_schemas:
diff --git a/graphql/schema/schemagen.go b/graphql/schema/schemagen.go
index b2c7a645115..6b15a4d929a 100644
--- a/graphql/schema/schemagen.go
+++ b/graphql/schema/schemagen.go
@@ -18,6 +18,7 @@ package schema
 
 import (
 	"fmt"
+	"sort"
 	"strings"
 
 	"github.com/pkg/errors"
@@ -189,7 +190,7 @@ func fieldName(def *ast.FieldDefinition, typName string) string {
 func getDgraphTypeError(f *ast.FieldDefinition, defName, typStr string) *gqlerror.Error {
 	return gqlerror.ErrorPosf(f.Position,
 		"Type: %s; Field: %s has its dgraph Type: %s; which is different from a previous field"+
-			" with same dgraph predicate name.", defName, f.Name, typStr)
+			" with same dgraph predicate.", defName, f.Name, typStr)
 }
 
 // genDgSchema generates Dgraph schema from a valid graphql schema.
@@ -248,7 +249,7 @@ func genDgSchema(gqlSch *ast.Schema, definitions []string) (string, gqlerror.Lis
 					reverse == "" {
 					errs = append(errs, gqlerror.ErrorPosf(f.Position,
 						"Type: %s; Field: %s has its GraphQL Type: %s; which is different from a"+
-							" previous field with same dgraph predicate name.", def.Name, f.Name,
+							" previous field with same dgraph predicate.", def.Name, f.Name,
 						f.Type.Name()))
 					continue
 				}
@@ -313,7 +314,7 @@ func genDgSchema(gqlSch *ast.Schema, definitions []string) (string, gqlerror.Lis
 						if ok && edge.typ != typStr {
 							errs = append(errs, getDgraphTypeError(f, def.Name, typStr))
 							continue
-						} else {
+						} else if !ok {
 							edge = dgPred{
 								typ:     typStr,
 								gqlType: f.Type.Name(),
@@ -345,7 +346,7 @@ func genDgSchema(gqlSch *ast.Schema, definitions []string) (string, gqlerror.Lis
 						if ok && edge.typ != typStr {
 							errs = append(errs, getDgraphTypeError(f, def.Name, typStr))
 							continue
-						} else {
+						} else if !ok {
 							edge = dgPred{
 								typ:     typStr,
 								gqlType: f.Type.Name(),
@@ -398,21 +399,16 @@ func genDgSchema(gqlSch *ast.Schema, definitions []string) (string, gqlerror.Lis
 			}
 			fmt.Fprintf(&typeDef, "  %s\n", fld.name)
 			if !fld.inherited && !predWritten[fld.name] {
-				var indexStr strings.Builder
-				indexCount := len(f.indexes)
-				if indexCount > 0 {
-					i := 1
-					indexStr.WriteString(" @index(")
+				indexStr := ""
+				if len(f.indexes) > 0 {
+					indexes := make([]string, 0)
 					for index, _ := range f.indexes {
-						indexStr.WriteString(index)
-						if i != indexCount {
-							indexStr.WriteString(", ")
-						}
-						i++
+						indexes = append(indexes, index)
 					}
-					indexStr.WriteString(")")
+					sort.Strings(indexes)
+					indexStr = fmt.Sprintf(" @index(%s)", strings.Join(indexes, ", "))
 				}
-				fmt.Fprintf(&preds, "%s: %s%s %s%s.\n", fld.name, f.typ, indexStr.String(), f.upsert,
+				fmt.Fprintf(&preds, "%s: %s%s %s%s.\n", fld.name, f.typ, indexStr, f.upsert,
 					f.reverse)
 				predWritten[fld.name] = true
 			}
diff --git a/graphql/schema/schemagen_test.yml b/graphql/schema/schemagen_test.yml
index a317d5ac769..5fccd88879a 100644
--- a/graphql/schema/schemagen_test.yml
+++ b/graphql/schema/schemagen_test.yml
@@ -358,7 +358,7 @@ schemas:
       type A {
         A.id
       }
-      A.id: string @index(trigram, hash) @upsert .
+      A.id: string @index(hash, trigram) @upsert .
       type B {
         A.id
         B.correct
@@ -434,3 +434,43 @@ schemas:
       }
       A.p: string .
       A.q: string .
+
+  -
+    name: "fields with same @dgraph(pred: ...) occur only once in schema"
+    input: |
+      type A {
+        p: String @dgraph(pred: "name")
+      }
+      type B {
+        q: String @dgraph(pred: "name")
+        name: String
+      }
+    output: |
+      type A {
+        name
+      }
+      name: string .
+      type B {
+        name
+        B.name
+      }
+      B.name: string .
+
+  -
+    name: "fields with same @dgraph(pred: ...) and different @search(by: [...]) have indexes
+    combined"
+    input: |
+      type A {
+        p: String @dgraph(pred: "name") @search(by: [exact, term])
+      }
+      type B {
+        q: String @dgraph(pred: "name") @search(by: [trigram])
+      }
+    output: |
+      type A {
+        name
+      }
+      name: string @index(exact, term, trigram) .
+      type B {
+        name
+      }

From d967bb17977a6323ef9c381f9eedb346919869c0 Mon Sep 17 00:00:00 2001
From: Abhimanyu Singh Gaur
 <12651351+abhimanyusinghgaur@users.noreply.github.com>
Date: Wed, 25 Mar 2020 00:07:58 +0530
Subject: [PATCH 3/8] fix ci comment

---
 graphql/schema/schemagen.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/graphql/schema/schemagen.go b/graphql/schema/schemagen.go
index 6b15a4d929a..0bf41bffca0 100644
--- a/graphql/schema/schemagen.go
+++ b/graphql/schema/schemagen.go
@@ -402,7 +402,7 @@ func genDgSchema(gqlSch *ast.Schema, definitions []string) (string, gqlerror.Lis
 				indexStr := ""
 				if len(f.indexes) > 0 {
 					indexes := make([]string, 0)
-					for index, _ := range f.indexes {
+					for index := range f.indexes {
 						indexes = append(indexes, index)
 					}
 					sort.Strings(indexes)

From ae76dec6a2cba6cd10db77a800e6f59e96d00494 Mon Sep 17 00:00:00 2001
From: Abhimanyu Singh Gaur
 <12651351+abhimanyusinghgaur@users.noreply.github.com>
Date: Fri, 27 Mar 2020 01:58:48 +0530
Subject: [PATCH 4/8] resolve review comments

---
 graphql/schema/gqlschema.go       |  16 ++++
 graphql/schema/gqlschema_test.yml |  63 ++++++++-------
 graphql/schema/rules.go           | 123 +++++++++++++++++++++++++++++-
 graphql/schema/schemagen.go       |  72 ++++-------------
 4 files changed, 188 insertions(+), 86 deletions(-)

diff --git a/graphql/schema/gqlschema.go b/graphql/schema/gqlschema.go
index 1c43d65c54b..183c9e0aeee 100644
--- a/graphql/schema/gqlschema.go
+++ b/graphql/schema/gqlschema.go
@@ -246,6 +246,7 @@ var directiveValidators = map[string]directiveValidator{
 	},
 }
 
+var schemaValidations []func(schema *ast.Schema, definitions []string) gqlerror.List
 var defnValidations, typeValidations []func(defn *ast.Definition) *gqlerror.Error
 var fieldValidations []func(typ *ast.Definition, field *ast.FieldDefinition) *gqlerror.Error
 
@@ -353,6 +354,21 @@ func postGQLValidation(schema *ast.Schema, definitions []string) gqlerror.List {
 		}
 	}
 
+	errs = append(errs, applySchemaValidations(schema, definitions)...)
+
+	return errs
+}
+
+func applySchemaValidations(schema *ast.Schema, definitions []string) gqlerror.List {
+	var errs []*gqlerror.Error
+
+	for _, rule := range schemaValidations {
+		newErrs := rule(schema, definitions)
+		for _, err := range newErrs {
+			errs = appendIfNotNull(errs, err)
+		}
+	}
+
 	return errs
 }
 
diff --git a/graphql/schema/gqlschema_test.yml b/graphql/schema/gqlschema_test.yml
index d0f911a47d2..876958c3c3a 100644
--- a/graphql/schema/gqlschema_test.yml
+++ b/graphql/schema/gqlschema_test.yml
@@ -708,42 +708,37 @@ invalid_schemas:
     ]
 
   -
-    name: "Conflicting GraphQL types for predicate"
+    name: "@dgraph predicate type validation"
     input: |
-      type X {
+      interface W @secret(field: "f", pred: "pwd") {
         f1: String! @dgraph(pred: "name")
-        f2: Y @dgraph(pred: "link")
+        f2: [Float] @dgraph(pred: "val")
       }
-      type Y {
-        f1: Int @dgraph(pred: "name")
-        f2: X @dgraph(pred: "link")
-      }
-    errlist: [
-    {"message": "Type: Y; Field: f1 has its GraphQL Type: Int; which is different from a previous field with same dgraph predicate.",
-     "locations":[{"line":6, "column":3}]},
-    {"message": "Type: Y; Field: f2 has its GraphQL Type: X; which is different from a previous field with same dgraph predicate.",
-     "locations":[{"line":7, "column":3}]},
-    ]
-
-  -
-    name: "Conflicting dgraph types for predicate"
-    input: |
-      type X @secret(field: "f", pred: "pwd"){
-        f1: String @dgraph(pred: "name")
+      type X implements W {
+        f3: Y @dgraph(pred: "link")
+        f4: String! @dgraph(pred: "f4") @id
       }
       type Y {
-        f1: [String] @dgraph(pred: "name")
-        f2: String @dgraph(pred: "pwd")
+        f1: Int @dgraph(pred: "name")
+        f2: Float @dgraph(pred: "val")
+        f3: X @dgraph(pred: "link")
+        f4: String @dgraph(pred: "f4")
+        f5: String @dgraph(pred: "pwd")
       }
     errlist: [
-    {"message": "Type: Y; Field: f1 has its dgraph Type: [string]; which is different from a previous field with same dgraph predicate.",
-     "locations":[{"line":5, "column":3}]},
-    {"message": "Type: Y; Field: f2 has its dgraph Type: string; which is different from a previous field with same dgraph predicate.",
-     "locations":[{"line":6, "column":3}]},
+    {"message": "Type Y; Field f1: has type Int, which is different to type W; field f1, which has the same @dgraph directive but type String. These fields must have either the same GraphQL types, or use different Dgraph predicates.",
+     "locations":[{"line":10, "column":3}]},
+    {"message": "Type Y; Field f2: has type Float, which is different to type W; field f2, which has the same @dgraph directive but type [Float]. These fields must have either the same GraphQL types, or use different Dgraph predicates.",
+     "locations":[{"line":11, "column":3}]},
+    {"message": "Type Y; Field f3: has type X, which is different to type X; field f3, which has the same @dgraph directive but type Y. These fields must have either the same GraphQL types, or use different Dgraph predicates.",
+     "locations":[{"line":12, "column":3}]},
+    {"message": "Type Y; Field f4: doesn't have @id directive, which conflicts with type X; field f4, which has the same @dgraph directive along with @id directive. Both these fields must either use @id directive, or use different Dgraph predicates.",
+     "locations":[{"line":13, "column":3}]},
+    {"message": "Type Y; Field f5: has the @dgraph predicate, but that conflicts with type W @secret directive on the same predicate. @secret predicates are stored encrypted and so the same predicate can't be used as a String.",
+     "locations":[{"line":14, "column":3}]},
     ]
 
 
-
 valid_schemas:
   -
     name: "hasInverse directive on singleton"
@@ -845,3 +840,19 @@ valid_schemas:
       type Y {
         f1: [X] @dgraph(pred: "f1")
       }
+
+  -
+    name: "@dgraph predicate type validation gives no errors with non-null variations"
+    input: |
+      type X {
+        f1: String @dgraph(pred: "f1")
+        f2: [String] @dgraph(pred: "f2")
+        f3: [String!] @dgraph(pred: "f3")
+        f4: [String]! @dgraph(pred: "f4")
+      }
+      type Y {
+        f1: String! @dgraph(pred: "f1")
+        f2: [String!] @dgraph(pred: "f2")
+        f3: [String]! @dgraph(pred: "f3")
+        f4: [String!]! @dgraph(pred: "f4")
+      }
\ No newline at end of file
diff --git a/graphql/schema/rules.go b/graphql/schema/rules.go
index 8007023fdb9..13fa6f2fb93 100644
--- a/graphql/schema/rules.go
+++ b/graphql/schema/rules.go
@@ -27,6 +27,7 @@ import (
 )
 
 func init() {
+	schemaValidations = append(schemaValidations, dgraphDirectivePredicateValidation)
 	defnValidations = append(defnValidations, dataTypeCheck, nameCheck)
 
 	typeValidations = append(typeValidations, idCountCheck, dgraphDirectiveTypeValidation,
@@ -39,6 +40,126 @@ func init() {
 
 }
 
+func dgraphDirectivePredicateValidation(gqlSch *ast.Schema, definitions []string) gqlerror.List {
+	var errs []*gqlerror.Error
+
+	type pred struct {
+		name       string
+		parentName string
+		typ        string
+		position   *ast.Position
+		isId       bool
+		isSecret   bool
+	}
+
+	secretError := func(secretPred, newPred pred) *gqlerror.Error {
+		return gqlerror.ErrorPosf(newPred.position,
+			"Type %s; Field %s: has the @dgraph predicate, but that conflicts with type %s "+
+				"@secret directive on the same predicate. @secret predicates are stored encrypted"+
+				" and so the same predicate can't be used as a %s.", newPred.parentName,
+			newPred.name, secretPred.parentName, newPred.typ)
+	}
+
+	typeError := func(existingPred, newPred pred) *gqlerror.Error {
+		return gqlerror.ErrorPosf(newPred.position,
+			"Type %s; Field %s: has type %s, which is different to type %s; field %s, which has "+
+				"the same @dgraph directive but type %s. These fields must have either the same "+
+				"GraphQL types, or use different Dgraph predicates.", newPred.parentName,
+			newPred.name, newPred.typ, existingPred.parentName, existingPred.name,
+			existingPred.typ)
+	}
+
+	idError := func(idPred, newPred pred) *gqlerror.Error {
+		return gqlerror.ErrorPosf(newPred.position,
+			"Type %s; Field %s: doesn't have @id directive, which conflicts with type %s; field "+
+				"%s, which has the same @dgraph directive along with @id directive. Both these "+
+				"fields must either use @id directive, or use different Dgraph predicates.",
+			newPred.parentName, newPred.name, idPred.parentName, idPred.name)
+	}
+
+	preds := make(map[string]pred)
+
+	for _, key := range definitions {
+		def := gqlSch.Types[key]
+		switch def.Kind {
+		case ast.Object, ast.Interface:
+			typName := typeName(def)
+
+			for _, f := range def.Fields {
+				if f.Type.Name() == "ID" {
+					continue
+				}
+
+				fname := fieldName(f, typName)
+				// No validation needed if this field doesn't have @dgraph(pred : ...). Also,
+				// this field could have originally been defined in an interface that this type
+				// implements. If we get a parent interface, that means this field gets validated
+				// during the validation of that interface. So, no need to validate this field here.
+				if fname != typName+"."+f.Name && parentInterface(gqlSch, def, f.Name) == nil {
+
+					var prefix, suffix string
+					if f.Type.Elem != nil {
+						prefix = "["
+						suffix = "]"
+					}
+
+					thisPred := pred{
+						name:       f.Name,
+						parentName: def.Name,
+						typ:        fmt.Sprintf("%s%s%s", prefix, f.Type.Name(), suffix),
+						position:   f.Position,
+						isId:       f.Directives.ForName(idDirective) != nil,
+						isSecret:   false,
+					}
+
+					if pred, ok := preds[fname]; ok {
+						if pred.isSecret {
+							errs = append(errs, secretError(pred, thisPred))
+						} else if thisPred.typ != pred.typ {
+							errs = append(errs, typeError(pred, thisPred))
+						}
+						if pred.isId != thisPred.isId {
+							if pred.isId {
+								errs = append(errs, idError(pred, thisPred))
+							} else {
+								errs = append(errs, idError(thisPred, pred))
+							}
+						}
+					} else {
+						preds[fname] = thisPred
+					}
+				}
+			}
+
+			pwdField := getPasswordField(def)
+			if pwdField != nil {
+				fname := fieldName(pwdField, typName)
+				if fname != typName+"."+pwdField.Name && parentInterface(gqlSch, def,
+					pwdField.Name) == nil {
+					thisPred := pred{
+						name:       pwdField.Name,
+						parentName: def.Name,
+						typ:        pwdField.Type.Name(),
+						position:   pwdField.Position,
+						isId:       false,
+						isSecret:   true,
+					}
+
+					if pred, ok := preds[fname]; ok {
+						if thisPred.typ != pred.typ || !pred.isSecret {
+							errs = append(errs, secretError(thisPred, pred))
+						}
+					} else {
+						preds[fname] = thisPred
+					}
+				}
+			}
+		}
+	}
+
+	return errs
+}
+
 func dataTypeCheck(defn *ast.Definition) *gqlerror.Error {
 	if defn.Kind == ast.Object || defn.Kind == ast.Enum || defn.Kind == ast.Interface {
 		return nil
@@ -95,7 +216,7 @@ func passwordDirectiveValidation(typ *ast.Definition) *gqlerror.Error {
 	dirs := make([]string, 0)
 
 	for _, dir := range typ.Directives {
-		if dir.Name != "secret" {
+		if dir.Name != secretDirective {
 			continue
 		}
 		val := dir.Arguments.ForName("field").Value.Raw
diff --git a/graphql/schema/schemagen.go b/graphql/schema/schemagen.go
index 0bf41bffca0..eca840e3d32 100644
--- a/graphql/schema/schemagen.go
+++ b/graphql/schema/schemagen.go
@@ -133,10 +133,7 @@ func NewHandler(input string) (Handler, error) {
 		return nil, gqlErrList
 	}
 
-	dgSchema, gqlErrList := genDgSchema(sch, defns)
-	if gqlErrList != nil {
-		return nil, gqlErrList
-	}
+	dgSchema := genDgSchema(sch, defns)
 
 	completeSchema(sch, defns)
 
@@ -187,20 +184,12 @@ func fieldName(def *ast.FieldDefinition, typName string) string {
 	return predArg.Value.Raw
 }
 
-func getDgraphTypeError(f *ast.FieldDefinition, defName, typStr string) *gqlerror.Error {
-	return gqlerror.ErrorPosf(f.Position,
-		"Type: %s; Field: %s has its dgraph Type: %s; which is different from a previous field"+
-			" with same dgraph predicate.", defName, f.Name, typStr)
-}
-
 // genDgSchema generates Dgraph schema from a valid graphql schema.
-func genDgSchema(gqlSch *ast.Schema, definitions []string) (string, gqlerror.List) {
+func genDgSchema(gqlSch *ast.Schema, definitions []string) string {
 	var typeStrings []string
-	var errs []*gqlerror.Error
 
 	type dgPred struct {
 		typ     string
-		gqlType string
 		indexes map[string]bool
 		upsert  string
 		reverse string
@@ -245,15 +234,6 @@ func genDgSchema(gqlSch *ast.Schema, definitions []string) (string, gqlerror.Lis
 				}
 				fname := fieldName(f, typName)
 
-				if edge, ok := dgPreds[fname]; ok && edge.gqlType != f.Type.Name() && edge.
-					reverse == "" {
-					errs = append(errs, gqlerror.ErrorPosf(f.Position,
-						"Type: %s; Field: %s has its GraphQL Type: %s; which is different from a"+
-							" previous field with same dgraph predicate.", def.Name, f.Name,
-						f.Type.Name()))
-					continue
-				}
-
 				var prefix, suffix string
 				if f.Type.Elem != nil {
 					prefix = "["
@@ -273,15 +253,9 @@ func genDgSchema(gqlSch *ast.Schema, definitions []string) (string, gqlerror.Lis
 							forwardPred.reverse = "@reverse "
 							dgPreds[forwardEdge] = forwardPred
 						} else {
-							edge, ok := dgPreds[fname]
-							if ok && edge.typ != "" && edge.typ != typStr {
-								errs = append(errs, getDgraphTypeError(f, def.Name, typStr))
-								continue
-							} else {
-								edge.typ = typStr
-								edge.gqlType = f.Type.Name()
-								dgPreds[fname] = edge
-							}
+							edge := dgPreds[fname]
+							edge.typ = typStr
+							dgPreds[fname] = edge
 						}
 					}
 					typ.fields = append(typ.fields, field{fname, parentInt != nil})
@@ -291,19 +265,19 @@ func genDgSchema(gqlSch *ast.Schema, definitions []string) (string, gqlerror.Lis
 						prefix, scalarToDgraph[f.Type.Name()], suffix,
 					)
 
-					indexes := make([]string, 0)
+					var indexes []string
 					upsertStr := ""
 					search := f.Directives.ForName(searchDirective)
 					id := f.Directives.ForName(idDirective)
 					if id != nil {
 						upsertStr = "@upsert "
-						indexes = []string{"hash"}
+						indexes = append(indexes, "hash")
 					}
 
 					if search != nil {
 						arg := search.Arguments.ForName(searchArgs)
 						if arg != nil {
-							indexes = append(getAllSearchIndexes(arg.Value), indexes...)
+							indexes = append(indexes, getAllSearchIndexes(arg.Value)...)
 						} else {
 							indexes = append(indexes, defaultSearches[f.Type.Name()])
 						}
@@ -311,19 +285,13 @@ func genDgSchema(gqlSch *ast.Schema, definitions []string) (string, gqlerror.Lis
 
 					if parentInt == nil {
 						edge, ok := dgPreds[fname]
-						if ok && edge.typ != typStr {
-							errs = append(errs, getDgraphTypeError(f, def.Name, typStr))
-							continue
-						} else if !ok {
+						if !ok {
 							edge = dgPred{
 								typ:     typStr,
-								gqlType: f.Type.Name(),
 								indexes: make(map[string]bool),
+								upsert:  upsertStr,
 							}
 						}
-						if edge.upsert == "" {
-							edge.upsert = upsertStr
-						}
 						for _, index := range indexes {
 							edge.indexes[index] = true
 						}
@@ -343,13 +311,9 @@ func genDgSchema(gqlSch *ast.Schema, definitions []string) (string, gqlerror.Lis
 					}
 					if parentInt == nil {
 						edge, ok := dgPreds[fname]
-						if ok && edge.typ != typStr {
-							errs = append(errs, getDgraphTypeError(f, def.Name, typStr))
-							continue
-						} else if !ok {
+						if !ok {
 							edge = dgPred{
 								typ:     typStr,
-								gqlType: f.Type.Name(),
 								indexes: make(map[string]bool),
 							}
 						}
@@ -367,19 +331,9 @@ func genDgSchema(gqlSch *ast.Schema, definitions []string) (string, gqlerror.Lis
 					typName = typeName(parentInt)
 				}
 				fname := fieldName(fd, typName)
-				typStr := "password"
 
 				if parentInt == nil {
-					if edge, ok := dgPreds[fname]; ok && edge.typ != typStr {
-						errs = append(errs, getDgraphTypeError(fd, def.Name, typStr))
-						continue
-					} else {
-						edge = dgPred{
-							typ:     typStr,
-							gqlType: fd.Type.Name(),
-						}
-						dgPreds[fname] = edge
-					}
+					dgPreds[fname] = dgPred{typ: "password"}
 				}
 
 				typ.fields = append(typ.fields, field{fname, parentInt != nil})
@@ -420,5 +374,5 @@ func genDgSchema(gqlSch *ast.Schema, definitions []string) (string, gqlerror.Lis
 		)
 	}
 
-	return strings.Join(typeStrings, ""), errs
+	return strings.Join(typeStrings, "")
 }

From 09fa918b7a2cc932112008a3529d8ff2abb05dde Mon Sep 17 00:00:00 2001
From: Abhimanyu Singh Gaur
 <12651351+abhimanyusinghgaur@users.noreply.github.com>
Date: Tue, 31 Mar 2020 22:59:15 +0530
Subject: [PATCH 5/8] resolve review comments

---
 graphql/schema/gqlschema_test.yml | 16 ++++++---
 graphql/schema/rules.go           |  5 ++-
 graphql/schema/schemagen.go       | 54 ++++++++++++++-----------------
 graphql/schema/schemagen_test.yml |  6 ++++
 4 files changed, 43 insertions(+), 38 deletions(-)

diff --git a/graphql/schema/gqlschema_test.yml b/graphql/schema/gqlschema_test.yml
index 876958c3c3a..0738c663177 100644
--- a/graphql/schema/gqlschema_test.yml
+++ b/graphql/schema/gqlschema_test.yml
@@ -705,6 +705,8 @@ invalid_schemas:
     errlist: [
       {"message": "Type X; has a secret directive and field of the same name f1",
       "locations":[{"line":1, "column":6}]},
+      {"message": "Type X; Field f1: has the @dgraph predicate, but that conflicts with type X @secret directive on the same predicate. @secret predicates are stored encrypted and so the same predicate can't be used as a String.",
+       "locations":[{"line":2, "column":3}]},
     ]
 
   -
@@ -717,6 +719,7 @@ invalid_schemas:
       type X implements W {
         f3: Y @dgraph(pred: "link")
         f4: String! @dgraph(pred: "f4") @id
+        f6: String
       }
       type Y {
         f1: Int @dgraph(pred: "name")
@@ -724,18 +727,21 @@ invalid_schemas:
         f3: X @dgraph(pred: "link")
         f4: String @dgraph(pred: "f4")
         f5: String @dgraph(pred: "pwd")
+        f6: Int @dgraph(pred: "X.f6")
       }
     errlist: [
     {"message": "Type Y; Field f1: has type Int, which is different to type W; field f1, which has the same @dgraph directive but type String. These fields must have either the same GraphQL types, or use different Dgraph predicates.",
-     "locations":[{"line":10, "column":3}]},
-    {"message": "Type Y; Field f2: has type Float, which is different to type W; field f2, which has the same @dgraph directive but type [Float]. These fields must have either the same GraphQL types, or use different Dgraph predicates.",
      "locations":[{"line":11, "column":3}]},
-    {"message": "Type Y; Field f3: has type X, which is different to type X; field f3, which has the same @dgraph directive but type Y. These fields must have either the same GraphQL types, or use different Dgraph predicates.",
+    {"message": "Type Y; Field f2: has type Float, which is different to type W; field f2, which has the same @dgraph directive but type [Float]. These fields must have either the same GraphQL types, or use different Dgraph predicates.",
      "locations":[{"line":12, "column":3}]},
-    {"message": "Type Y; Field f4: doesn't have @id directive, which conflicts with type X; field f4, which has the same @dgraph directive along with @id directive. Both these fields must either use @id directive, or use different Dgraph predicates.",
+    {"message": "Type Y; Field f3: has type X, which is different to type X; field f3, which has the same @dgraph directive but type Y. These fields must have either the same GraphQL types, or use different Dgraph predicates.",
      "locations":[{"line":13, "column":3}]},
-    {"message": "Type Y; Field f5: has the @dgraph predicate, but that conflicts with type W @secret directive on the same predicate. @secret predicates are stored encrypted and so the same predicate can't be used as a String.",
+    {"message": "Type Y; Field f4: doesn't have @id directive, which conflicts with type X; field f4, which has the same @dgraph directive along with @id directive. Both these fields must either use @id directive, or use different Dgraph predicates.",
      "locations":[{"line":14, "column":3}]},
+    {"message": "Type Y; Field f5: has the @dgraph predicate, but that conflicts with type W @secret directive on the same predicate. @secret predicates are stored encrypted and so the same predicate can't be used as a String.",
+     "locations":[{"line":15, "column":3}]},
+    {"message": "Type Y; Field f6: has type Int, which is different to type X; field f6, which has the same @dgraph directive but type String. These fields must have either the same GraphQL types, or use different Dgraph predicates.",
+     "locations":[{"line":16, "column":3}]},
     ]
 
 
diff --git a/graphql/schema/rules.go b/graphql/schema/rules.go
index 13fa6f2fb93..c45dffb1a4e 100644
--- a/graphql/schema/rules.go
+++ b/graphql/schema/rules.go
@@ -91,11 +91,10 @@ func dgraphDirectivePredicateValidation(gqlSch *ast.Schema, definitions []string
 				}
 
 				fname := fieldName(f, typName)
-				// No validation needed if this field doesn't have @dgraph(pred : ...). Also,
 				// this field could have originally been defined in an interface that this type
 				// implements. If we get a parent interface, that means this field gets validated
 				// during the validation of that interface. So, no need to validate this field here.
-				if fname != typName+"."+f.Name && parentInterface(gqlSch, def, f.Name) == nil {
+				if parentInterface(gqlSch, def, f.Name) == nil {
 
 					var prefix, suffix string
 					if f.Type.Elem != nil {
@@ -134,7 +133,7 @@ func dgraphDirectivePredicateValidation(gqlSch *ast.Schema, definitions []string
 			pwdField := getPasswordField(def)
 			if pwdField != nil {
 				fname := fieldName(pwdField, typName)
-				if fname != typName+"."+pwdField.Name && parentInterface(gqlSch, def,
+				if parentInterface(gqlSch, def,
 					pwdField.Name) == nil {
 					thisPred := pred{
 						name:       pwdField.Name,
diff --git a/graphql/schema/schemagen.go b/graphql/schema/schemagen.go
index eca840e3d32..239c14256f3 100644
--- a/graphql/schema/schemagen.go
+++ b/graphql/schema/schemagen.go
@@ -210,6 +210,21 @@ func genDgSchema(gqlSch *ast.Schema, definitions []string) string {
 	dgTypes := make([]dgType, 0, len(definitions))
 	dgPreds := make(map[string]dgPred)
 
+	getUpdatedPred := func(fname, typStr, upsertStr string, indexes []string) dgPred {
+		pred, ok := dgPreds[fname]
+		if !ok {
+			pred = dgPred{
+				typ:     typStr,
+				indexes: make(map[string]bool),
+				upsert:  upsertStr,
+			}
+		}
+		for _, index := range indexes {
+			pred.indexes[index] = true
+		}
+		return pred
+	}
+
 	for _, key := range definitions {
 		def := gqlSch.Types[key]
 		switch def.Kind {
@@ -217,7 +232,7 @@ func genDgSchema(gqlSch *ast.Schema, definitions []string) string {
 			typName := typeName(def)
 
 			typ := dgType{name: typName}
-			fd := getPasswordField(def)
+			pwdField := getPasswordField(def)
 
 			for _, f := range def.Fields {
 				if f.Type.Name() == "ID" {
@@ -253,9 +268,9 @@ func genDgSchema(gqlSch *ast.Schema, definitions []string) string {
 							forwardPred.reverse = "@reverse "
 							dgPreds[forwardEdge] = forwardPred
 						} else {
-							edge := dgPreds[fname]
-							edge.typ = typStr
-							dgPreds[fname] = edge
+							pred := dgPreds[fname]
+							pred.typ = typStr
+							dgPreds[fname] = pred
 						}
 					}
 					typ.fields = append(typ.fields, field{fname, parentInt != nil})
@@ -284,18 +299,7 @@ func genDgSchema(gqlSch *ast.Schema, definitions []string) string {
 					}
 
 					if parentInt == nil {
-						edge, ok := dgPreds[fname]
-						if !ok {
-							edge = dgPred{
-								typ:     typStr,
-								indexes: make(map[string]bool),
-								upsert:  upsertStr,
-							}
-						}
-						for _, index := range indexes {
-							edge.indexes[index] = true
-						}
-						dgPreds[fname] = edge
+						dgPreds[fname] = getUpdatedPred(fname, typStr, upsertStr, indexes)
 					}
 					typ.fields = append(typ.fields, field{fname, parentInt != nil})
 				case ast.Enum:
@@ -310,27 +314,17 @@ func genDgSchema(gqlSch *ast.Schema, definitions []string) string {
 						}
 					}
 					if parentInt == nil {
-						edge, ok := dgPreds[fname]
-						if !ok {
-							edge = dgPred{
-								typ:     typStr,
-								indexes: make(map[string]bool),
-							}
-						}
-						for _, index := range indexes {
-							edge.indexes[index] = true
-						}
-						dgPreds[fname] = edge
+						dgPreds[fname] = getUpdatedPred(fname, typStr, "", indexes)
 					}
 					typ.fields = append(typ.fields, field{fname, parentInt != nil})
 				}
 			}
-			if fd != nil {
-				parentInt := parentInterface(gqlSch, def, fd.Name)
+			if pwdField != nil {
+				parentInt := parentInterface(gqlSch, def, pwdField.Name)
 				if parentInt != nil {
 					typName = typeName(parentInt)
 				}
-				fname := fieldName(fd, typName)
+				fname := fieldName(pwdField, typName)
 
 				if parentInt == nil {
 					dgPreds[fname] = dgPred{typ: "password"}
diff --git a/graphql/schema/schemagen_test.yml b/graphql/schema/schemagen_test.yml
index 5fccd88879a..5f9de3a37a3 100644
--- a/graphql/schema/schemagen_test.yml
+++ b/graphql/schema/schemagen_test.yml
@@ -466,6 +466,9 @@ schemas:
       type B {
         q: String @dgraph(pred: "name") @search(by: [trigram])
       }
+      type C {
+        q: String @dgraph(pred: "name") @search(by: [exact, term])
+      }
     output: |
       type A {
         name
@@ -474,3 +477,6 @@ schemas:
       type B {
         name
       }
+      type C {
+        name
+      }

From dd791f50acbdaf67d57020ddbd07d27eb0e59400 Mon Sep 17 00:00:00 2001
From: Abhimanyu Singh Gaur
 <12651351+abhimanyusinghgaur@users.noreply.github.com>
Date: Wed, 1 Apr 2020 04:44:32 +0530
Subject: [PATCH 6/8] add interface tests

---
 graphql/schema/gqlschema_test.yml | 62 +++++++++++++---------
 graphql/schema/rules.go           | 87 +++++++++++++++++++++++++++++--
 graphql/schema/schemagen.go       |  2 +-
 graphql/schema/schemagen_test.yml | 22 +++++---
 graphql/schema/wrappers.go        | 16 ++++++
 5 files changed, 153 insertions(+), 36 deletions(-)

diff --git a/graphql/schema/gqlschema_test.yml b/graphql/schema/gqlschema_test.yml
index 0738c663177..6ee64341f6e 100644
--- a/graphql/schema/gqlschema_test.yml
+++ b/graphql/schema/gqlschema_test.yml
@@ -710,38 +710,48 @@ invalid_schemas:
     ]
 
   -
-    name: "@dgraph predicate type validation"
+    name: "@dgraph(pred: ...) validation"
     input: |
+      interface V {
+        f1: String @dgraph(pred: "ff1")
+      }
       interface W @secret(field: "f", pred: "pwd") {
-        f1: String! @dgraph(pred: "name")
-        f2: [Float] @dgraph(pred: "val")
+        f2: String! @dgraph(pred: "name")
+        f3: [Float] @dgraph(pred: "val")
+        f4: String @dgraph(pred: "ff4")
+        f5: String @dgraph(pred: "ff1")
       }
-      type X implements W {
-        f3: Y @dgraph(pred: "link")
-        f4: String! @dgraph(pred: "f4") @id
-        f6: String
+      type X implements V & W {
+        f6: Y @dgraph(pred: "link")
+        f7: String! @dgraph(pred: "ff7") @id
+        f8: String
+        f9: String @dgraph(pred: "ff4")
       }
       type Y {
-        f1: Int @dgraph(pred: "name")
-        f2: Float @dgraph(pred: "val")
-        f3: X @dgraph(pred: "link")
-        f4: String @dgraph(pred: "f4")
-        f5: String @dgraph(pred: "pwd")
-        f6: Int @dgraph(pred: "X.f6")
-      }
-    errlist: [
-    {"message": "Type Y; Field f1: has type Int, which is different to type W; field f1, which has the same @dgraph directive but type String. These fields must have either the same GraphQL types, or use different Dgraph predicates.",
-     "locations":[{"line":11, "column":3}]},
-    {"message": "Type Y; Field f2: has type Float, which is different to type W; field f2, which has the same @dgraph directive but type [Float]. These fields must have either the same GraphQL types, or use different Dgraph predicates.",
-     "locations":[{"line":12, "column":3}]},
-    {"message": "Type Y; Field f3: has type X, which is different to type X; field f3, which has the same @dgraph directive but type Y. These fields must have either the same GraphQL types, or use different Dgraph predicates.",
-     "locations":[{"line":13, "column":3}]},
-    {"message": "Type Y; Field f4: doesn't have @id directive, which conflicts with type X; field f4, which has the same @dgraph directive along with @id directive. Both these fields must either use @id directive, or use different Dgraph predicates.",
+        f2: Int @dgraph(pred: "name")
+        f3: Float @dgraph(pred: "val")
+        f6: X @dgraph(pred: "link")
+        f7: String @dgraph(pred: "ff7")
+        f8: Int @dgraph(pred: "X.f8")
+        f10: String @dgraph(pred: "pwd")
+      }
+    errlist: [
+    {"message": "Type X; implements interfaces [V W], all of which have fields with @dgraph predicate: ff1. These fields must use different Dgraph predicates.",
+     "locations":[{"line":10, "column":6}]},
+    {"message": "Type X; Field f9: has the @dgraph directive, which conflicts with interface W; field f4, that this type implements. These fields must use different Dgraph predicates.",
      "locations":[{"line":14, "column":3}]},
-    {"message": "Type Y; Field f5: has the @dgraph predicate, but that conflicts with type W @secret directive on the same predicate. @secret predicates are stored encrypted and so the same predicate can't be used as a String.",
-     "locations":[{"line":15, "column":3}]},
-    {"message": "Type Y; Field f6: has type Int, which is different to type X; field f6, which has the same @dgraph directive but type String. These fields must have either the same GraphQL types, or use different Dgraph predicates.",
-     "locations":[{"line":16, "column":3}]},
+    {"message": "Type Y; Field f2: has type Int, which is different to type W; field f2, which has the same @dgraph directive but type String. These fields must have either the same GraphQL types, or use different Dgraph predicates.",
+     "locations":[{"line":17, "column":3}]},
+    {"message": "Type Y; Field f3: has type Float, which is different to type W; field f3, which has the same @dgraph directive but type [Float]. These fields must have either the same GraphQL types, or use different Dgraph predicates.",
+     "locations":[{"line":18, "column":3}]},
+    {"message": "Type Y; Field f6: has type X, which is different to type X; field f6, which has the same @dgraph directive but type Y. These fields must have either the same GraphQL types, or use different Dgraph predicates.",
+     "locations":[{"line":19, "column":3}]},
+    {"message": "Type Y; Field f7: doesn't have @id directive, which conflicts with type X; field f7, which has the same @dgraph directive along with @id directive. Both these fields must either use @id directive, or use different Dgraph predicates.",
+     "locations":[{"line":20, "column":3}]},
+    {"message": "Type Y; Field f8: has type Int, which is different to type X; field f8, which has the same @dgraph directive but type String. These fields must have either the same GraphQL types, or use different Dgraph predicates.",
+     "locations":[{"line":21, "column":3}]},
+    {"message": "Type Y; Field f10: has the @dgraph predicate, but that conflicts with type W @secret directive on the same predicate. @secret predicates are stored encrypted and so the same predicate can't be used as a String.",
+     "locations":[{"line":22, "column":3}]},
     ]
 
 
diff --git a/graphql/schema/rules.go b/graphql/schema/rules.go
index c45dffb1a4e..67d3baebdb4 100644
--- a/graphql/schema/rules.go
+++ b/graphql/schema/rules.go
@@ -52,6 +52,9 @@ func dgraphDirectivePredicateValidation(gqlSch *ast.Schema, definitions []string
 		isSecret   bool
 	}
 
+	preds := make(map[string]pred)
+	interfacePreds := make(map[string]map[string]bool)
+
 	secretError := func(secretPred, newPred pred) *gqlerror.Error {
 		return gqlerror.ErrorPosf(newPred.position,
 			"Type %s; Field %s: has the @dgraph predicate, but that conflicts with type %s "+
@@ -77,13 +80,87 @@ func dgraphDirectivePredicateValidation(gqlSch *ast.Schema, definitions []string
 			newPred.parentName, newPred.name, idPred.parentName, idPred.name)
 	}
 
-	preds := make(map[string]pred)
+	existingInterfaceFieldError := func(interfacePred, newPred pred) *gqlerror.Error {
+		return gqlerror.ErrorPosf(newPred.position,
+			"Type %s; Field %s: has the @dgraph directive, which conflicts with interface %s; "+
+				"field %s, that this type implements. These fields must use different Dgraph "+
+				"predicates.", newPred.parentName, newPred.name, interfacePred.parentName,
+			interfacePred.name)
+	}
+
+	conflictingFieldsInImplementedInterfacesError := func(def *ast.Definition,
+		interfaces []string, pred string) *gqlerror.Error {
+		return gqlerror.ErrorPosf(def.Position,
+			"Type %s; implements interfaces %v, all of which have fields with @dgraph predicate:"+
+				" %s. These fields must use different Dgraph predicates.", def.Name, interfaces,
+			pred)
+	}
+
+	checkExistingInterfaceFieldError := func(def *ast.Definition, existingPred, newPred pred) {
+		if def.Kind == ast.Interface {
+			for _, defName := range def.Types {
+				if existingPred.parentName == defName {
+					errs = append(errs, existingInterfaceFieldError(newPred, existingPred))
+				}
+			}
+		} else {
+			for _, defName := range def.Interfaces {
+				if existingPred.parentName == defName {
+					errs = append(errs, existingInterfaceFieldError(existingPred, newPred))
+				}
+			}
+		}
+	}
+
+	checkConflictingFieldsInImplementedInterfacesError := func(typ *ast.Definition) {
+		fieldsToReport := make(map[string][]string)
+		interfaces := typ.Interfaces
+
+		for i := 0; i < len(interfaces); i++ {
+			intr1 := interfaces[i]
+			interfacePreds1 := interfacePreds[intr1]
+			for j := i + 1; j < len(interfaces); j++ {
+				intr2 := interfaces[j]
+				for fname, _ := range interfacePreds[intr2] {
+					if interfacePreds1[fname] {
+						if len(fieldsToReport[fname]) == 0 {
+							fieldsToReport[fname] = append(fieldsToReport[fname], intr1)
+						}
+						fieldsToReport[fname] = append(fieldsToReport[fname], intr2)
+					}
+				}
+			}
+		}
+
+		for fname, interfaces := range fieldsToReport {
+			errs = append(errs, conflictingFieldsInImplementedInterfacesError(typ, interfaces,
+				fname))
+		}
+	}
+
+	// make sure all the interfaces are validated before validating any concrete types
+	// this is required when validating that a type if implements two interfaces, then none of the
+	// fields in those interfaces has the same dgraph predicate
+	var interfaces, concreteTypes []string
+	for _, def := range definitions {
+		if gqlSch.Types[def].Kind == ast.Interface {
+			interfaces = append(interfaces, def)
+		} else {
+			concreteTypes = append(concreteTypes, def)
+		}
+	}
+	definitions = append(interfaces, concreteTypes...)
 
 	for _, key := range definitions {
 		def := gqlSch.Types[key]
 		switch def.Kind {
 		case ast.Object, ast.Interface:
 			typName := typeName(def)
+			if def.Kind == ast.Interface {
+				interfacePreds[def.Name] = make(map[string]bool)
+			} else {
+				checkConflictingFieldsInImplementedInterfacesError(def)
+			}
 
 			for _, f := range def.Fields {
 				if f.Type.Name() == "ID" {
@@ -95,6 +172,9 @@ func dgraphDirectivePredicateValidation(gqlSch *ast.Schema, definitions []string
 				// implements. If we get a parent interface, that means this field gets validated
 				// during the validation of that interface. So, no need to validate this field here.
 				if parentInterface(gqlSch, def, f.Name) == nil {
+					if def.Kind == ast.Interface {
+						interfacePreds[def.Name][fname] = true
+					}
 
 					var prefix, suffix string
 					if f.Type.Elem != nil {
@@ -124,6 +204,7 @@ func dgraphDirectivePredicateValidation(gqlSch *ast.Schema, definitions []string
 								errs = append(errs, idError(thisPred, pred))
 							}
 						}
+						checkExistingInterfaceFieldError(def, pred, thisPred)
 					} else {
 						preds[fname] = thisPred
 					}
@@ -133,8 +214,7 @@ func dgraphDirectivePredicateValidation(gqlSch *ast.Schema, definitions []string
 			pwdField := getPasswordField(def)
 			if pwdField != nil {
 				fname := fieldName(pwdField, typName)
-				if parentInterface(gqlSch, def,
-					pwdField.Name) == nil {
+				if parentInterfaceForPwdField(gqlSch, def, pwdField.Name) == nil {
 					thisPred := pred{
 						name:       pwdField.Name,
 						parentName: def.Name,
@@ -148,6 +228,7 @@ func dgraphDirectivePredicateValidation(gqlSch *ast.Schema, definitions []string
 						if thisPred.typ != pred.typ || !pred.isSecret {
 							errs = append(errs, secretError(thisPred, pred))
 						}
+						checkExistingInterfaceFieldError(def, pred, thisPred)
 					} else {
 						preds[fname] = thisPred
 					}
diff --git a/graphql/schema/schemagen.go b/graphql/schema/schemagen.go
index 239c14256f3..9d36da2de2e 100644
--- a/graphql/schema/schemagen.go
+++ b/graphql/schema/schemagen.go
@@ -320,7 +320,7 @@ func genDgSchema(gqlSch *ast.Schema, definitions []string) string {
 				}
 			}
 			if pwdField != nil {
-				parentInt := parentInterface(gqlSch, def, pwdField.Name)
+				parentInt := parentInterfaceForPwdField(gqlSch, def, pwdField.Name)
 				if parentInt != nil {
 					typName = typeName(parentInt)
 				}
diff --git a/graphql/schema/schemagen_test.yml b/graphql/schema/schemagen_test.yml
index 5f9de3a37a3..c026bf728ca 100644
--- a/graphql/schema/schemagen_test.yml
+++ b/graphql/schema/schemagen_test.yml
@@ -438,23 +438,33 @@ schemas:
   -
     name: "fields with same @dgraph(pred: ...) occur only once in schema"
     input: |
-      type A {
-        p: String @dgraph(pred: "name")
+      interface A {
+        p: String @dgraph(pred: "key")
       }
-      type B {
+      type B implements A {
         q: String @dgraph(pred: "name")
+      }
+      type C {
+        r: String @dgraph(pred: "name")
+        s: String @dgraph(pred: "key")
         name: String
       }
     output: |
       type A {
+        key
+      }
+      key: string .
+      type B {
+        key
         name
       }
       name: string .
-      type B {
+      type C {
         name
-        B.name
+        key
+        C.name
       }
-      B.name: string .
+      C.name: string .
 
   -
     name: "fields with same @dgraph(pred: ...) and different @search(by: [...]) have indexes
diff --git a/graphql/schema/wrappers.go b/graphql/schema/wrappers.go
index 4cefc2e4bda..4fa93acbe9b 100644
--- a/graphql/schema/wrappers.go
+++ b/graphql/schema/wrappers.go
@@ -287,6 +287,22 @@ func parentInterface(sch *ast.Schema, typDef *ast.Definition, fieldName string)
 	return nil
 }
 
+func parentInterfaceForPwdField(sch *ast.Schema, typDef *ast.Definition,
+	fieldName string) *ast.Definition {
+	if len(typDef.Interfaces) == 0 {
+		return nil
+	}
+
+	for _, iface := range typDef.Interfaces {
+		interfaceDef := sch.Types[iface]
+		pwdField := getPasswordField(interfaceDef)
+		if pwdField != nil && fieldName == pwdField.Name {
+			return interfaceDef
+		}
+	}
+	return nil
+}
+
 func convertPasswordDirective(dir *ast.Directive) *ast.FieldDefinition {
 	if dir.Name != "secret" {
 		return nil

From 4e9e079113f32f5e848f2b91c9c838b8ec1cd02e Mon Sep 17 00:00:00 2001
From: Abhimanyu Singh Gaur
 <12651351+abhimanyusinghgaur@users.noreply.github.com>
Date: Wed, 1 Apr 2020 04:55:15 +0530
Subject: [PATCH 7/8] fix ci comment

---
 graphql/schema/rules.go | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/graphql/schema/rules.go b/graphql/schema/rules.go
index 67d3baebdb4..012ec763288 100644
--- a/graphql/schema/rules.go
+++ b/graphql/schema/rules.go
@@ -97,17 +97,9 @@ func dgraphDirectivePredicateValidation(gqlSch *ast.Schema, definitions []string
 	}
 
 	checkExistingInterfaceFieldError := func(def *ast.Definition, existingPred, newPred pred) {
-		if def.Kind == ast.Interface {
-			for _, defName := range def.Types {
-				if existingPred.parentName == defName {
-					errs = append(errs, existingInterfaceFieldError(newPred, existingPred))
-				}
-			}
-		} else {
-			for _, defName := range def.Interfaces {
-				if existingPred.parentName == defName {
-					errs = append(errs, existingInterfaceFieldError(existingPred, newPred))
-				}
+		for _, defName := range def.Interfaces {
+			if existingPred.parentName == defName {
+				errs = append(errs, existingInterfaceFieldError(existingPred, newPred))
 			}
 		}
 	}
@@ -121,7 +113,7 @@ func dgraphDirectivePredicateValidation(gqlSch *ast.Schema, definitions []string
 			interfacePreds1 := interfacePreds[intr1]
 			for j := i + 1; j < len(interfaces); j++ {
 				intr2 := interfaces[j]
-				for fname, _ := range interfacePreds[intr2] {
+				for fname := range interfacePreds[intr2] {
 					if interfacePreds1[fname] {
 						if len(fieldsToReport[fname]) == 0 {
 							fieldsToReport[fname] = append(fieldsToReport[fname], intr1)
@@ -204,7 +196,9 @@ func dgraphDirectivePredicateValidation(gqlSch *ast.Schema, definitions []string
 								errs = append(errs, idError(thisPred, pred))
 							}
 						}
-						checkExistingInterfaceFieldError(def, pred, thisPred)
+						if def.Kind == ast.Object {
+							checkExistingInterfaceFieldError(def, pred, thisPred)
+						}
 					} else {
 						preds[fname] = thisPred
 					}
@@ -228,7 +222,9 @@ func dgraphDirectivePredicateValidation(gqlSch *ast.Schema, definitions []string
 						if thisPred.typ != pred.typ || !pred.isSecret {
 							errs = append(errs, secretError(thisPred, pred))
 						}
-						checkExistingInterfaceFieldError(def, pred, thisPred)
+						if def.Kind == ast.Object {
+							checkExistingInterfaceFieldError(def, pred, thisPred)
+						}
 					} else {
 						preds[fname] = thisPred
 					}

From fc2645c8d8cafedb7487066bcfb47facdf12c68a Mon Sep 17 00:00:00 2001
From: Abhimanyu Singh Gaur
 <12651351+abhimanyusinghgaur@users.noreply.github.com>
Date: Wed, 1 Apr 2020 10:29:17 +0530
Subject: [PATCH 8/8] resolve review comments

---
 graphql/schema/gqlschema_test.yml |  4 ++--
 graphql/schema/rules.go           |  3 ++-
 graphql/schema/schemagen.go       | 18 +++++++++++-------
 3 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/graphql/schema/gqlschema_test.yml b/graphql/schema/gqlschema_test.yml
index 6ee64341f6e..31f5c8598fd 100644
--- a/graphql/schema/gqlschema_test.yml
+++ b/graphql/schema/gqlschema_test.yml
@@ -705,8 +705,6 @@ invalid_schemas:
     errlist: [
       {"message": "Type X; has a secret directive and field of the same name f1",
       "locations":[{"line":1, "column":6}]},
-      {"message": "Type X; Field f1: has the @dgraph predicate, but that conflicts with type X @secret directive on the same predicate. @secret predicates are stored encrypted and so the same predicate can't be used as a String.",
-       "locations":[{"line":2, "column":3}]},
     ]
 
   -
@@ -865,10 +863,12 @@ valid_schemas:
         f2: [String] @dgraph(pred: "f2")
         f3: [String!] @dgraph(pred: "f3")
         f4: [String]! @dgraph(pred: "f4")
+        f5: String
       }
       type Y {
         f1: String! @dgraph(pred: "f1")
         f2: [String!] @dgraph(pred: "f2")
         f3: [String]! @dgraph(pred: "f3")
         f4: [String!]! @dgraph(pred: "f4")
+        f5: String @dgraph(pred: "X.f5")
       }
\ No newline at end of file
diff --git a/graphql/schema/rules.go b/graphql/schema/rules.go
index 012ec763288..85d9641d0d6 100644
--- a/graphql/schema/rules.go
+++ b/graphql/schema/rules.go
@@ -208,7 +208,8 @@ func dgraphDirectivePredicateValidation(gqlSch *ast.Schema, definitions []string
 			pwdField := getPasswordField(def)
 			if pwdField != nil {
 				fname := fieldName(pwdField, typName)
-				if parentInterfaceForPwdField(gqlSch, def, pwdField.Name) == nil {
+				if getDgraphDirPredArg(pwdField) != nil && parentInterfaceForPwdField(gqlSch, def,
+					pwdField.Name) == nil {
 					thisPred := pred{
 						name:       pwdField.Name,
 						parentName: def.Name,
diff --git a/graphql/schema/schemagen.go b/graphql/schema/schemagen.go
index 9d36da2de2e..1e35b23dd95 100644
--- a/graphql/schema/schemagen.go
+++ b/graphql/schema/schemagen.go
@@ -169,19 +169,23 @@ func typeName(def *ast.Definition) string {
 }
 
 // fieldName returns the dgraph predicate corresponding to a field.
-// If the field had a dgraph directive, then it returns the value of the name field otherwise
+// If the field had a dgraph directive, then it returns the value of the pred arg otherwise
 // it returns typeName + "." + fieldName.
 func fieldName(def *ast.FieldDefinition, typName string) string {
-	name := typName + "." + def.Name
+	predArg := getDgraphDirPredArg(def)
+	if predArg == nil {
+		return typName + "." + def.Name
+	}
+	return predArg.Value.Raw
+}
+
+func getDgraphDirPredArg(def *ast.FieldDefinition) *ast.Argument {
 	dir := def.Directives.ForName(dgraphDirective)
 	if dir == nil {
-		return name
+		return nil
 	}
 	predArg := dir.Arguments.ForName(dgraphPredArg)
-	if predArg == nil {
-		return name
-	}
-	return predArg.Value.Raw
+	return predArg
 }
 
 // genDgSchema generates Dgraph schema from a valid graphql schema.