Feat(GraphQL): @id field uniqueness at interface level - fixes #8434 #7710

JatinDev543 · 2021-04-09T09:52:33Z

Currently, @id fields in the interface are unique only in one implementing type. But there are several use cases that require @id field to be unique across all the implementation types. Also currently. get a query on the interface can result in unexpected errors as we can have multiple implementing types have the same value for that @id field.

Now we are allowing the @id field in the interface to be unique across all the implementing types. In order to do this, we have added a new argument interface of boolean type in the @id field.

Whenever a @id field in interface type has an interface argument set then its value will be unique across all the implementing types. Users will get errors if they try to add a node with such a field and there is already a node with the same value of that field even in some other implementing types. This is true for other scenarios like adding nested value or while using upserts.

If the interface argument is not present or its value is false then that field will be unique only for one implementing type.
But such fields won't be allowed in argument to get query on interface in the future, see this PR also #7602

Example Schema,

 interface LibraryItem {
    refID: String! @id                   //  This field is unique only for one implementing type
    itemID: String! @id(interface:true)    // This field will be unique over all the implementing types inheriting this interface
}

type Book implements LibraryItem {
    title: String
    author: String
}

Related discuss Post: https://discuss.dgraph.io/t/uniqueness-for-id-fields-on-interface/13294

This change is

…in/GRAPHQL-1127

vmrajas

Good test coverage !!

vmrajas · 2021-04-14T04:41:45Z

graphql/schema/wrappers.go

+// FieldOriginatedFrom returns the definition of the interface from which given field was inherited.
+// If the field wasn't inherited, but belonged to this type, this type's definition is returned.
+// Otherwise, nil is returned.
+func (t *astType) FieldOriginatedFrom(fieldName string) (*ast.Definition, bool) {


Also write a comment on what the bool value contains. I believe it is set to true if the field is inherited from interface.

vmrajas · 2021-04-14T04:46:03Z

graphql/schema/wrappers.go

@@ -2331,6 +2332,30 @@ func hasIDDirective(fd *ast.FieldDefinition) bool {
 	return id != nil


nit: "id" can be replaced by idDirective in above line.

vmrajas · 2021-04-14T04:54:09Z

graphql/schema/gqlschema.go

@@ -276,7 +277,7 @@ input GenerateMutationParams {
 directive @hasInverse(field: String!) on FIELD_DEFINITION
 directive @search(by: [DgraphIndex!]) on FIELD_DEFINITION
 directive @dgraph(type: String, pred: String) on OBJECT | INTERFACE | FIELD_DEFINITION
-directive @id on FIELD_DEFINITION
+directive @id(unique: Boolean) on FIELD_DEFINITION


As discussed yesterday with @abhimanyusinghgaur , we should have a different name instead of "unique". It could be

uniqueAcrossImplementingTypes (too long)

global (may still be confusing)

We can have a discussion in graphql team to discuss an appropriate name

vmrajas · 2021-04-14T05:01:06Z

graphql/schema/gqlschema.go

+							"it will be removed in v21.11.0, please update @id directive to have unique argument if" +
+							" you want to add the @id field in argument to get query for interface",
+							Kind: ast.StringValue}}}})
+		}


In case we have schema like:

interface Root {
name: String! @id (unique:true)
name2: String! @id
}

We won't show the error in this case even though the argument to getRoot API are going to change after 21.11. We will be removing name2 from the argument.

vmrajas · 2021-04-14T05:03:29Z

graphql/schema/gqlschema.go

-							"please update your query to not use that argument",
-							Kind: ast.StringValue}}}})
-		}
+		var idWithoutUniqueArgExist bool


nit: idWithoutUniqueArgExists

vmrajas · 2021-04-14T11:40:48Z

graphql/e2e/auth/schema.graphql

