From 30cd50f6aa936da620e25c1e002a467b6fb2ab65 Mon Sep 17 00:00:00 2001
From: Aman Mangal <aman@dgraph.io>
Date: Thu, 13 Feb 2020 10:37:25 +0530
Subject: [PATCH] Fix bug, aggregate value var works with blank node in upsert

Fixes #4712
---
 dgraph/cmd/alpha/upsert_test.go | 60 +++++++++++++++++++++++++++++++++
 edgraph/server.go               | 34 +++++++++++--------
 2 files changed, 80 insertions(+), 14 deletions(-)

diff --git a/dgraph/cmd/alpha/upsert_test.go b/dgraph/cmd/alpha/upsert_test.go
index a1c189d42ac..b76c44e286b 100644
--- a/dgraph/cmd/alpha/upsert_test.go
+++ b/dgraph/cmd/alpha/upsert_test.go
@@ -2802,6 +2802,66 @@ func TestUpsertMultiValueJson(t *testing.T) {
 	require.Equal(t, 2, len(result.Queries["user2"]))
 }
 
+func TestValVarWithBlankNode(t *testing.T) {
+	require.NoError(t, dropAll())
+	require.NoError(t, alterSchema(`version: int .`))
+
+	m := `
+upsert {
+  query {
+    q(func: has(version), orderdesc: version, first: 1) {
+      Ver as version
+      VerIncr as math(Ver + 1)
+    }
+
+    me() {
+      sVerIncr as sum(val(VerIncr))
+    }
+  }
+
+  mutation @if(gt(len(VerIncr), 0)) {
+    set {
+      _:newNode <version> val(sVerIncr) .
+    }
+  }
+
+  mutation @if(eq(len(VerIncr), 0)) {
+    set {
+      _:newNode <version> "1" .
+    }
+  }
+}`
+	mr, err := mutationWithTs(m, "application/rdf", false, true, 0)
+	require.NoError(t, err)
+	require.True(t, len(mr.keys) == 0)
+	require.Equal(t, []string{"version"}, splitPreds(mr.preds))
+
+	for i := 0; i < 10; i++ {
+		mr, err = mutationWithTs(m, "application/rdf", false, true, 0)
+		require.NoError(t, err)
+		require.True(t, len(mr.keys) == 0)
+		require.Equal(t, []string{"version"}, splitPreds(mr.preds))
+	}
+
+	q1 := `
+{
+  q(func: has(version), orderdesc: version, first: 1) {
+    version
+  }
+}`
+	res, _, err := queryWithTs(q1, "application/graphql+-", "", 0)
+	expectedRes := `
+{
+  "data": {
+    "q": [{
+       "version": 11
+     }]
+   }
+}`
+	require.NoError(t, err)
+	testutil.CompareJSON(t, res, expectedRes)
+}
+
 // This test may fail sometimes because ACL token
 // can get expired while the mutations is running.
 func upsertTooBigTest(t *testing.T) {
diff --git a/edgraph/server.go b/edgraph/server.go
index f8cb5f887c5..2855be06cee 100644
--- a/edgraph/server.go
+++ b/edgraph/server.go
@@ -473,7 +473,7 @@ func findMutationVars(qc *queryContext) []string {
 // Assumption is that Subject can contain UID, whereas Object can contain Val
 // If val(variable) exists in a query, but the values are not there for the variable,
 // it will ignore the mutation silently.
-func updateValInNQuads(nquads []*api.NQuad, qc *queryContext) []*api.NQuad {
+func updateValInNQuads(nquads []*api.NQuad, qc *queryContext, isSet bool) []*api.NQuad {
 	getNewVals := func(s string) (map[uint64]types.Val, bool) {
 		if strings.HasPrefix(s, "val(") {
 			varName := s[4 : len(s)-1]
@@ -512,18 +512,24 @@ func updateValInNQuads(nquads []*api.NQuad, qc *queryContext) []*api.NQuad {
 		// to *api.Value before applying the mutation. For that, first
 		// we convert key to uint64 and get the UID to Value map from
 		// the result of the query.
-		if nq.Subject[0] == '_' {
-			// UID is of format "_:uid(u)". Ignore silently
-			continue
-		}
-
-		key, err := strconv.ParseUint(nq.Subject, 0, 64)
-		if err != nil {
-			// Key conversion failed, ignoring the nquad. Ideally,
-			// it shouldn't happen as this is the result of a query.
-			glog.Errorf("Conversion of subject %s failed. Error: %s",
-				nq.Subject, err.Error())
+		var key uint64
+		var err error
+		switch {
+		case nq.Subject[0] == '_' && isSet:
+			// in case aggregate val(var) is there, that should work with blank node.
+			key = 0
+		case nq.Subject[0] == '_' && !isSet:
+			// UID is of format "_:uid(u)". Ignore the delete silently
 			continue
+		default:
+			key, err = strconv.ParseUint(nq.Subject, 0, 64)
+			if err != nil {
+				// Key conversion failed, ignoring the nquad. Ideally,
+				// it shouldn't happen as this is the result of a query.
+				glog.Errorf("Conversion of subject %s failed. Error: %s",
+					nq.Subject, err.Error())
+				continue
+			}
 		}
 
 		// Get the value to the corresponding UID(key) from the query result
@@ -551,8 +557,8 @@ func updateValInNQuads(nquads []*api.NQuad, qc *queryContext) []*api.NQuad {
 // updateValInMuations does following transformations:
 // 0x123 <amount> val(v) -> 0x123 <amount> 13.0
 func updateValInMutations(gmu *gql.Mutation, qc *queryContext) {
-	gmu.Del = updateValInNQuads(gmu.Del, qc)
-	gmu.Set = updateValInNQuads(gmu.Set, qc)
+	gmu.Del = updateValInNQuads(gmu.Del, qc, false)
+	gmu.Set = updateValInNQuads(gmu.Set, qc, true)
 }
 
 // updateUIDInMutations does following transformations: