Skip to content

Commit

Permalink
Fix bug, aggregate value var works with blank node in upsert (#4767)
Browse files Browse the repository at this point in the history
Fixes #4712
  • Loading branch information
mangalaman93 authored and danielmai committed Apr 22, 2020
1 parent ab72b83 commit cfd9aa6
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 14 deletions.
60 changes: 60 additions & 0 deletions dgraph/cmd/alpha/upsert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
34 changes: 20 additions & 14 deletions edgraph/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,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]
Expand Down Expand Up @@ -469,18 +469,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
Expand Down Expand Up @@ -508,8 +514,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:
Expand Down

0 comments on commit cfd9aa6

Please sign in to comment.