+    query: { rule: "{$ROLE: { eq: \"ADMIN\" } }" },
+){
+    refID: String! @id (unique:true)
+    name:  String! @id


Can you add (unique: false) to this field. It shouldn't have any effect on the behaviour. It will test out if unique arg can take false as well. Can this also be done at 1-2 places in the graphql files in schemagen/input ?

vmrajas · 2021-04-14T11:44:30Z

graphql/resolve/add_mutation_test.yaml

+    }
+  qnametouid: |-
+    {
+      "LibraryMember_3": "0x11"


Shouldn't it also report the same UID for LibraryMember_4

vmrajas · 2021-04-14T11:45:19Z

graphql/resolve/add_mutation_test.yaml

+            ],
+            "uid": "_:SportsMember_6"
+        }
+


Good coverage with yaml test cases.

vmrajas · 2021-04-14T11:46:25Z

graphql/resolve/add_mutation_test.yaml

+        uid
+      }
+    }
+  dgmutations:


No uid is returned in the result of existence query. The upsert is not taking place. Can you add qnametouid map to ensure that the upsert actually takes place.

vmrajas · 2021-04-14T11:47:19Z

graphql/resolve/query_test.yaml

@@ -30,8 +30,7 @@
      }
    }

-


JatinDev543

Reviewable status: 0 of 82 files reviewed, 15 unresolved discussions (waiting on @abhimanyusinghgaur, @id, @jatindevdg, @manishrjain, @pawanrawal, and @vmrajas)

graphql/e2e/auth/schema.graphql, line 855 at r1 (raw file):

Previously, vmrajas wrote…

Can you add (unique: false) to this field. It shouldn't have any effect on the behaviour. It will test out if unique arg can take false as well. Can this also be done at 1-2 places in the graphql files in schemagen/input ?

done.

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

Previously, vmrajas wrote…

nit: directory ? Should it be directive or unique arg

fixed

graphql/e2e/common/mutation.go, line 6263 at r1 (raw file):

Previously, vmrajas wrote…

This error could be confusing. I think in this error case we are sure that this is some other implementing type. Can we make that clear in the error message.

changed the error message.

graphql/resolve/add_mutation_test.yaml, line 1281 at r1 (raw file):

Previously, vmrajas wrote…

No uid is returned in the result of existence query. The upsert is not taking place. Can you add qnametouid map to ensure that the upsert actually takes place.

added.

graphql/resolve/add_mutation_test.yaml, line 1511 at r1 (raw file):

Previously, vmrajas wrote…

Shouldn't it also report the same UID for LibraryMember_4

yeah, added. but tests work fine without that also because in that case we only check existence for the given type.

graphql/resolve/add_mutation_test.yaml, line 1594 at r1 (raw file):

Previously, vmrajas wrote…

Good coverage with yaml test cases.

Thanks!

graphql/resolve/mutation_rewriter.go, line 1228 at r1 (raw file):

Previously, vmrajas wrote…

Add comment as to what is done.
I believe this is adding an interface filter in case of inherited @id field.

yeah,added.

graphql/resolve/mutation_rewriter.go, line 1451 at r1 (raw file):

Previously, vmrajas wrote…

Why was this error message changed ? I think the error message was alright.

This is the kind of the standard error message in our GraphQL implementation." Type %s; field %s: error". As this part of the code is changing so thought of changing the format a bit.

graphql/resolve/mutation_rewriter.go, line 1714 at r1 (raw file):

Previously, vmrajas wrote…

The error message was probably changed to make it consistent with this error message. I feel the old error message can also be used with interface. "id %s already exists for field %s inside one of the implementing types of interface %s "

No, I came to know this format recently as a standard error message in our gql implementation.

graphql/resolve/mutation_rewriter.go, line 1838 at r1 (raw file):

Previously, vmrajas wrote…

As discussed offline in a call, we will have to handle case of duplicate XID for different implementing types using the variableObjMap or add a TODO

added TODO

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

Previously, vmrajas wrote…

As discussed yesterday with @abhimanyusinghgaur , we should have a different name instead of "unique". It could be

