diff --git a/dgraph/cmd/alpha/run_test.go b/dgraph/cmd/alpha/run_test.go index bd85ae1e921..88aaffb1c90 100644 --- a/dgraph/cmd/alpha/run_test.go +++ b/dgraph/cmd/alpha/run_test.go @@ -237,7 +237,7 @@ func TestDeletePredicate(t *testing.T) { ` var s1 = ` - friend: uid @reverse . + friend: [uid] @reverse . name: string @index(term) . ` @@ -323,18 +323,18 @@ type Received struct { func TestSchemaMutation(t *testing.T) { require.NoError(t, dropAll()) var m = ` - name:string @index(term, exact) . - alias:string @index(exact, term) . - dob:dateTime @index(year) . - film.film.initial_release_date:dateTime @index(year) . - loc:geo @index(geo) . - genre:uid @reverse . + name: string @index(term, exact) . + alias: string @index(exact, term) . + dob: dateTime @index(year) . + film.film.initial_release_date: dateTime @index(year) . + loc: geo @index(geo) . + genre: [uid] @reverse . survival_rate : float . alive : bool . age : int . shadow_deep : int . - friend:uid @reverse . - geometry:geo @index(geo) . ` + friend: [uid] @reverse . + geometry: geo @index(geo) . ` expected := S{ Predicate: "name", @@ -410,7 +410,7 @@ func TestSchemaMutation2Error(t *testing.T) { // index on uid type func TestSchemaMutation3Error(t *testing.T) { var m = ` - age:uid @index . + age: uid @index . ` err := alterSchema(m) require.Error(t, err) @@ -429,6 +429,76 @@ func TestMutation4Error(t *testing.T) { require.Error(t, err) } +func TestMutationSingleUid(t *testing.T) { + // reset Schema + require.NoError(t, schema.ParseBytes([]byte(""), 1)) + + var s = ` + friend: uid . + ` + require.NoError(t, alterSchema(s)) + + var m = ` + { + set { + <0x1> <0x2> . + <0x1> <0x3> . + } + } + ` + require.Error(t, runMutation(m)) +} + +// Verify multiple uids are allowed after mutation. +func TestSchemaMutationUid(t *testing.T) { + // reset Schema + require.NoError(t, schema.ParseBytes([]byte(""), 1)) + + var s1 = ` + friend: uid . + ` + require.NoError(t, alterSchema(s1)) + var m1 = ` + { + set { + <0x1> <0x2> . + <0x1> <0x3> . + } + } + ` + require.Error(t, runMutation(m1)) + + var s2 = ` + friend: [uid] . + ` + require.NoError(t, alterSchema(s2)) + var m2 = ` + { + set { + <0x1> <0x2> . + <0x1> <0x3> . + } + } + ` + require.NoError(t, runMutation(m2)) +} + +// Verify a list uid predicate cannot be converted to a single-element predicate. +func TestSchemaMutationUidError1(t *testing.T) { + // reset Schema + require.NoError(t, schema.ParseBytes([]byte(""), 1)) + + var s1 = ` + friend: [uid] . + ` + require.NoError(t, alterSchema(s1)) + + var s2 = ` + friend: uid . + ` + require.Error(t, alterSchema(s2)) +} + // add index func TestSchemaMutationIndexAdd(t *testing.T) { var q1 = ` @@ -533,7 +603,7 @@ func TestSchemaMutationReverseAdd(t *testing.T) { } ` - var s = `friend:uid @reverse .` + var s = `friend: [uid] @reverse .` // reset Schema schema.ParseBytes([]byte(""), 1) @@ -572,11 +642,11 @@ func TestSchemaMutationReverseRemove(t *testing.T) { ` var s1 = ` - friend:uid @reverse . + friend: [uid] @reverse . ` var s2 = ` - friend:uid . + friend: [uid] . ` // reset Schema @@ -623,7 +693,7 @@ func TestSchemaMutationCountAdd(t *testing.T) { ` var s = ` - friend:uid @count . + friend: [uid] @count . ` // reset Schema @@ -825,7 +895,7 @@ func TestDeleteAll(t *testing.T) { ` var s1 = ` - friend:uid @reverse . + friend: [uid] @reverse . name: string @index(term) . ` schema.ParseBytes([]byte(""), 1) @@ -1155,7 +1225,7 @@ func TestSchemaMutation4Error(t *testing.T) { m = ` mutation { schema { - age:uid . + age: uid . } } ` @@ -1166,7 +1236,7 @@ func TestSchemaMutation4Error(t *testing.T) { // change from uid to scalar or vice versa func TestSchemaMutation5Error(t *testing.T) { var m = ` - friends:uid . + friends: [uid] . ` // reset Schema schema.ParseBytes([]byte(""), 1) @@ -1184,7 +1254,7 @@ func TestSchemaMutation5Error(t *testing.T) { require.NoError(t, err) m = ` - friends:string . + friends: string . ` err = alterSchema(m) require.Error(t, err) @@ -1439,7 +1509,7 @@ func TestRecurseExpandAll(t *testing.T) { } func TestIllegalCountInQueryFn(t *testing.T) { - s := `friend: uid @count .` + s := `friend: [uid] @count .` require.NoError(t, alterSchemaWithRetry(s)) q := ` diff --git a/gql/mutation.go b/gql/mutation.go index 1569501d36a..e5436245090 100644 --- a/gql/mutation.go +++ b/gql/mutation.go @@ -160,6 +160,7 @@ func (nq NQuad) createEdgePrototype(subjectUid uint64) *pb.DirectedEdge { func (nq NQuad) CreateUidEdge(subjectUid uint64, objectUid uint64) *pb.DirectedEdge { out := nq.createEdgePrototype(subjectUid) out.ValueId = objectUid + out.ValueType = pb.Posting_UID return out } diff --git a/posting/index.go b/posting/index.go index 40f5712579c..42c2a9842f8 100644 --- a/posting/index.go +++ b/posting/index.go @@ -295,6 +295,10 @@ func (txn *Txn) addMutationHelper(ctx context.Context, l *List, doUpdateIndex bo "Acquired lock %v %v %v", dur, t.Attr, t.Entity) } + if err := l.canMutateUid(txn, t); err != nil { + return val, found, emptyCountParams, err + } + if doUpdateIndex { // Check original value BEFORE any mutation actually happens. val, found, err = l.findValue(txn.StartTs, fingerprintEdge(t)) diff --git a/posting/index_test.go b/posting/index_test.go index ea226208c4c..c276b79816a 100644 --- a/posting/index_test.go +++ b/posting/index_test.go @@ -146,10 +146,10 @@ func addMutation(t *testing.T, l *List, edge *pb.DirectedEdge, op uint32, } const schemaVal = ` -name:string @index(term) . -name2:string @index(term) . -dob:dateTime @index(year) . -friend:uid @reverse . +name: string @index(term) . +name2: string @index(term) . +dob: dateTime @index(year) . +friend: [uid] @reverse . ` // TODO(Txn): We can't read index key on disk if it was written in same txn. diff --git a/posting/list.go b/posting/list.go index 6af1acd96f8..a641b29759a 100644 --- a/posting/list.go +++ b/posting/list.go @@ -275,6 +275,39 @@ func fingerprintEdge(t *pb.DirectedEdge) uint64 { return id } +// canMutateUid returns an error if all the following conditions are met. +// * Predicate is of type UidID. +// * Predicate is not set to a list of uids in the schema. +// * The existing posting list has an entry that does not match the proposed +// mutation's uid. +// In this case, the user should delete the existing predicate and retry, or mutate +// the schema to allow for multiple uids. This method is necessary to support uid +// predicates with single values because previously all uid predicates were +// considered lists. +// This functions returns a nil error in all other cases. +func (l *List) canMutateUid(txn *Txn, edge *pb.DirectedEdge) error { + l.AssertRLock() + + if types.TypeID(edge.ValueType) != types.UidID { + return nil + } + + if schema.State().IsList(edge.Attr) { + return nil + } + + return l.iterate(txn.StartTs, 0, func(obj *pb.Posting) error { + if obj.Uid != edge.GetValueId() { + return fmt.Errorf( + "cannot add value with uid %x to predicate %s because one of the existing "+ + "values does not match this uid, either delete the existing values first or "+ + "modify the schema to '%s: [uid]'", + edge.GetValueId(), edge.Attr, edge.Attr) + } + return nil + }) +} + func (l *List) addMutation(ctx context.Context, txn *Txn, t *pb.DirectedEdge) error { if atomic.LoadInt32(&l.deleteMe) == 1 { return ErrRetry diff --git a/query/common_test.go b/query/common_test.go index f9924a3ccca..eea4eab7495 100644 --- a/query/common_test.go +++ b/query/common_test.go @@ -62,10 +62,15 @@ func addEdge(t *testing.T, attr string, src uint64, edge *pb.DirectedEdge) { // Mutations don't go through normal flow, so default schema for predicate won't be present. // Lets add it. if _, ok := schema.State().Get(attr); !ok { - schema.State().Set(attr, pb.SchemaUpdate{ + update := pb.SchemaUpdate{ Predicate: attr, ValueType: edge.ValueType, - }) + } + // Edges of type pb.Posting_UID should default to a list. + if edge.ValueType == pb.Posting_UID { + update.List = true + } + schema.State().Set(attr, update) } startTs := timestamp() txn := posting.Oracle().RegisterStartTs(startTs) @@ -305,19 +310,19 @@ dob : dateTime @index(year) . dob_day : dateTime @index(day) . film.film.initial_release_date : dateTime @index(year) . loc : geo @index(geo) . -genre : uid @reverse . +genre : [uid] @reverse . survival_rate : float . alive : bool @index(bool) . age : int @index(int) . shadow_deep : int . -friend : uid @reverse @count . +friend : [uid] @reverse @count . geometry : geo @index(geo) . value : string @index(trigram) . full_name : string @index(hash) . nick_name : string @index(term) . royal_title : string @index(hash, term, fulltext) @lang . noindex_name : string . -school : uid @count . +school : [uid] @count . lossy : string @index(term) @lang . occupations : [string] @index(term) . graduation : [dateTime] @index(year) @count . @@ -325,7 +330,7 @@ salary : float @index(float) . password : password . symbol : string @index(exact) . room : string @index(term) . -office.room : uid . +office.room : [uid] . ` err := schema.ParseBytes([]byte(schemaStr), 1) diff --git a/schema/parse.go b/schema/parse.go index 194bf8872ec..d9b4efa2486 100644 --- a/schema/parse.go +++ b/schema/parse.go @@ -126,10 +126,6 @@ func parseScalarPair(it *lex.ItemIterator, predicate string) (*pb.SchemaUpdate, return nil, x.Errorf("Undefined Type") } if schema.List { - if !t.IsScalar() { - return nil, x.Errorf("Expected scalar type inside []. Got: [%s] for attr: [%s].", - t.Name(), predicate) - } if uint32(t) == uint32(types.PasswordID) || uint32(t) == uint32(types.BoolID) { return nil, x.Errorf("Unsupported type for list: [%s].", types.TypeID(t).Name()) } diff --git a/schema/parse_test.go b/schema/parse_test.go index 07a1a759282..829922198e2 100644 --- a/schema/parse_test.go +++ b/schema/parse_test.go @@ -136,15 +136,15 @@ func TestSchemaIndex_Error1(t *testing.T) { } var schemaIndexVal3Uid = ` -person:uid @index . +person: uid @index . ` var schemaIndexVal3Default = ` -value:default @index . +value: default @index . ` var schemaIndexVal3Password = ` -pass:password @index . +pass: password @index . ` // Object types cant be indexed. @@ -303,16 +303,6 @@ func TestParseScalarList(t *testing.T) { } func TestParseScalarListError1(t *testing.T) { - reset() - schemas, err := Parse(` - friend: [uid] . - `) - require.Error(t, err) - require.Contains(t, err.Error(), "Expected scalar type inside []. Got: [uid] for attr: [friend].") - require.Nil(t, schemas) -} - -func TestParseScalarListError2(t *testing.T) { reset() schemas, err := Parse(` friend: [string . @@ -322,7 +312,7 @@ func TestParseScalarListError2(t *testing.T) { require.Nil(t, schemas) } -func TestParseScalarListError3(t *testing.T) { +func TestParseScalarListError2(t *testing.T) { reset() schemas, err := Parse(` friend: string] . @@ -332,7 +322,7 @@ func TestParseScalarListError3(t *testing.T) { require.Nil(t, schemas) } -func TestParseScalarListError4(t *testing.T) { +func TestParseScalarListError3(t *testing.T) { reset() _, err := Parse(` friend: [bool] . @@ -341,6 +331,34 @@ func TestParseScalarListError4(t *testing.T) { require.Contains(t, err.Error(), "Unsupported type for list: [bool]") } +func TestParseUidList(t *testing.T) { + reset() + schemas, err := Parse(` + friend: [uid] . + `) + require.NoError(t, err) + require.Equal(t, 1, len(schemas)) + require.EqualValues(t, &pb.SchemaUpdate{ + Predicate: "friend", + ValueType: 7, + List: true, + }, schemas[0]) +} + +func TestParseUidSingleValue(t *testing.T) { + reset() + schemas, err := Parse(` + friend: uid . + `) + require.NoError(t, err) + require.Equal(t, 1, len(schemas)) + require.EqualValues(t, &pb.SchemaUpdate{ + Predicate: "friend", + ValueType: 7, + List: false, + }, schemas[0]) +} + var ps *badger.DB func TestMain(m *testing.M) { diff --git a/systest/mutations_test.go b/systest/mutations_test.go index f4b3b2dfcbb..d5ac4a738ba 100644 --- a/systest/mutations_test.go +++ b/systest/mutations_test.go @@ -282,7 +282,7 @@ func ExpandAllReversePredicatesTest(t *testing.T, c *dgo.Dgraph) { ctx := context.Background() require.NoError(t, c.Alter(ctx, &api.Operation{ - Schema: `link: uid @reverse .`, + Schema: `link: [uid] @reverse .`, })) _, err := c.NewTxn().Mutate(ctx, &api.Mutation{ @@ -629,7 +629,7 @@ func SchemaAfterDeleteNode(t *testing.T, c *dgo.Dgraph) { require.JSONEq(t, `[`+ `{"predicate":"_predicate_","type":"string","list":true},`+ x.AclPredsJson+","+ - `{"predicate":"friend","type":"uid"},`+ + `{"predicate":"friend","type":"uid","list":true},`+ `{"predicate":"married","type":"bool"},`+ `{"predicate":"name","type":"default"}]`, string(b)) @@ -652,7 +652,7 @@ func SchemaAfterDeleteNode(t *testing.T, c *dgo.Dgraph) { require.JSONEq(t, `[`+ `{"predicate":"_predicate_","type":"string","list":true},`+ x.AclPredsJson+","+ - `{"predicate":"friend","type":"uid"},`+ + `{"predicate":"friend","type":"uid","list":true},`+ `{"predicate":"name","type":"default"}]`, string(b)) } @@ -906,7 +906,7 @@ func EmptyRoomsWithTermIndex(t *testing.T, c *dgo.Dgraph) { op := &api.Operation{} op.Schema = ` room: string @index(term) . - office.room: uid . + office.room: [uid] . ` ctx := context.Background() err := c.Alter(ctx, op) @@ -1076,7 +1076,7 @@ func FacetExpandAll(t *testing.T, c *dgo.Dgraph) { check(t, (c.Alter(ctx, &api.Operation{ Schema: `name: string @index(hash) . - friend: uid @reverse .`, + friend: [uid] @reverse .`, }))) txn := c.NewTxn() diff --git a/systest/queries_test.go b/systest/queries_test.go index e3d647b1467..05475749a9e 100644 --- a/systest/queries_test.go +++ b/systest/queries_test.go @@ -89,7 +89,8 @@ func SchemaQueryTest(t *testing.T, c *dgo.Dgraph) { { "predicate": "dgraph.user.group", "type": "uid", - "reverse": true + "reverse": true, + "list": true }, { "predicate": "dgraph.xid", @@ -294,7 +295,8 @@ func SchemaQueryTestHTTP(t *testing.T, c *dgo.Dgraph) { { "predicate": "dgraph.user.group", "type": "uid", - "reverse": true + "reverse": true, + "list": true }, { "predicate": "dgraph.xid", diff --git a/worker/groups.go b/worker/groups.go index 928900578a9..b6f66b6fb89 100644 --- a/worker/groups.go +++ b/worker/groups.go @@ -168,6 +168,7 @@ func (g *groupi) proposeInitialSchema() { Predicate: "dgraph.user.group", Directive: pb.SchemaUpdate_REVERSE, ValueType: pb.Posting_UID, + List: true, }) g.upsertSchema(&pb.SchemaUpdate{ Predicate: "dgraph.group.acl", diff --git a/worker/mutation.go b/worker/mutation.go index 5b1a10b328e..f943911f08a 100644 --- a/worker/mutation.go +++ b/worker/mutation.go @@ -168,6 +168,11 @@ func updateSchemaType(attr string, typ types.TypeID, index uint64) { s.ValueType = typ.Enum() } else { s = pb.SchemaUpdate{ValueType: typ.Enum(), Predicate: attr} + // For type UidID, set List to true. This is done because previously + // all predicates of type UidID were implicitly considered lists. + if typ == types.UidID { + s.List = true + } } updateSchema(attr, s) } diff --git a/worker/worker_test.go b/worker/worker_test.go index 6d9900aed59..776b536f839 100644 --- a/worker/worker_test.go +++ b/worker/worker_test.go @@ -116,7 +116,7 @@ func helpProcessTask(query *pb.Query, gid uint32) (*pb.Result, error) { } func TestProcessTask(t *testing.T) { - initTest(t, `neighbour: uid .`) + initTest(t, `neighbour: [uid] .`) query := newQuery("neighbour", []uint64{10, 11, 12}, nil) r, err := helpProcessTask(query, 1) diff --git a/x/x.go b/x/x.go index 3b5d25220ed..90276a0dc2a 100644 --- a/x/x.go +++ b/x/x.go @@ -88,7 +88,7 @@ var ( AclPredsJson = ` {"predicate":"dgraph.group.acl", "type":"string"}, {"predicate":"dgraph.password", "type":"password"}, -{"reverse":true, "predicate":"dgraph.user.group", "type":"uid"}, +{"reverse":true, "predicate":"dgraph.user.group", "type":"uid", "list":true}, {"index":true, "tokenizer":["exact"], "predicate":"dgraph.xid", "type":"string"} ` Nilbyte []byte