From 5b0c77b019e1a5353ac0263195aa4dcb7d04f5ac Mon Sep 17 00:00:00 2001 From: Martin Martinez Rivera Date: Mon, 14 Sep 2020 12:47:37 -0700 Subject: [PATCH] (release/v20.07) fix(Dgraph): fix bug when deleting and adding to a single UID predicate in the same transaction. 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 (cherry picked from commit 56c8f3c670052ee999d1a77fbc351cc6159ea6ce) --- posting/list.go | 17 +++++++++--- systest/mutations_test.go | 55 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 4 deletions(-) diff --git a/posting/list.go b/posting/list.go index 1fc86cf4754..236b11d94d0 100644 --- a/posting/list.go +++ b/posting/list.go @@ -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 @@ -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 { @@ -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 } diff --git a/systest/mutations_test.go b/systest/mutations_test.go index fd8908fae65..98c429b4d7a 100644 --- a/systest/mutations_test.go +++ b/systest/mutations_test.go @@ -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)) } @@ -2341,6 +2342,60 @@ func OverwriteUidPredicatesReverse(t *testing.T, c *dgo.Dgraph) { } +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 "Alice" . + _:bob "Bob" . + _:alice _: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> <%s> .", alice, bob)), + }) + require.NoError(t, err) + + _, err = txn.Mutate(context.Background(), &api.Mutation{ + SetNquads: []byte(fmt.Sprintf(`<%s> _:carl . + _:carl "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()