uniqueAcrossImplementingTypes (too long)

global (may still be confusing)

We can have a discussion in graphql team to discuss an appropriate name

sure.

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

Previously, vmrajas wrote…

nit: idWithoutUniqueArgExists

done

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

Previously, vmrajas wrote…

In case we have schema like:

interface Root {
name: String! @id (unique:true)
name2: String! @id
}

We won't show the error in this case even though the argument to getRoot API are going to change after 21.11. We will be removing name2 from the argument.

yeah, but one name2 doesn't have a unique arg set, so the message is for it.May be I can change the error message a bit to make it more clear.something like this can be added in message"only those fields with @id directive will be added to getQuery which have unique arg set. ..."

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

Previously, vmrajas wrote…

nit: "id" can be replaced by idDirective in above line.

changed.

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

Previously, vmrajas wrote…

Also write a comment on what the bool value contains. I believe it is set to true if the field is inherited from interface.

done.

vmrajas

LGTM
Lets also create JIRAs to and link it to the TODOs mentioned in the files.
Example: TODO(GRAPHQL-XYZW): "Description of TODO"
That will help in easier tracking of the TODO.

vmrajas · 2021-04-16T01:27:00Z

graphql/resolve/add_mutation_test.yaml

+  qnametouid: |-
+    {
+      "LibraryMember_1": "0x11"
+    }


nit: keep the qnametouid field above dgquerysec as done in other test cases. It will then be easier to follow the test case.

graphql/resolve/mutation_rewriter.go

abhimanyusinghgaur

Overall, looks good. Just got some minor comments, feel free to merge once those are addressed.

Reviewed 6 of 81 files at r1, 75 of 76 files at r2.
Reviewable status: 81 of 82 files reviewed, 28 unresolved discussions (waiting on @id, @jatindevdg, @manishrjain, @pawanrawal, and @vmrajas)

graphql/admin/admin.go, line 192 at r2 (raw file):

interface: Boolean)

this shouldn't be required here.
We don't use interfaces at all in admin server :)

graphql/e2e/auth/debug_off/debugoff_test.go, line 167 at r2 (raw file):

	gqlResponse = addSportsMemberParams.ExecuteAsPost(t, common.GraphqlURL)
	common.RequireNoGQLErrors(t, gqlResponse)

till the point we don't have that error, should we also check that numUIds is 0 for the 2nd mutation, while 1 for the 1st mutation?
That would help really validate this with auth.

graphql/e2e/common/common.go, line 840 at r2 (raw file):

t.Run("query @id field with interface arg on interface", queryWithIDFieldAndInterfaceArg)

is a query test even required? @id(interface: true) is really only about mutation so the query test seems unnecessary.

graphql/e2e/common/mutation.go, line 6097 at r2 (raw file):

expected  string

not used, can be removed

graphql/e2e/common/mutation.go, line 6222 at r2 (raw file):

returns returns

typo

graphql/resolve/mutation_rewriter.go, line 1714 at r1 (raw file):

Previously, JatinDevDG (Jatin Dev) wrote…

No, I came to know this format recently as a standard error message in our gql implementation.

The format: Type %s; Field %s: ... is for errors during schema parsing, as that makes it easier for a user to understand that while updating the schema, where the error was in his schema.

Here, I guess the old format is fine because these errors would be reported during query execution. At that time, they already have enough path and location information present in them to point to the correct place.

graphql/resolve/query_test.yaml, line 3274 at r2 (raw file):

  

Nice indentation fixes 👍

graphql/resolve/query_test.yaml, line 3288 at r2 (raw file):

Quoted 8 lines of code…

    	getMember(refID: "101") {
    		refID
    		name
    		fineAccumulated
    		... on SportsMember {
              plays
            }
    	}

nit: something wrong with indentation here, seems tabs and spaces are mixed up

graphql/schema/gqlschema_test.yml, line 2945 at r2 (raw file):

