From d6af37853169fccd5b266606c8d23beacdb66187 Mon Sep 17 00:00:00 2001 From: Aman Mangal Date: Tue, 6 Aug 2019 13:12:18 -0700 Subject: [PATCH] Add support for Conditional Upsert (#3612) --- dgraph/cmd/alpha/http.go | 7 + dgraph/cmd/alpha/upsert_test.go | 658 +++++++++++++++++++++++++++----- edgraph/server.go | 119 ++++-- gql/parser_mutation.go | 34 +- gql/state.go | 95 ++--- gql/upsert_test.go | 215 +++++++++-- 6 files changed, 916 insertions(+), 212 deletions(-) diff --git a/dgraph/cmd/alpha/http.go b/dgraph/cmd/alpha/http.go index 080ff55d9be..f006d631c47 100644 --- a/dgraph/cmd/alpha/http.go +++ b/dgraph/cmd/alpha/http.go @@ -326,6 +326,13 @@ func mutationHandler(w http.ResponseWriter, r *http.Request) { return } } + if condText, ok := ms["cond"]; ok && condText != nil { + mu.Cond, err = strconv.Unquote(string(condText.bs)) + if err != nil { + x.SetStatus(w, x.ErrorInvalidRequest, err.Error()) + return + } + } case "application/rdf": // Parse N-Quads. diff --git a/dgraph/cmd/alpha/upsert_test.go b/dgraph/cmd/alpha/upsert_test.go index 677e86747ac..1a9a84e8544 100644 --- a/dgraph/cmd/alpha/upsert_test.go +++ b/dgraph/cmd/alpha/upsert_test.go @@ -17,13 +17,13 @@ package alpha import ( + "fmt" "strings" "sync" "testing" - "github.com/dgraph-io/dgraph/testutil" - "github.com/dgraph-io/dgo/y" + "github.com/dgraph-io/dgraph/testutil" "github.com/stretchr/testify/require" ) @@ -45,16 +45,16 @@ func TestUpsertExample0(t *testing.T) { // Mutation with wrong name m1 := ` upsert { - mutation { - set { - uid(v) "Wrong" . - uid(v) "ashish@dgraph.io" . + query { + me(func: eq(email, "email@company.io")) { + v as uid } } - query { - me(func: eq(email, "ashish@dgraph.io")) { - v as uid + mutation { + set { + uid(v) "Wrong" . + uid(v) "email@company.io" . } } }` @@ -80,15 +80,15 @@ upsert { // mutation with correct name m2 := ` upsert { - mutation { - set { - uid(v) "Ashish" . + query { + me(func: eq(email, "email@company.io")) { + v as uid } } - query { - me(func: eq(email, "ashish@dgraph.io")) { - v as uid + mutation { + set { + uid(v) "Ashish" . } } }` @@ -110,7 +110,7 @@ func TestUpsertExample0JSON(t *testing.T) { // Mutation with wrong name m1 := ` { - "query": "{me(func: eq(email, \"ashish@dgraph.io\")) {v as uid}}", + "query": "{me(func: eq(email, \"email@company.io\")) {v as uid}}", "set": [ { "uid": "uid(v)", @@ -118,7 +118,7 @@ func TestUpsertExample0JSON(t *testing.T) { }, { "uid": "uid(v)", - "email": "ashish@dgraph.io" + "email": "email@company.io" } ] }` @@ -142,7 +142,7 @@ func TestUpsertExample0JSON(t *testing.T) { // mutation with correct name m2 := ` { - "query": "{me(func: eq(email, \"ashish@dgraph.io\")) {v as uid}}", + "query": "{me(func: eq(email, \"email@company.io\")) {v as uid}}", "set": [ { "uid": "uid(v)", @@ -169,12 +169,6 @@ friend: uid @reverse .`)) m1 := ` upsert { - mutation { - set { - _:user1 "45" . - } - } - query { me(func: eq(age, 34)) { ...fragmentA @@ -188,6 +182,12 @@ upsert { fragment fragmentA { uid } + + mutation { + set { + _:user1 "45" . + } + } }` _, _, _, err := mutationWithTs(m1, "application/rdf", false, true, true, 0) require.Contains(t, err.Error(), "upsert query block has no variables") @@ -201,12 +201,6 @@ friend: uid @reverse .`)) m1 := ` upsert { - mutation { - set { - uid(variable) "45" . - } - } - query { me(func: eq(age, 34)) { friend { @@ -218,6 +212,12 @@ upsert { fragment fragmentA { variable as uid } + + mutation { + set { + uid(variable) "45" . + } + } }` keys, preds, _, err := mutationWithTs(m1, "application/rdf", false, true, true, 0) require.NoError(t, err) @@ -257,13 +257,6 @@ friend: uid @reverse .`)) m1 := ` upsert { - mutation { - set { - uid(42) "45" . - uid(variable) "45" . - } - } - query { me(func: eq(age, 34)) { friend { @@ -275,6 +268,13 @@ upsert { fragment fragmentA { variable as uid } + + mutation { + set { + uid(42) "45" . + uid(variable) "45" . + } + } }` _, _, _, err := mutationWithTs(m1, "application/rdf", false, true, true, 0) require.Contains(t, err.Error(), "Some variables are used but not defined") @@ -291,12 +291,6 @@ friend: uid @reverse .`)) m1 := ` upsert { - mutation { - set { - uid(var2) "45" . - } - } - query { me(func: eq(age, 34)) { var2 as uid @@ -310,6 +304,12 @@ upsert { var1 as uid name } + + mutation { + set { + uid(var2) "45" . + } + } }` _, _, _, err := mutationWithTs(m1, "application/rdf", false, true, true, 0) require.Contains(t, err.Error(), "Some variables are defined but not used") @@ -340,12 +340,6 @@ friend: uid @reverse .`)) m1 := ` upsert { - mutation { - set { - uid( u) "true" . - } - } - query { var(func: has(age)) { a as age @@ -357,6 +351,12 @@ upsert { age } } + + mutation { + set { + uid( u) "true" . + } + } }` _, _, _, err = mutationWithTs(m1, "application/rdf", false, true, true, 0) require.NoError(t, err) @@ -377,17 +377,17 @@ upsert { m2 := ` upsert { - mutation { - delete { - uid (u1) * . - } - } - query { user1(func: eq(name@en, "user1")) { u1 as uid } } + + mutation { + delete { + uid (u1) * . + } + } }` _, _, _, err = mutationWithTs(m2, "application/rdf", false, true, true, 0) require.NoError(t, err) @@ -427,12 +427,6 @@ friend: uid @reverse .`)) m1 := ` upsert { - mutation { - set { - uid ( u1 ) uid ( u2 ) . - } - } - query { user1(func: eq(name@en, "user1")) { u1 as uid @@ -442,6 +436,12 @@ upsert { u2 as uid } } + + mutation { + set { + uid ( u1 ) uid ( u2 ) . + } + } }` _, _, _, err = mutationWithTs(m1, "application/rdf", false, true, true, 0) require.NoError(t, err) @@ -460,12 +460,6 @@ upsert { m2 := ` upsert { - mutation { - delete { - uid (u1) uid ( u2 ) . - } - } - query { user1(func: eq(name@en, "user1")) { u1 as uid @@ -475,6 +469,12 @@ upsert { u2 as uid } } + + mutation { + delete { + uid (u1) uid ( u2 ) . + } + } }` _, _, _, err = mutationWithTs(m2, "application/rdf", false, true, true, 0) require.NoError(t, err) @@ -515,14 +515,13 @@ friend: uid @reverse .`)) m1 := ` { + "query": "{var(func: has(age)) {a as age} oldest(func: uid(a), orderdesc: val(a), first: 1) {u as uid}}", "set": [ { "uid": "uid(u)", "oldest": "true" } - ], - - "query": "{var(func: has(age)) {a as age} oldest(func: uid(a), orderdesc: val(a), first: 1) {u as uid}}" + ] }` _, _, _, err = mutationWithTs(m1, "application/json", false, true, true, 0) require.NoError(t, err) @@ -543,14 +542,13 @@ friend: uid @reverse .`)) m2 := ` { + "query": "{user1(func: eq(name@en, \"user1\")) {u1 as uid}}", "delete": [ { "uid": "uid (u1)", "name": null } - ], - - "query": "{user1(func: eq(name@en, \"user1\")) {u1 as uid}}" + ] }` _, _, _, err = mutationWithTs(m2, "application/json", false, true, true, 0) require.NoError(t, err) @@ -590,14 +588,13 @@ friend: uid @reverse .`)) m1 := ` { + "query": "{user1(func: eq(name@en, \"user1\")) {u1 as uid} user2(func: eq(name@en, \"user2\")) {u2 as uid}}", "set": [ { "uid": "uid(u1)", "friend": "uid (u2 ) " } - ], - - "query": "{user1(func: eq(name@en, \"user1\")) {u1 as uid} user2(func: eq(name@en, \"user2\")) {u2 as uid}}" + ] }` _, _, _, err = mutationWithTs(m1, "application/json", false, true, true, 0) require.NoError(t, err) @@ -616,14 +613,13 @@ friend: uid @reverse .`)) m3 := ` { + "query": "{user1(func: eq(name@en, \"user1\")) {u1 as uid} user2(func: eq(name@en, \"user2\")) {u2 as uid}}", "delete": [ { "uid": "uid (u1)", "friend": "uid ( u2 )" } - ], - - "query": "{user1(func: eq(name@en, \"user1\")) {u1 as uid} user2(func: eq(name@en, \"user2\")) {u2 as uid}}" + ] }` _, _, _, err = mutationWithTs(m3, "application/json", false, true, true, 0) require.NoError(t, err) @@ -647,18 +643,18 @@ func TestUpsertBlankNodeWithVar(t *testing.T) { m := ` upsert { + query { + users(func: eq(name, "user1")) { + u as uid + } + } + mutation { set { uid(u) "user1" . _:u "user2" . } } - - query { - users(func: eq(name, "user1")) { - u as uid - } - } }` _, _, _, err := mutationWithTs(m, "application/rdf", false, true, true, 0) require.NoError(t, err) @@ -685,6 +681,20 @@ friend: uid @reverse .`)) m := ` upsert { + query { + user1(func: eq(email, "user1@dgraph.io")) { + u1 as uid + } + + user2(func: eq(email, "user2@dgraph.io")) { + u2 as uid + } + + user3(func: eq(email, "user3@dgraph.io")) { + u3 as uid + } + } + mutation { set { uid(u1) "user1@dgraph.io" . @@ -700,20 +710,6 @@ upsert { uid(u3) * . } } - - query { - user1(func: eq(email, "user1@dgraph.io")) { - u1 as uid - } - - user2(func: eq(email, "user2@dgraph.io")) { - u2 as uid - } - - user3(func: eq(email, "user3@dgraph.io")) { - u3 as uid - } - } }` doUpsert := func(wg *sync.WaitGroup) { defer wg.Done() @@ -775,22 +771,476 @@ friend: uid @reverse .`)) m := ` upsert { + query { + user1(func: eq(name@en, "user1")) { + u1 as uid + } + + user2(func: eq(name@en, "user2")) { + u2 as uid + } + } + mutation { delete { uid (u1) uid ( u2 ) . } } +}` + _, _, _, err := mutationWithTs(m, "application/rdf", false, true, true, 0) + require.NoError(t, err) +} + +func TestConditionalUpsertExample0(t *testing.T) { + require.NoError(t, dropAll()) + require.NoError(t, alterSchema(`email: string @index(exact) .`)) + // Mutation with wrong name + m1 := ` +upsert { query { - user1(func: eq(name@en, "user1")) { - u1 as uid + me(func: eq(email, "email@company.io")) { + v as uid } + } - user2(func: eq(name@en, "user2")) { - u2 as uid + mutation @if(eq(len(v), 0)) { + set { + uid(v) "Wrong" . + uid(v) "email@company.io" . } } }` - _, _, _, err := mutationWithTs(m, "application/rdf", false, true, true, 0) + keys, preds, _, err := mutationWithTs(m1, "application/rdf", false, true, true, 0) + require.NoError(t, err) + require.True(t, len(keys) == 0) + require.True(t, contains(preds, "email")) + require.True(t, contains(preds, "name")) + + // Trying again, should be a NOOP + _, _, _, err = mutationWithTs(m1, "application/rdf", false, true, true, 0) + require.NoError(t, err) + + // query should return the wrong name + q1 := ` +{ + q(func: has(email)) { + uid + name + email + } +}` + res, _, err := queryWithTs(q1, "application/graphql+-", "", 0) + require.NoError(t, err) + require.Contains(t, res, "Wrong") + + // mutation with correct name + m2 := ` +upsert { + query { + me(func: eq(email, "email@company.io")) { + v as uid + } + } + + mutation @if(eq(len(v), 1)) { + set { + uid(v) "Ashish" . + } + } +}` + keys, preds, _, err = mutationWithTs(m2, "application/rdf", false, true, true, 0) require.NoError(t, err) + require.True(t, len(keys) == 0) + require.True(t, contains(preds, "name")) + + // query should return correct name + res, _, err = queryWithTs(q1, "application/graphql+-", "", 0) + require.NoError(t, err) + require.Contains(t, res, "Ashish") +} + +func TestConditionalUpsertExample0JSON(t *testing.T) { + require.NoError(t, dropAll()) + require.NoError(t, alterSchema(`email: string @index(exact) .`)) + + // Mutation with wrong name + m1 := ` +{ + "query": "{me(func: eq(email, \"email@company.io\")) {v as uid}}", + "cond": " @if(eq(len(v), 0)) ", + "set": [ + { + "uid": "uid(v)", + "name": "Wrong" + }, + { + "uid": "uid(v)", + "email": "email@company.io" + } + ] +}` + keys, _, _, err := mutationWithTs(m1, "application/json", false, true, true, 0) + require.NoError(t, err) + require.True(t, len(keys) == 0) + + // query should return the wrong name + q1 := ` +{ + q(func: has(email)) { + uid + name + email + } +}` + res, _, err := queryWithTs(q1, "application/graphql+-", "", 0) + require.NoError(t, err) + require.Contains(t, res, "Wrong") + + // mutation with correct name + m2 := ` +{ + "query": "{me(func: eq(email, \"email@company.io\")) {v as uid}}", + "cond": "@if(eq(len(v), 1))", + "set": [ + { + "uid": "uid(v)", + "name": "Ashish" + } + ] +}` + keys, preds, _, err := mutationWithTs(m2, "application/json", false, true, true, 0) + require.NoError(t, err) + require.True(t, len(keys) == 0) + require.True(t, contains(preds, "name")) + + // query should return correct name + res, _, err = queryWithTs(q1, "application/graphql+-", "", 0) + require.NoError(t, err) + require.Contains(t, res, "Ashish") +} + +func populateCompanyData(t *testing.T) { + require.NoError(t, alterSchema(` +email: string @index(exact) . +works_for: string @index(exact) . +works_with: [uid] .`)) + + m1 := ` +{ + set { + _:user1 "user1" . + _:user1 "user1@company1.io" . + _:user1 "company1" . + + _:user2 "user2" . + _:user2 "user2@company1.io" . + _:user2 "company1" . + + _:user3 "user3" . + _:user3 "user3@company2.io" . + _:user3 "company2" . + + _:user4 "user4" . + _:user4 "user4@company2.io" . + _:user4 "company2" . + } +}` + _, _, _, err := mutationWithTs(m1, "application/rdf", false, true, true, 0) + require.NoError(t, err) +} + +func TestUpsertMultiValue(t *testing.T) { + require.NoError(t, dropAll()) + populateCompanyData(t) + + // add color to all employees of company1 + m2 := ` +upsert { + query { + me(func: eq(works_for, "company1")) { + u as uid + } + } + + mutation { + set { + uid(u) "red" . + } + } +}` + keys, preds, _, err := mutationWithTs(m2, "application/rdf", false, true, true, 0) + require.NoError(t, err) + require.True(t, len(keys) == 0) + require.True(t, contains(preds, "color")) + require.False(t, contains(preds, "works_for")) + + q2 := ` +{ + q(func: eq(works_for, "%s")) { + name + works_for + color + works_with + } +}` + res, _, err := queryWithTs(fmt.Sprintf(q2, "company1"), "application/graphql+-", "", 0) + require.NoError(t, err) + testutil.CompareJSON(t, `{"data":{"q":[{"name":"user1","works_for":"company1","color":"red"},`+ + `{"name":"user2","works_for":"company1","color":"red"}]}}`, res) + + // delete color for employess of company1 and set color for employees of company2 + m3 := ` +upsert { + query { + c1 as var(func: eq(works_for, "company1")) + c2 as var(func: eq(works_for, "company2")) + } + + mutation @if(le(len(c1), 100) AND lt(len(c2), 100)) { + delete { + uid(c1) * . + } + + set { + uid(c2) "blue" . + } + } +}` + keys, preds, _, err = mutationWithTs(m3, "application/rdf", false, true, true, 0) + require.NoError(t, err) + + // The following mutation should have no effect on the state of the database + m4 := ` +upsert { + query { + c1 as var(func: eq(works_for, "company1")) + c2 as var(func: eq(works_for, "company2")) + } + + mutation @if(gt(len(c1), 2) OR ge(len(c2), 3)) { + delete { + uid(c1) * . + } + + set { + uid(c2) "blue" . + } + } +}` + keys, preds, _, err = mutationWithTs(m4, "application/rdf", false, true, true, 0) + require.NoError(t, err) + + res, _, err = queryWithTs(fmt.Sprintf(q2, "company1"), "application/graphql+-", "", 0) + require.NoError(t, err) + testutil.CompareJSON(t, `{"data":{"q":[{"name":"user1","works_for":"company1"},`+ + `{"name":"user2","works_for":"company1"}]}}`, res) + + res, _, err = queryWithTs(fmt.Sprintf(q2, "company2"), "application/graphql+-", "", 0) + require.NoError(t, err) + testutil.CompareJSON(t, `{"data":{"q":[{"name":"user3","works_for":"company2","color":"blue"},`+ + `{"name":"user4","works_for":"company2","color":"blue"}]}}`, res) +} + +func TestUpsertMultiValueEdge(t *testing.T) { + require.NoError(t, dropAll()) + populateCompanyData(t) + + // All employees of company1 now works with all employees of company2 + m1 := ` +upsert { + query { + c1 as var(func: eq(works_for, "company1")) + c2 as var(func: eq(works_for, "company2")) + } + + mutation @if(eq(len(c1), 2) AND eq(len(c2), 2)) { + set { + uid(c1) uid(c2) . + uid(c2) uid(c1) . + } + } +}` + _, _, _, err := mutationWithTs(m1, "application/rdf", false, true, true, 0) + require.NoError(t, err) + + q1 := ` +{ + q(func: eq(works_for, "%s")) { + name + works_with { + name + } + } +}` + res, _, err := queryWithTs(fmt.Sprintf(q1, "company1"), "application/graphql+-", "", 0) + require.NoError(t, err) + testutil.CompareJSON(t, `{"data":{"q":[{"name":"user2","works_with":[{"name":"user3"},{"name":"user4"}]},`+ + `{"name":"user1","works_with":[{"name":"user3"},{"name":"user4"}]}]}}`, res) + + res, _, err = queryWithTs(fmt.Sprintf(q1, "company2"), "application/graphql+-", "", 0) + require.NoError(t, err) + testutil.CompareJSON(t, `{"data":{"q":[{"name":"user3","works_with":[{"name":"user1"},{"name":"user2"}]},`+ + `{"name":"user4","works_with":[{"name":"user1"},{"name":"user2"}]}]}}`, res) + + // user1 and user3 do not work with each other anymore + m2 := ` +upsert { + query { + u1 as var(func: eq(email, "user1@company1.io")) + u3 as var(func: eq(email, "user3@company2.io")) + } + + mutation @if(eq(len(u1), 1) AND eq(len(u3), 1)) { + delete { + uid (u1) uid (u3) . + uid (u3) uid (u1) . + } + } +}` + _, _, _, err = mutationWithTs(m2, "application/rdf", false, true, true, 0) + require.NoError(t, err) + + res, _, err = queryWithTs(fmt.Sprintf(q1, "company1"), "application/graphql+-", "", 0) + require.NoError(t, err) + testutil.CompareJSON(t, `{"data":{"q":[{"name":"user1","works_with":[{"name":"user4"}]},`+ + `{"name":"user2","works_with":[{"name":"user4"},{"name":"user3"}]}]}}`, res) + + res, _, err = queryWithTs(fmt.Sprintf(q1, "company2"), "application/graphql+-", "", 0) + require.NoError(t, err) + testutil.CompareJSON(t, `{"data":{"q":[{"name":"user3","works_with":[{"name":"user2"}]},`+ + `{"name":"user4","works_with":[{"name":"user1"},{"name":"user2"}]}]}}`, res) +} + +func TestUpsertEdgeWithBlankNode(t *testing.T) { + require.NoError(t, dropAll()) + populateCompanyData(t) + + // Add a new employee who works with every employee in company2 + m1 := ` +upsert { + query { + c1 as var(func: eq(works_for, "company1")) + c2 as var(func: eq(works_for, "company2")) + } + + mutation @if(lt(len(c1), 3)) { + set { + _:user5 "user5" . + _:user5 "user5@company1.io" . + _:user5 "company1" . + _:user5 uid(c2) . + } + } +}` + _, _, _, err := mutationWithTs(m1, "application/rdf", false, true, true, 0) + require.NoError(t, err) + + q1 := ` +{ + q(func: eq(email, "user5@company1.io")) { + name + email + works_for + works_with { + name + } + } +}` + res, _, err := queryWithTs(q1, "application/graphql+-", "", 0) + require.NoError(t, err) + testutil.CompareJSON(t, `{"data":{"q":[{"name":"user5","email":"user5@company1.io",`+ + `"works_for":"company1","works_with":[{"name":"user3"},{"name":"user4"}]}]}}`, res) +} + +func TestConditionalUpsertWithFilterErr(t *testing.T) { + require.NoError(t, dropAll()) + populateCompanyData(t) + + m1 := ` +upsert { + query { + me(func: eq(email, "email@company.io")) { + v as uid + } + } + + mutation @filter(eq(len(v), 0)) { + set { + uid(v) "Wrong" . + uid(v) "email@company.io" . + } + } +}` + _, _, _, err := mutationWithTs(m1, "application/rdf", false, true, true, 0) + require.Contains(t, err.Error(), "Expected @if, found [@filter]") +} + +func TestConditionalUpsertMissingAtErr(t *testing.T) { + require.NoError(t, dropAll()) + populateCompanyData(t) + + m1 := ` +upsert { + query { + me(func: eq(email, "email@company.io")) { + v as uid + } + } + + mutation if(eq(len(v), 0)) { + set { + uid(v) "Wrong" . + uid(v) "email@company.io" . + } + } +}` + _, _, _, err := mutationWithTs(m1, "application/rdf", false, true, true, 0) + require.Contains(t, err.Error(), `Unrecognized character inside mutation: U+0028 '('`) +} + +func TestConditionalUpsertDoubleIfErr(t *testing.T) { + require.NoError(t, dropAll()) + populateCompanyData(t) + + m1 := ` +upsert { + query { + me(func: eq(email, "email@company.io")) { + v as uid + } + } + + mutation @if(eq(len(v), 0)) @if(eq(len(v), 0)) { + set { + uid(v) "Wrong" . + uid(v) "email@company.io" . + } + } +}` + _, _, _, err := mutationWithTs(m1, "application/rdf", false, true, true, 0) + require.Contains(t, err.Error(), "Expected { at the start of block") +} + +func TestConditionalUpsertMissingRightRoundErr(t *testing.T) { + require.NoError(t, dropAll()) + populateCompanyData(t) + + m1 := ` +upsert { + query { + me(func: eq(email, "email@company.io")) { + v as uid + } + } + + mutation @if(eq(len(v), 0) { + set { + uid(v) "Wrong" . + uid(v) "email@company.io" . + } + } +}` + _, _, _, err := mutationWithTs(m1, "application/rdf", false, true, true, 0) + require.Contains(t, err.Error(), "Matching brackets not found") } diff --git a/edgraph/server.go b/edgraph/server.go index d2d88a3c4a4..68a539398fe 100644 --- a/edgraph/server.go +++ b/edgraph/server.go @@ -21,8 +21,10 @@ import ( "encoding/json" "fmt" "math" + "math/rand" "os" "sort" + "strconv" "strings" "time" "unicode" @@ -577,40 +579,79 @@ func doQueryInUpsert(ctx context.Context, mu *api.Mutation, gmu *gql.Mutation) ( return l, nil } + upsertQuery := mu.Query needVars := findVars(gmu) + isCondUpsert := strings.TrimSpace(mu.Cond) != "" + varName := fmt.Sprintf("__dgraph%d__", rand.Int()) + if isCondUpsert { + // @if in upsert is same as @filter in the query + cond := strings.Replace(mu.Cond, "@if", "@filter", 1) + + // Add dummy query to evaluate the @if directive, ok to use uid(0) because + // dgraph doesn't check for existence of UIDs until we query for other predicates. + // Here, we are only querying for uid predicate in the dummy query. + // + // For example if - mu.Query = { + // me(...) {...} + // } + // + // Then, upsertQuery = { + // me(...) {...} + // __dgraph0__ as var(func: uid(0)) @if(...) + // } + // + // The variable __dgraph0__ will - + // * be empty if the condition is true + // * have 1 UID (the 0 UID) if the condition is false + upsertQuery = strings.TrimSuffix(strings.TrimSpace(mu.Query), "}") + upsertQuery += varName + ` as var(func: uid(0)) ` + cond + `}` + needVars = append(needVars, varName) + } + startParsingTime := time.Now() parsedReq, err := gql.ParseWithNeedVars(gql.Request{ - Str: mu.Query, + Str: upsertQuery, Variables: make(map[string]string), }, needVars) l.Parsing += time.Since(startParsingTime) if err != nil { - return nil, errors.Wrapf(err, "while parsing query: %q", mu.Query) + return nil, errors.Wrapf(err, "while parsing query: %q", upsertQuery) } if err := validateQuery(parsedReq.Query); err != nil { - return nil, errors.Wrapf(err, "while validating query: %q", mu.Query) + return nil, errors.Wrapf(err, "while validating query: %q", upsertQuery) } qr := query.Request{Latency: l, GqlQuery: &parsedReq, ReadTs: mu.StartTs} if err := qr.ProcessQuery(ctx); err != nil { - return nil, errors.Wrapf(err, "while processing query: %q", mu.Query) + return nil, errors.Wrapf(err, "while processing query: %q", upsertQuery) } if len(qr.Vars) <= 0 { return nil, errors.Errorf("upsert query block has no variables") } - // TODO(Aman): allow multiple values for each variable. // If a variable doesn't have any UID, we generate one ourselves later. - varToUID := make(map[string]string) + varToUID := make(map[string][]string) for name, v := range qr.Vars { - if v.Uids == nil { + if v.Uids == nil || len(v.Uids.Uids) <= 0 { continue } - if len(v.Uids.Uids) > 1 { - return nil, errors.Errorf("more than one values found for var (%s)", name) - } else if len(v.Uids.Uids) == 1 { - varToUID[name] = fmt.Sprintf("%d", v.Uids.Uids[0]) + + uids := make([]string, len(v.Uids.Uids)) + for i, u := range v.Uids.Uids { + uids[i] = strconv.FormatUint(u, 10) + } + varToUID[name] = uids + } + + // If @if condition is false, no need to process the mutations + if isCondUpsert { + v, ok := qr.Vars[varName] + isMut := ok && v.Uids != nil && len(v.Uids.Uids) == 1 + if !isMut { + gmu.Set = nil + gmu.Del = nil + return l, nil } } @@ -650,39 +691,65 @@ func findVars(gmu *gql.Mutation) []string { // updateMutations does following transformations: // * uid(v) -> 0x123 -- If v is defined in query block // * uid(v) -> _:uid(v) -- Otherwise -func updateMutations(gmu *gql.Mutation, varToUID map[string]string) { - getNewVal := func(s string) string { +func updateMutations(gmu *gql.Mutation, varToUID map[string][]string) { + getNewVals := func(s string) []string { if strings.HasPrefix(s, "uid(") { varName := s[4 : len(s)-1] - if uid, ok := varToUID[varName]; ok { - return uid + if uids, ok := varToUID[varName]; ok { + return uids } - return "_:" + s + return []string{"_:" + s} } - return s + return []string{s} + } + + getNewNQuad := func(nq *api.NQuad, s, o string) *api.NQuad { + // The following copy is fine because we only modify Subject and ObjectId. + // The pointer values are not modified across different copies of NQuad. + n := *nq + + n.Subject = s + n.ObjectId = o + return &n } // Remove the mutations from gmu.Del when no UID was found. - gmuDel := gmu.Del[:0] + gmuDel := make([]*api.NQuad, 0, len(gmu.Del)) for _, nq := range gmu.Del { - nq.Subject = getNewVal(nq.Subject) - nq.ObjectId = getNewVal(nq.ObjectId) - - if !strings.HasPrefix(nq.Subject, "_:uid(") && - !strings.HasPrefix(nq.ObjectId, "_:uid(") { + // if Subject or/and Object are variables, each NQuad can result + // in multiple NQuads if any variable stores more than one UIDs. + newSubs := getNewVals(nq.Subject) + newObs := getNewVals(nq.ObjectId) + + for _, s := range newSubs { + for _, o := range newObs { + // Blank node has no meaning in case of deletion. + if strings.HasPrefix(s, "_:uid(") || + strings.HasPrefix(o, "_:uid(") { + continue + } - gmuDel = append(gmuDel, nq) + gmuDel = append(gmuDel, getNewNQuad(nq, s, o)) + } } } gmu.Del = gmuDel // Update the values in mutation block from the query block. + gmuSet := make([]*api.NQuad, 0, len(gmu.Set)) for _, nq := range gmu.Set { - nq.Subject = getNewVal(nq.Subject) - nq.ObjectId = getNewVal(nq.ObjectId) + newSubs := getNewVals(nq.Subject) + newObs := getNewVals(nq.ObjectId) + + for _, s := range newSubs { + for _, o := range newObs { + gmuSet = append(gmuSet, getNewNQuad(nq, s, o)) + } + } } + gmu.Set = gmuSet } // Query handles queries and returns the data. diff --git a/gql/parser_mutation.go b/gql/parser_mutation.go index d8c8d9798e1..76b6fca6f72 100644 --- a/gql/parser_mutation.go +++ b/gql/parser_mutation.go @@ -39,11 +39,11 @@ func ParseMutation(mutation string) (mu *api.Mutation, err error) { item := it.Item() switch item.Typ { case itemUpsertBlock: - if mu, err = ParseUpsertBlock(it); err != nil { + if mu, err = parseUpsertBlock(it); err != nil { return nil, err } case itemLeftCurl: - if mu, err = ParseMutationBlock(it); err != nil { + if mu, err = parseMutationBlock(it); err != nil { return nil, err } default: @@ -58,11 +58,11 @@ func ParseMutation(mutation string) (mu *api.Mutation, err error) { return mu, nil } -// ParseUpsertBlock parses the upsert block -func ParseUpsertBlock(it *lex.ItemIterator) (*api.Mutation, error) { +// parseUpsertBlock parses the upsert block +func parseUpsertBlock(it *lex.ItemIterator) (*api.Mutation, error) { var mu *api.Mutation - var queryText string - var queryFound bool + var queryText, condText string + var queryFound, condFound bool // ===>upsert<=== {...} if !it.Next() { @@ -109,10 +109,26 @@ func ParseUpsertBlock(it *lex.ItemIterator) (*api.Mutation, error) { if !it.Next() { return nil, it.Errorf("Unexpected end of upsert block") } + + // upsert { mutation ===>@if(...)<=== {....} query{...}} + item = it.Item() + if item.Typ == itemUpsertBlockOpContent { + if condFound { + return nil, it.Errorf("Multiple @if directive inside upsert block") + } + condFound = true + condText = item.Val + if !it.Next() { + return nil, it.Errorf("Unexpected end of upsert block") + } + } + + // upsert @if(...) ===>{<=== ....} var err error - if mu, err = ParseMutationBlock(it); err != nil { + if mu, err = parseMutationBlock(it); err != nil { return nil, err } + mu.Cond = condText // upsert { mutation{...} ===>fragment<==={...}} case item.Typ == itemUpsertBlockOp && item.Val == "fragment": @@ -133,8 +149,8 @@ func ParseUpsertBlock(it *lex.ItemIterator) (*api.Mutation, error) { return nil, it.Errorf("Invalid upsert block") } -// ParseMutationBlock parses the mutation block -func ParseMutationBlock(it *lex.ItemIterator) (*api.Mutation, error) { +// parseMutationBlock parses the mutation block +func parseMutationBlock(it *lex.ItemIterator) (*api.Mutation, error) { var mu api.Mutation item := it.Item() diff --git a/gql/state.go b/gql/state.go index e1d12645065..4ca6c90a7b7 100644 --- a/gql/state.go +++ b/gql/state.go @@ -91,20 +91,14 @@ func lexIdentifyBlock(l *lex.Lexer) lex.StateFn { // lexNameBlock lexes the blocks, for now, only upsert block func lexNameBlock(l *lex.Lexer) lex.StateFn { - for { - // The caller already checked isNameBegin, and absorbed one rune. - r := l.Next() - if isNameSuffix(r) { - continue - } - l.Backup() - switch word := l.Input[l.Start:l.Pos]; word { - case "upsert": - l.Emit(itemUpsertBlock) - return lexUpsertBlock - default: - return l.Errorf("Invalid block: [%s]", word) - } + // The caller already checked isNameBegin, and absorbed one rune. + l.AcceptRun(isNameSuffix) + switch word := l.Input[l.Start:l.Pos]; word { + case "upsert": + l.Emit(itemUpsertBlock) + return lexUpsertBlock + default: + return l.Errorf("Invalid block: [%s]", word) } } @@ -138,59 +132,76 @@ func lexUpsertBlock(l *lex.Lexer) lex.StateFn { // lexNameUpsertOp parses the operation names inside upsert block func lexNameUpsertOp(l *lex.Lexer) lex.StateFn { - for { - // The caller already checked isNameBegin, and absorbed one rune. - r := l.Next() - if isNameSuffix(r) { - continue - } - l.Backup() - word := l.Input[l.Start:l.Pos] - switch word { - case "query": - l.Emit(itemUpsertBlockOp) - return lexBlockContent - case "mutation": - l.Emit(itemUpsertBlockOp) - return lexInsideMutation - case "fragment": - l.Emit(itemUpsertBlockOp) - return lexBlockContent - default: - return l.Errorf("Invalid operation type: %s", word) - } + // The caller already checked isNameBegin, and absorbed one rune. + l.AcceptRun(isNameSuffix) + word := l.Input[l.Start:l.Pos] + switch word { + case "query": + l.Emit(itemUpsertBlockOp) + return lexBlockContent + case "mutation": + l.Emit(itemUpsertBlockOp) + return lexInsideMutation + case "fragment": + l.Emit(itemUpsertBlockOp) + return lexBlockContent + default: + return l.Errorf("Invalid operation type: %s", word) } } // lexBlockContent lexes and absorbs the text inside a block (covered by braces). func lexBlockContent(l *lex.Lexer) lex.StateFn { + return lexContent(l, leftCurl, rightCurl, lexUpsertBlock) +} + +// lexIfContent lexes the whole of @if directive in a mutation block (covered by small brackets) +func lexIfContent(l *lex.Lexer) lex.StateFn { + if r := l.Next(); r != at { + return l.Errorf("Expected [@], found; [%#U]", r) + } + + l.AcceptRun(isNameSuffix) + word := l.Input[l.Start:l.Pos] + if word != "@if" { + return l.Errorf("Expected @if, found [%v]", word) + } + + return lexContent(l, '(', ')', lexInsideMutation) +} + +func lexContent(l *lex.Lexer, leftRune, rightRune rune, returnTo lex.StateFn) lex.StateFn { depth := 0 for { switch l.Next() { case lex.EOF: - return l.Errorf("Unclosed block (matching braces not found)") + return l.Errorf("Matching brackets not found") case quote: if err := l.LexQuotedString(); err != nil { return l.Errorf(err.Error()) } - case leftCurl: + case leftRune: depth++ - case rightCurl: + case rightRune: depth-- if depth < 0 { - return l.Errorf("Unopened } found") + return l.Errorf("Unopened %c found", rightRune) } else if depth == 0 { l.Emit(itemUpsertBlockOpContent) - return lexUpsertBlock + return returnTo } } } + } func lexInsideMutation(l *lex.Lexer) lex.StateFn { l.Mode = lexInsideMutation for { switch r := l.Next(); { + case r == at: + l.Backup() + return lexIfContent case r == rightCurl: l.Depth-- l.Emit(itemRightCurl) @@ -586,10 +597,8 @@ func lexOperationType(l *lex.Lexer) lex.StateFn { l.Emit(itemOpType) return lexInsideSchema } else { - l.Errorf("Invalid operation type: %s", word) + return l.Errorf("Invalid operation type: %s", word) } - - return lexQuery } // lexArgName lexes and emits the name part of an argument. diff --git a/gql/upsert_test.go b/gql/upsert_test.go index de02e6c53e4..7318d0616ad 100644 --- a/gql/upsert_test.go +++ b/gql/upsert_test.go @@ -79,12 +79,6 @@ upsert { func TestMultipleQueryErr(t *testing.T) { query := ` upsert { - mutation { - set { - "_:user1" "45" . - } - } - query { me(func: eq(age, 34)) { uid @@ -104,6 +98,12 @@ upsert { } } } + + mutation { + set { + "_:user1" "45" . + } + } } ` _, err := ParseMutation(query) @@ -159,12 +159,6 @@ upsert { func TestUpsertWithFragment(t *testing.T) { query := ` upsert { - mutation { - set { - "_:user1" "45" . - } - } - query { me(func: eq(age, 34)) { ...fragmentA @@ -178,6 +172,12 @@ upsert { fragment fragmentA { uid } + + mutation { + set { + "_:user1" "45" . + } + } } ` _, err := ParseMutation(query) @@ -187,12 +187,6 @@ upsert { func TestUpsertEx1(t *testing.T) { query := ` upsert { - mutation { - set { - "_:user1" "45" . - } - } - query { me(func: eq(age, "{")) { uid @@ -202,6 +196,12 @@ upsert { } } } + + mutation { + set { + "_:user1" "45" . + } + } } ` _, err := ParseMutation(query) @@ -213,6 +213,18 @@ func TestUpsertWithSpaces(t *testing.T) { upsert { + query + + { + me(func: eq(age, "{")) { + uid + friend { + uid + age + } + } + } + mutation { @@ -223,11 +235,23 @@ upsert # This is a comment "_:user1" "{vishesh" . }} +} +` + _, err := ParseMutation(query) + require.Nil(t, err) +} - query +func TestUpsertWithBlankNode(t *testing.T) { + query := ` +upsert { + mutation { + set { + "_:user1" "45" . + } + } - { - me(func: eq(age, "{")) { + query { + me(func: eq(age, 34)) { uid friend { uid @@ -241,7 +265,7 @@ upsert require.Nil(t, err) } -func TestUpsertWithBlankNode(t *testing.T) { +func TestUpsertMutationThenQuery(t *testing.T) { query := ` upsert { query { @@ -265,17 +289,63 @@ upsert { require.Nil(t, err) } -func TestUpsertMutationThenQuery(t *testing.T) { +func TestUpsertWithFilter(t *testing.T) { query := ` upsert { + query { + me(func: eq(age, 34)) @filter(ge(name, "user")) { + uid + friend { + uid + age + } + } + } + mutation { set { - "_:user1" "45" . + uid(a) "45" + uid(b) "45" . } } +} +` + _, err := ParseMutation(query) + require.Nil(t, err) +} +func TestConditionalUpsertWithNewlines(t *testing.T) { + query := ` +upsert { query { - me(func: eq(age, 34)) { + me(func: eq(age, 34)) @filter(ge(name, "user")) { + m as uid + friend { + f as uid + age + } + } + } + + mutation @if(eq(len(m), 1) + AND + gt(len(f), 0)) { + set { + uid(m) "45" . + uid(f) "45" . + } + } +} +` + _, err := ParseMutation(query) + require.Nil(t, err) +} + +func TestConditionalUpsertFuncTree(t *testing.T) { + query := ` +upsert { + query { + me(func: eq(age, 34)) @filter(ge(name, "user")) { uid friend { uid @@ -283,18 +353,103 @@ upsert { } } } + + mutation @if( ( eq(len(m), 1) + OR + lt(90, len(h))) + AND + gt(len(f), 0)) { + set { + uid(m) "45" . + uid(f) "45" . + } + } } ` _, err := ParseMutation(query) require.Nil(t, err) } -func TestUpsertWithFilter(t *testing.T) { +func TestConditionalUpsertMultipleFuncArg(t *testing.T) { + query := ` +upsert { + query { + me(func: eq(age, 34)) @filter(ge(name, "user")) { + uid + friend { + uid + age + } + } + } + + mutation @if( ( eq(len(m), len(t)) + OR + lt(90, len(h))) + AND + gt(len(f), 0)) { + set { + uid(m) "45" . + uid(f) "45" . + } + } +} +` + _, err := ParseMutation(query) + require.Nil(t, err) +} + +func TestConditionalUpsertErrMissingRightRound(t *testing.T) { + query := ` +upsert { + query { + me(func: eq(age, 34)) @filter(ge(name, "user")) { + uid + friend { + uid + age + } + } + } + + mutation @if(eq(len(m, 1) + AND + gt(len(f), 0)) { + set { + uid(m) "45" . + uid(f) "45" . + } + } +} +` + _, err := ParseMutation(query) + require.Contains(t, err.Error(), "Matching brackets not found") +} + +func TestConditionalUpsertErrUnclosed(t *testing.T) { query := `upsert { - mutation { + mutation @if(eq(len(m), 1) AND gt(len(f), 0))` + _, err := ParseMutation(query) + require.Contains(t, err.Error(), "Unclosed mutation action") +} + +func TestConditionalUpsertErrInvalidIf(t *testing.T) { + query := `upsert { + mutation @if` + _, err := ParseMutation(query) + require.Contains(t, err.Error(), "Matching brackets not found") +} + +func TestConditionalUpsertErrWrongIf(t *testing.T) { + query := `upsert { + mutation @fi( ( eq(len(m), 1) + OR + lt(len(h), 90)) + AND + gt(len(f), 0)) { set { - uid(a) "45" - uid(b) "45" . + uid(m) "45" . + uid(f) "45" . } } @@ -310,5 +465,5 @@ func TestUpsertWithFilter(t *testing.T) { } ` _, err := ParseMutation(query) - require.Nil(t, err) + require.Contains(t, err.Error(), "Expected @if, found [@fi]") }