[BREAKING] Namespace dgraph internal types/predicates with dgraph.

[animesh2049]

Dgraph's internal types User, Group and Rule were too generic. Prefixed them with dgraph.type so that there is no chance of collision with user-defined types.

Also, reserved dgraph. as the namespace for dgraph's internal types/predicates so that no user could create any such type/predicate.

Fixes #4878.
Fixes #DGRAPH-1090.

---

This change is 

Docs Preview:

[pawanrawal]

We need a test case showing that the user can't create or update types with the prefix dgraph.type. as those are reserved types. Do we already have one?
Would this be a breaking change and would require migrating data? How would users migrate from older versions?

Reviewed 1 of 9 files at r1, 4 of 9 files at r2, 4 of 4 files at r3.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @animesh2049 and @manishrjain)

[animesh2049]

Now we do restrict Alter of internal types. I have also added tests for the same. Now we need to figure out a way to migrate users from older version.

Reviewable status: 5 of 12 files reviewed, 6 unresolved discussions (waiting on @animesh2049, @manishrjain, and @pawanrawal)

[abhimanyusinghgaur]

Reviewable status: 5 of 12 files reviewed, 7 unresolved discussions (waiting on @animesh2049, @manishrjain, and @pawanrawal)

a discussion (no related file):
I was thinking should we reserve dgraph. as a namespace for the internal types and predicates that we may create in future? So that people don't create types/predicates starting with dgraph.. Otherwise, they may create such a type/predicates now, and later if we are introducing that type/predicates internally, then those people will have to change their schema, which won't be a good experience for them.

---

[pawanrawal]

Reviewed 2 of 2 files at r4, 5 of 5 files at r5.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @abhimanyusinghgaur, @animesh2049, and @manishrjain)

a discussion (no related file):

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

I was thinking should we reserve dgraph. as a namespace for the internal types that we may create in future? So that people don't create types starting with dgraph.. Otherwise, they may create such a type now, and later if we are introducing that type internally, then those people will have to change their schema, which won't be a good experience for them.

Yeah, agree. We should ideally not allow the user to create/alter predicates/types prefixed with dgraph.

---

[abhimanyusinghgaur]

Reviewable status: 6 of 15 files reviewed, 5 unresolved discussions (waiting on @manishrjain, @MichaelJCompton, @pawanrawal, and @vvbalaji-dgraph)

a discussion (no related file):

Previously, pawanrawal (Pawan Rawal) wrote…

We need to figure out a migration story for this as this would require migration of ACL data to work for old users.

Created a story in JIRA for this.

---

---

edgraph/server.go, line 266 at r6 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Add more comments about how pre-defined are just a subset of reserved namespace so that this is easier to understand and follow. Also, say that we allow schema updates for predefined predicate if they are the same as before but disallow for reserved predicates that are not pre-defined.

Done

---

query/mutation_test.go, line 34 at r6 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Should this be TestAlteringReservedTypesAndPredicatesShouldFail?

Done

---

worker/proposal.go, line 169 at r6 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Please add a comment here saying that we don't allow mutations for reserved predicates if the schema for them doesn't already exist.

Done

---

x/keys.go, line 573 at r6 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Add a comment that this would be a subset of IsReservedPredicate

Done

---

x/keys.go, line 610 at r6 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Comment is old

Done

[pawanrawal]

Reviewed 1 of 11 files at r6, 8 of 9 files at r7.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @manishrjain, @MichaelJCompton, and @vvbalaji-dgraph)

a discussion (no related file):

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

Created a story in JIRA for this.

---

[martinmr]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @animesh2049, @manishrjain, @MichaelJCompton, and @vvbalaji-dgraph)

---

x/keys.go, line 591 at r7 (raw file):

reservedPredicateMap

maybe this map should be renamed to predefinedPredicateMap?

[abhimanyusinghgaur]

Reviewable status: 14 of 16 files reviewed, 1 unresolved discussion (waiting on @manishrjain, @martinmr, @MichaelJCompton, @pawanrawal, and @vvbalaji-dgraph)

---

x/keys.go, line 591 at r7 (raw file):

Previously, martinmr (Martin Martinez Rivera) wrote…

reservedPredicateMap

maybe this map should be renamed to predefinedPredicateMap?

Renamed it to starAllPredicateMap as it was being used only for finding predicates which were needed when doing edge expansion for queries containing * in nquads.

[manishrjain]

Not sure if the dgraph. prefix is the right one. Perhaps use _dgraph. or something.

Reviewable status: 12 of 16 files reviewed, 3 unresolved discussions (waiting on @animesh2049, @manishrjain, @martinmr, @MichaelJCompton, @pawanrawal, and @vvbalaji-dgraph)

---

edgraph/access_ee.go, line 828 at r8 (raw file):

	acl info by applying filter of userid and groupid to acl predicates. A query like
	Conversion pattern:
		* me(func: type(dgraph.type.Group)) ->

Perhaps use _dgraph. as the name.

---

worker/proposal.go, line 172 at r8 (raw file):

				// already exist.
				if x.IsReservedPredicate(edge.Attr) {
					return errors.Errorf("Can't store predicate `%s` as it is prefixed with `dgraph.`"+

100 chars.

[abhimanyusinghgaur]

Reviewable status: 11 of 16 files reviewed, 3 unresolved discussions (waiting on @manishrjain, @martinmr, @MichaelJCompton, @pawanrawal, and @vvbalaji-dgraph)

---

edgraph/access_ee.go, line 828 at r8 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Perhaps use _dgraph. as the name.

As discussed, keeping it to dgraph. because changing it to _dgraph. will force all users to migrate data.

---

worker/proposal.go, line 172 at r8 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

100 chars.

Done