Add ability to delete triples of scalar non-list predicates.

[martinmr]

Currently, only using * works to delete this type of triples. This change allows setting a value and the triple will be deleted if the existing value matches the value in the delete triple (or there is no existing value).

Fixes #2419

---

This change is 

[manishrjain]

Reviewed 4 of 4 files at r1.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @martinmr)

---

posting/index.go, line 321 at r1 (raw file):

	}

	// Check original value BEFORE any mutation actually happens.

You can keep the if above, and add another if about del star. That way, you only retrieve the value if any of these if conds are met.

---

posting/index.go, line 322 at r1 (raw file):

	// Check original value BEFORE any mutation actually happens.
	val, found, err = l.findValue(txn.StartTs, fingerprintEdge(t))

I think we might be able to get rid of the findValue, and just use findPosting directly. And as needed, convert it over to valType.

---

posting/index.go, line 333 at r1 (raw file):

		newVal := valueToTypesVal(mpost)

		if found && !reflect.DeepEqual(val, newVal) {

Not a fan of reflect package. We have been able to not use it directly in Dgraph so far. Possible to do this equality comparison otherwise? In fact, you could just write an equality func for val.

val.IsEqual(newVal), where you can check their types, and do comparisons accordingly? Seems like a bit of extra work, but would avoid future devs from using reflect pkg.

Update: Looks like if you get the posting, then you can just compare the postings directly, without any issues.

[martinmr]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @manishrjain)

---

posting/index.go, line 321 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

You can keep the if above, and add another if about del star. That way, you only retrieve the value if any of these if conds are met.

Done.

---

posting/index.go, line 322 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

I think we might be able to get rid of the findValue, and just use findPosting directly. And as needed, convert it over to valType.

Done.

---

posting/index.go, line 333 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Not a fan of reflect package. We have been able to not use it directly in Dgraph so far. Possible to do this equality comparison otherwise? In fact, you could just write an equality func for val.

val.IsEqual(newVal), where you can check their types, and do comparisons accordingly? Seems like a bit of extra work, but would avoid future devs from using reflect pkg.

Update: Looks like if you get the posting, then you can just compare the postings directly, without any issues.

Done.

Addressed comments

[martinmr]

Addressed all the comments. PR is ready for another round of review.

[manishrjain]

Got one very important comment. Do address that before merging.

Reviewed 1 of 1 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @martinmr)

---

posting/index.go, line 332 at r2 (raw file):

	if !schema.State().IsList(t.Attr) && t.Op == pb.DirectedEdge_DEL && string(t.Value) != x.Star {
		newPost := NewPosting(t)
		pFound, currPost, _ := l.findPosting(txn.StartTs, fingerprintEdge(t))

You're not checking error? Always handle errors. They should be returned.