Skip to content

Commit

Permalink
fix(Dgraph): fix bug when deleting and adding to a single UID predica…
Browse files Browse the repository at this point in the history
…te in the same transaction. (#6431)

The iterate method does not account for delete postings in the same transaction
so they have to be preserved manually. Otherwise, if a deletion and an add operation
to the same uid happen in the same predicate, the deletion operation is not
applied.

Fixes DGRAPH-1309
  • Loading branch information
martinmr authored Sep 14, 2020
1 parent 4792d8b commit 56c8f3c
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 4 deletions.
17 changes: 13 additions & 4 deletions posting/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -359,8 +359,17 @@ func (l *List) updateMutationLayer(mpost *pb.Posting, singleUidUpdate bool) erro
// The current value should be deleted in favor of this value. This needs to
// be done because the fingerprint for the value is not math.MaxUint64 as is
// the case with the rest of the scalar predicates.
plist := &pb.PostingList{}
plist.Postings = append(plist.Postings, mpost)
newPlist := &pb.PostingList{}
newPlist.Postings = append(newPlist.Postings, mpost)

// Add the deletions in the existing plist because those postings are not picked
// up by iterating. Not doing so would result in delete operations that are not
// applied when the transaction is committed.
for _, post := range plist.Postings {
if post.Op == Del && post.Uid != mpost.Uid {
newPlist.Postings = append(newPlist.Postings, post)
}
}

err := l.iterate(mpost.StartTs, 0, func(obj *pb.Posting) error {
// Ignore values which have the same uid as they will get replaced
Expand All @@ -374,7 +383,7 @@ func (l *List) updateMutationLayer(mpost *pb.Posting, singleUidUpdate bool) erro
// for the mutation stored in mpost.
objCopy := proto.Clone(obj).(*pb.Posting)
objCopy.Op = Del
plist.Postings = append(plist.Postings, objCopy)
newPlist.Postings = append(newPlist.Postings, objCopy)
return nil
})
if err != nil {
Expand All @@ -383,7 +392,7 @@ func (l *List) updateMutationLayer(mpost *pb.Posting, singleUidUpdate bool) erro

// Update the mutation map with the new plist. Return here since the code below
// does not apply for predicates of type uid.
l.mutationMap[mpost.StartTs] = plist
l.mutationMap[mpost.StartTs] = newPlist
return nil
}

Expand Down
55 changes: 55 additions & 0 deletions systest/mutations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ func TestSystem(t *testing.T) {
t.Run("count index delete on non list predicate", wrap(CountIndexNonlistPredicateDelete))
t.Run("Reverse count index delete", wrap(ReverseCountIndexDelete))
t.Run("overwrite uid predicates", wrap(OverwriteUidPredicates))
t.Run("overwrite uid predicates across txns", wrap(OverwriteUidPredicatesMultipleTxn))
t.Run("overwrite uid predicates reverse index", wrap(OverwriteUidPredicatesReverse))
t.Run("delete and query same txn", wrap(DeleteAndQuerySameTxn))
t.Run("add and query zero datetime value", wrap(AddAndQueryZeroTimeValue))
Expand Down Expand Up @@ -2349,6 +2350,60 @@ func OverwriteUidPredicatesReverse(t *testing.T, c *dgo.Dgraph) {
string(resp.GetJson()))
}

func OverwriteUidPredicatesMultipleTxn(t *testing.T, c *dgo.Dgraph) {
ctx := context.Background()
op := &api.Operation{DropAll: true}
require.NoError(t, c.Alter(ctx, op))

op = &api.Operation{
Schema: `
best_friend: uid .
name: string @index(exact) .`,
}
err := c.Alter(ctx, op)
require.NoError(t, err)

resp, err := c.NewTxn().Mutate(context.Background(), &api.Mutation{
CommitNow: true,
SetNquads: []byte(`
_:alice <name> "Alice" .
_:bob <name> "Bob" .
_:alice <best_friend> _:bob .`),
})
require.NoError(t, err)

alice := resp.Uids["alice"]
bob := resp.Uids["bob"]

txn := c.NewTxn()
_, err = txn.Mutate(context.Background(), &api.Mutation{
DelNquads: []byte(fmt.Sprintf("<%s> <best_friend> <%s> .", alice, bob)),
})
require.NoError(t, err)

_, err = txn.Mutate(context.Background(), &api.Mutation{
SetNquads: []byte(fmt.Sprintf(`<%s> <best_friend> _:carl .
_:carl <name> "Carl" .`, alice)),
})
require.NoError(t, err)
err = txn.Commit(context.Background())
require.NoError(t, err)

query := fmt.Sprintf(`{
me(func:uid(%s)) {
name
best_friend {
name
}
}
}`, alice)

resp, err = c.NewReadOnlyTxn().Query(ctx, query)
require.NoError(t, err)
testutil.CompareJSON(t, `{"me":[{"name":"Alice","best_friend": {"name": "Carl"}}]}`,
string(resp.GetJson()))
}

func DeleteAndQuerySameTxn(t *testing.T, c *dgo.Dgraph) {
// Set the schema.
ctx := context.Background()
Expand Down

0 comments on commit 56c8f3c

Please sign in to comment.