Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to set schema to a single UID schema. #2895

Merged
merged 8 commits into from
Jan 15, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 89 additions & 19 deletions dgraph/cmd/alpha/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ func TestDeletePredicate(t *testing.T) {
`

var s1 = `
friend: uid @reverse .
friend: [uid] @reverse .
name: string @index(term) .
`

Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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)
Expand All @@ -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> <friend> <0x2> .
<0x1> <friend> <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> <friend> <0x2> .
<0x1> <friend> <0x3> .
}
}
`
require.Error(t, runMutation(m1))

var s2 = `
friend: [uid] .
`
require.NoError(t, alterSchema(s2))
var m2 = `
{
set {
<0x1> <friend> <0x2> .
<0x1> <friend> <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 = `
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -623,7 +693,7 @@ func TestSchemaMutationCountAdd(t *testing.T) {
`

var s = `
friend:uid @count .
friend: [uid] @count .
`

// reset Schema
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -1155,7 +1225,7 @@ func TestSchemaMutation4Error(t *testing.T) {
m = `
mutation {
schema {
age:uid .
age: uid .
}
}
`
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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 := `
Expand Down
1 change: 1 addition & 0 deletions gql/mutation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
4 changes: 4 additions & 0 deletions posting/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
8 changes: 4 additions & 4 deletions posting/index_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
33 changes: 33 additions & 0 deletions posting/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 11 additions & 6 deletions query/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -305,27 +310,27 @@ 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 .
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)
Expand Down
4 changes: 0 additions & 4 deletions schema/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Expand Down
Loading