From b6069b66cf81ad1ddec025bebd83a5075d620ed2 Mon Sep 17 00:00:00 2001 From: Pawan Rawal Date: Wed, 21 Aug 2019 16:10:20 +1000 Subject: [PATCH 1/3] Return found as false if value doesn't match while doing delete mutation. --- posting/index.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/posting/index.go b/posting/index.go index 6a2223de447..6859f1a4f24 100644 --- a/posting/index.go +++ b/posting/index.go @@ -331,7 +331,7 @@ func (txn *Txn) addMutationHelper(ctx context.Context, l *List, doUpdateIndex bo if pFound && !(bytes.Equal(currPost.Value, newPost.Value) && types.TypeID(currPost.ValType) == types.TypeID(newPost.ValType)) { - return val, found, emptyCountParams, err + return val, false, emptyCountParams, err } } From 79d3832debe905cbdfe39223d13a1fd4e5caee13 Mon Sep 17 00:00:00 2001 From: Pawan Rawal Date: Wed, 21 Aug 2019 16:35:55 +1000 Subject: [PATCH 2/3] Update DeleteScalarValue to test this scenario. --- dgraph/cmd/alpha/run_test.go | 21 +++++++++++++++++++-- posting/index.go | 2 +- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/dgraph/cmd/alpha/run_test.go b/dgraph/cmd/alpha/run_test.go index a3b486a9bba..2f8950b3fd5 100644 --- a/dgraph/cmd/alpha/run_test.go +++ b/dgraph/cmd/alpha/run_test.go @@ -1294,14 +1294,15 @@ func TestDeleteAllSP2(t *testing.T) { } func TestDeleteScalarValue(t *testing.T) { - var s = `name: string .` + var s = `name: string @index(exact) .` require.NoError(t, schema.ParseBytes([]byte(""), 1)) require.NoError(t, alterSchemaWithRetry(s)) var m = ` { set { - <0x12345> "xxx" . + <0x12345> "xxx" . + <0x12346> "xxx" . } } ` @@ -1342,6 +1343,17 @@ func TestDeleteScalarValue(t *testing.T) { require.NoError(t, err) require.JSONEq(t, `{"data": {"me":[{"name":"xxx"}]}}`, output) + indexQuery := ` + { + me(func: eq(name, "xxx")) { + name + } + } + ` + output, err = runGraphqlQuery(indexQuery) + require.NoError(t, err) + require.JSONEq(t, `{"data": {"me":[{"name":"xxx"}, {"name":"xxx"}]}}`, output) + var d2 = ` { delete { @@ -1356,6 +1368,11 @@ func TestDeleteScalarValue(t *testing.T) { output, err = runGraphqlQuery(q) require.NoError(t, err) require.JSONEq(t, `{"data": {"me":[]}}`, output) + + // Verify index was also updated this time and one of the triples got deleted. + output, err = runGraphqlQuery(indexQuery) + require.NoError(t, err) + require.JSONEq(t, `{"data": {"me":[{"name": "xxx"}]}}`, output) } func TestDeleteValueLang(t *testing.T) { diff --git a/posting/index.go b/posting/index.go index 6859f1a4f24..db92cc26ad5 100644 --- a/posting/index.go +++ b/posting/index.go @@ -331,7 +331,7 @@ func (txn *Txn) addMutationHelper(ctx context.Context, l *List, doUpdateIndex bo if pFound && !(bytes.Equal(currPost.Value, newPost.Value) && types.TypeID(currPost.ValType) == types.TypeID(newPost.ValType)) { - return val, false, emptyCountParams, err + return val, false, emptyCountParams, nil } } From e3257e9ff04c9eee2aa219a25115af1d1b748dd3 Mon Sep 17 00:00:00 2001 From: Pawan Rawal Date: Thu, 22 Aug 2019 08:50:43 +1000 Subject: [PATCH 3/3] Add an elaborate comment to explain the reason behind returning found as false. --- posting/index.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/posting/index.go b/posting/index.go index db92cc26ad5..b785b446cc0 100644 --- a/posting/index.go +++ b/posting/index.go @@ -329,6 +329,12 @@ func (txn *Txn) addMutationHelper(ctx context.Context, l *List, doUpdateIndex bo return val, found, emptyCountParams, err } + // This is a scalar value of non-list type and a delete edge mutation, so if the value + // given by the user doesn't match the value we have, we return found to be false, to avoid + // deleting the uid from index posting list. + // This second check is required because we fingerprint the scalar values as math.MaxUint64, + // so even though they might be different the check in the doUpdateIndex block above would + // return found to be true. if pFound && !(bytes.Equal(currPost.Value, newPost.Value) && types.TypeID(currPost.ValType) == types.TypeID(newPost.ValType)) { return val, false, emptyCountParams, nil