Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug, aggregate value var works with blank node in upsert #4767

Merged
merged 1 commit into from
Mar 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -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]
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down