in interface but it's given in Type

in an interface, not in a type.

graphql/schema/schemagen.go, line 479 at r2 (raw file):

TypeName

so, this won't be exposed outside of this package. Can be renamed back to typename.

graphql/schema/wrappers.go, line 2346 at r2 (raw file):

uniqueArg

interfaceArg

graphql/schema/wrappers.go, line 3049 at r2 (raw file):

*ast.Definition

we shouldn't have a need to expose the AST outside of the schema package.
We expose only a wrapper to the outside world. So, return a schema.Type from here.
That can be constructed, given we have the AST.

JatinDev543

Reviewable status: 81 of 82 files reviewed, 28 unresolved discussions (waiting on @abhimanyusinghgaur, @id, @jatindevdg, @manishrjain, @pawanrawal, and @vmrajas)

graphql/admin/admin.go, line 192 at r2 (raw file):

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

interface: Boolean)
this shouldn't be required here.
We don't use interfaces at all in admin server :)

removed.

graphql/e2e/auth/debug_off/debugoff_test.go, line 167 at r2 (raw file):

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

till the point we don't have that error, should we also check that numUIds is 0 for the 2nd mutation, while 1 for the 1st mutation?
That would help really validate this with auth.

done.

graphql/e2e/common/common.go, line 840 at r2 (raw file):

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

t.Run("query @id field with interface arg on interface", queryWithIDFieldAndInterfaceArg)
is a query test even required? @id(interface: true) is really only about mutation so the query test seems unnecessary.

ok. yeah just wanted to make that get query on interface works correctly with @id field having interface argument. not necessary though.

graphql/e2e/common/mutation.go, line 6097 at r2 (raw file):

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

expected  string
not used, can be removed

removed.

graphql/e2e/common/mutation.go, line 6222 at r2 (raw file):

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

returns returns

typo

fixed.

graphql/resolve/add_mutation_test.yaml, line 1290 at r2 (raw file):

Previously, vmrajas wrote…

nit: keep the qnametouid field above dgquerysec as done in other test cases. It will then be easier to follow the test case.

fixed.

graphql/resolve/mutation_rewriter.go, line 1714 at r1 (raw file):

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

The format: Type %s; Field %s: ... is for errors during schema parsing, as that makes it easier for a user to understand that while updating the schema, where the error was in his schema.

Here, I guess the old format is fine because these errors would be reported during query execution. At that time, they already have enough path and location information present in them to point to the correct place.

changed to old format.

graphql/resolve/mutation_rewriter.go, line 1421 at r2 (raw file):

Previously, vmrajas wrote…

We are making the assumption in the algorithm that if typeUidExist is true then interfaceUidExist is also true. This assumption is valid for newly generated schemas but may not be valid from schemas edited after inserting data.
Can you add the following assert:
if typeUidExists {
// Assert that interfaceUidExists is also true.
}

I think this case won't occur even after schema updates. if a type doesn't implement an interface then there won't be any @id(interface: true) fields in it. And if later user change schema to have an interface with @id(interface: true) and modify the type to implements the interface then we are sure that the new nodes for that type will contain @id(interface: true) and interfacUidExist will be true for them and as old nodes don't have @id with interface arg, interfacUidExist will be false for them which is expected.

graphql/resolve/query_test.yaml, line 3288 at r2 (raw file):

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

    	getMember(refID: "101") {
    		refID
    		name
    		fineAccumulated
    		... on SportsMember {
              plays
            }
    	}
nit: something wrong with indentation here, seems tabs and spaces are mixed up

fixed

graphql/schema/gqlschema_test.yml, line 2945 at r2 (raw file):

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

in interface but it's given in Type
in an interface, not in a type.

changed.

graphql/schema/schemagen.go, line 479 at r2 (raw file):

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

TypeName
so, this won't be exposed outside of this package. Can be renamed back to typename.

changed.

graphql/schema/wrappers.go, line 2346 at r2 (raw file):

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

uniqueArg
interfaceArg

changed

graphql/schema/wrappers.go, line 3049 at r2 (raw file):

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

*ast.Definition
we shouldn't have a need to expose the AST outside of the schema package.
We expose only a wrapper to the outside world. So, return a schema.Type from here.
That can be constructed, given we have the AST.

done.

…QL-1127

@id

…oss all the implementing types. (#7710) Currently, @id fields in the interface are unique only in one implementing type. But there are several use cases that require @id field to be unique across all the implementation types. Also currently. get a query on the interface can result in unexpected errors as we can have multiple implementing types have the same value for that @id field. Now we are allowing the @id field in the interface to be unique across all the implementing types. In order to do this, we have added a new argument interface of boolean type in the @id field. Whenever a @id field in interface type has an interface argument set then its value will be unique across all the implementing types. Users will get errors if they try to add a node with such a field and there is already a node with the same value of that field even in some other implementing types. This is true for other scenarios like adding nested value or while using upserts. If the interface argument is not present or its value is false then that field will be unique only for one implementing type. But such fields won't be allowed in argument to get query on interface in the future, see this PR also #7602 Example Schema, interface LibraryItem { refID: String! @id // This field is unique only for one implementing type itemID: String! @id(interface:true) // This field will be unique over all the implementing types inheriting this interface } type Book implements LibraryItem { title: String author: String } Related discuss Post: https://discuss.dgraph.io/t/uniqueness-for-id-fields-on-interface/13294

@id

…oss all the implementing types. (#7710) Currently, @id fields in the interface are unique only in one implementing type. But there are several use cases that require @id field to be unique across all the implementation types. Also currently. get a query on the interface can result in unexpected errors as we can have multiple implementing types have the same value for that @id field. Now we are allowing the @id field in the interface to be unique across all the implementing types. In order to do this, we have added a new argument interface of boolean type in the @id field. Whenever a @id field in interface type has an interface argument set then its value will be unique across all the implementing types. Users will get errors if they try to add a node with such a field and there is already a node with the same value of that field even in some other implementing types. This is true for other scenarios like adding nested value or while using upserts. If the interface argument is not present or its value is false then that field will be unique only for one implementing type. But such fields won't be allowed in argument to get query on interface in the future, see this PR also #7602 Example Schema, interface LibraryItem { refID: String! @id // This field is unique only for one implementing type itemID: String! @id(interface:true) // This field will be unique over all the implementing types inheriting this interface } type Book implements LibraryItem { title: String author: String } Related discuss Post: https://discuss.dgraph.io/t/uniqueness-for-id-fields-on-interface/13294

@id

…oss all the implementing types. (#7710) Currently, @id fields in the interface are unique only in one implementing type. But there are several use cases that require @id field to be unique across all the implementation types. Also currently. get a query on the interface can result in unexpected errors as we can have multiple implementing types have the same value for that @id field. Now we are allowing the @id field in the interface to be unique across all the implementing types. In order to do this, we have added a new argument interface of boolean type in the @id field. Whenever a @id field in interface type has an interface argument set then its value will be unique across all the implementing types. Users will get errors if they try to add a node with such a field and there is already a node with the same value of that field even in some other implementing types. This is true for other scenarios like adding nested value or while using upserts. If the interface argument is not present or its value is false then that field will be unique only for one implementing type. But such fields won't be allowed in argument to get query on interface in the future, see this PR also #7602 Example Schema, interface LibraryItem { refID: String! @id // This field is unique only for one implementing type itemID: String! @id(interface:true) // This field will be unique over all the implementing types inheriting this interface } type Book implements LibraryItem { title: String author: String } Related discuss Post: https://discuss.dgraph.io/t/uniqueness-for-id-fields-on-interface/13294

@id

…oss all the implementing types. (#7710) Currently, @id fields in the interface are unique only in one implementing type. But there are several use cases that require @id field to be unique across all the implementation types. Also currently. get a query on the interface can result in unexpected errors as we can have multiple implementing types have the same value for that @id field. Now we are allowing the @id field in the interface to be unique across all the implementing types. In order to do this, we have added a new argument interface of boolean type in the @id field. Whenever a @id field in interface type has an interface argument set then its value will be unique across all the implementing types. Users will get errors if they try to add a node with such a field and there is already a node with the same value of that field even in some other implementing types. This is true for other scenarios like adding nested value or while using upserts. If the interface argument is not present or its value is false then that field will be unique only for one implementing type. But such fields won't be allowed in argument to get query on interface in the future, see this PR also #7602 Example Schema, interface LibraryItem { refID: String! @id // This field is unique only for one implementing type itemID: String! @id(interface:true) // This field will be unique over all the implementing types inheriting this interface } type Book implements LibraryItem { title: String author: String } Related discuss Post: https://discuss.dgraph.io/t/uniqueness-for-id-fields-on-interface/13294

@id

…oss all the implementing types. (#7710) Currently, @id fields in the interface are unique only in one implementing type. But there are several use cases that require @id field to be unique across all the implementation types. Also currently. get a query on the interface can result in unexpected errors as we can have multiple implementing types have the same value for that @id field. Now we are allowing the @id field in the interface to be unique across all the implementing types. In order to do this, we have added a new argument interface of boolean type in the @id field. Whenever a @id field in interface type has an interface argument set then its value will be unique across all the implementing types. Users will get errors if they try to add a node with such a field and there is already a node with the same value of that field even in some other implementing types. This is true for other scenarios like adding nested value or while using upserts. If the interface argument is not present or its value is false then that field will be unique only for one implementing type. But such fields won't be allowed in argument to get query on interface in the future, see this PR also #7602 Example Schema, interface LibraryItem { refID: String! @id // This field is unique only for one implementing type itemID: String! @id(interface:true) // This field will be unique over all the implementing types inheriting this interface } type Book implements LibraryItem { title: String author: String } Related discuss Post: https://discuss.dgraph.io/t/uniqueness-for-id-fields-on-interface/13294

@id

…oss all implementing types (#8876) Currently, @id fields in the interface are unique only in one implementing type. But there are several use cases that require @id field to be unique across all the implementation types. Also currently. get a query on the interface can result in unexpected errors as we can have multiple implementing types have the same value for that @id field. Now we are allowing the @id field in the interface to be unique across all the implementing types. In order to do this, we have added a new argument interface of boolean type in the @id field. Whenever a @id field in interface type has an interface argument set then its value will be unique across all the implementing types. Users will get errors if they try to add a node with such a field and there is already a node with the same value of that field even in some other implementing types. This is true for other scenarios like adding nested value or while using upserts. If the interface argument is not present or its value is false then that field will be unique only for one implementing type. But such fields won't be allowed in argument to get query on interface in the future, see this PR also #7602 Example Schema, interface LibraryItem { refID: String! @id // This field is unique only for one implementing type itemID: String! @id(interface:true) // This field will be unique over all the implementing types inheriting this interface } type Book implements LibraryItem { title: String author: String } Related discuss Post: https://discuss.dgraph.io/t/uniqueness-for-id-fields-on-interface/13294 Cherry picked from: #7710

JatinDev543 added 4 commits April 6, 2021 20:00

added unique directory

b2b5025

aadded existence query

e61d295

modify rewrite object to check xid across implementation of interface

bb31c4e

fix a panic,added mutation unit tests

5f468ce

JatinDev543 requested a review from pawanrawal as a code owner April 9, 2021 09:52

github-actions bot added the area/graphql Issues related to GraphQL support on Dgraph. label Apr 9, 2021

fix test

d6e8441

JatinDev543 requested a review from manishrjain as a code owner April 9, 2021 10:06

JatinDev543 added 17 commits April 9, 2021 17:36

fix test and add more test case for add mutation

9dd3c22

add schema

8bc068c

Merge branch 'master' of https://github.com/dgraph-io/dgraph into jat…

1f4cb04

…in/GRAPHQL-1127

added e2e mutation test

d8a8510

clean test

3c6c61a

clean test

b437d2f

clean test

15b4501

fix spaces

0dd1ebb

added interface auth test

e00a0df

added some more tests and schema

1d2eec5

fix schema test

4f0a7a6

clean test

8dd224b

clean code

6f04115

clean code

2af9a35

clean code

c2838e2

fix formatting and test

16ae849

fix formatting

8291064

JatinDev543 changed the title ~~Feat(GraphQL): This PR allows @id field to be unique across all the implementing types of the interface.~~ Feat(GraphQL): This PR allows @id field in interface to be unique across all the implementing types. Apr 11, 2021

fix test

78f2938

vmrajas reviewed Apr 14, 2021

View reviewed changes

addressed rajas comments

4a6a86c

JatinDev543 commented Apr 14, 2021

View reviewed changes

changed arg name from unique to interface

5a94e2a

vmrajas approved these changes Apr 16, 2021

View reviewed changes

abhimanyusinghgaur approved these changes Apr 16, 2021

View reviewed changes

JatinDev543 commented Apr 17, 2021

View reviewed changes

JatinDev543 added 4 commits April 18, 2021 01:35

addressed abhimanyu's comments

fba56a4

change variable names

ca8e34d

Merge branch 'master' of github.com:dgraph-io/dgraph into jatin/GRAPH…

0a147ce

…QL-1127

fixed schema test

9613499

JatinDev543 merged commit 69c8389 into master Apr 17, 2021

JatinDev543 deleted the jatin/GRAPHQL-1127 branch April 17, 2021 21:17

rderbier mentioned this pull request Nov 16, 2022

[FEATURE]: GraphQL @id uniqueness at interface level #8434

Closed

rderbier changed the title ~~Feat(GraphQL): This PR allows @id field in interface to be unique across all the implementing types.~~ Feat(GraphQL): @id field uniqueness at interface level - fixes #8434 Nov 16, 2022

harshil-goel mentioned this pull request Jun 14, 2023

Feat(GraphQL): This PR allows @id field in interface to be unique across all implementing types #8876

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat(GraphQL): @id field uniqueness at interface level - fixes #8434 #7710

Feat(GraphQL): @id field uniqueness at interface level - fixes #8434 #7710

JatinDev543 commented Apr 9, 2021 •

edited

Loading

vmrajas left a comment

vmrajas Apr 14, 2021

vmrajas Apr 14, 2021

vmrajas Apr 14, 2021

vmrajas Apr 14, 2021

vmrajas Apr 14, 2021

vmrajas Apr 14, 2021

vmrajas Apr 14, 2021

vmrajas Apr 14, 2021

vmrajas Apr 14, 2021

vmrajas Apr 14, 2021

JatinDev543 left a comment

vmrajas left a comment

vmrajas Apr 16, 2021

abhimanyusinghgaur left a comment

JatinDev543 left a comment

		@@ -2331,6 +2332,30 @@ func hasIDDirective(fd *ast.FieldDefinition) bool {
		return id != nil

@@ @@ -30,8 +30,7 @@ @@
                     }
                   }
-              -

Feat(GraphQL): @id field uniqueness at interface level - fixes #8434 #7710

Feat(GraphQL): @id field uniqueness at interface level - fixes #8434 #7710

Conversation

JatinDev543 commented Apr 9, 2021 • edited Loading

vmrajas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JatinDev543 left a comment

Choose a reason for hiding this comment

vmrajas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abhimanyusinghgaur left a comment

Choose a reason for hiding this comment

JatinDev543 left a comment

Choose a reason for hiding this comment

JatinDev543 commented Apr 9, 2021 •

edited

Loading