Skip to content

Commit

Permalink
minor changes
Browse files Browse the repository at this point in the history
  • Loading branch information
@mangalaman93
mangalaman93 committed Nov 7, 2019
1 parent 5180cf2 commit 24819a8
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 74 deletions.
1 change: 1 addition & 0 deletions dgraph/cmd/alpha/http_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ func mutationWithTs(m, t string, isJson bool, commitNow bool, ts uint64) (
mr.keys = r.Extensions.Txn.Keys
mr.preds = r.Extensions.Txn.Preds
mr.startTs = r.Extensions.Txn.StartTs
sort.Strings(mr.preds)

var d map[string]interface{}
if err := json.Unmarshal(r.Data, &d); err != nil {
Expand Down
97 changes: 41 additions & 56 deletions dgraph/cmd/alpha/upsert_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"context"
"encoding/json"
"fmt"
"sort"
"strings"
"sync"
"testing"
Expand Down Expand Up @@ -77,7 +76,6 @@ upsert {
mr, err := mutationWithTs(m1, "application/rdf", false, true, 0)
require.NoError(t, err)
require.True(t, len(mr.keys) == 0)
sort.Strings(mr.preds)
require.Equal(t, []string{"1-email", "1-name"}, mr.preds)
result := QueryResult{}
require.NoError(t, json.Unmarshal(mr.data, &result))
Expand Down Expand Up @@ -493,7 +491,7 @@ upsert {
m2 := `
upsert {
query {
q(func: eq(name@en, "user1")) {
user1(func: eq(name@en, "user1")) {
u1 as uid
}
}
Expand All @@ -508,7 +506,7 @@ upsert {
require.NoError(t, err)
result = QueryResult{}
require.NoError(t, json.Unmarshal(mr.data, &result))
require.Equal(t, 1, len(result.Q))
require.Equal(t, 1, len(result.User1))

q2 := `
{
Expand Down Expand Up @@ -950,7 +948,6 @@ upsert {
mr, err := mutationWithTs(m1, "application/rdf", false, true, 0)
require.NoError(t, err)
require.True(t, len(mr.keys) == 0)
sort.Strings(mr.preds)
require.Equal(t, []string{"1-email", "1-name"}, mr.preds)
result := QueryResult{}
require.NoError(t, json.Unmarshal(mr.data, &result))
Expand Down Expand Up @@ -1578,7 +1575,8 @@ amount: float .`))
m2 := `
upsert {
query {
u as var(func: has(amount)) {
q(func: has(amount)) {
u as uid
amt as amount
n as name
b as branch
Expand All @@ -1602,15 +1600,17 @@ upsert {
uid(u) <loc> val(l) .
}
}
}
`
}`

// This test is to ensure that all the types are being
// parsed correctly by the val function.
// User3 doesn't have all the fields. This test also ensures
// that val works when not all records have the values
_, err = mutationWithTs(m2, "application/rdf", false, true, 0)
mr, err := mutationWithTs(m2, "application/rdf", false, true, 0)
require.NoError(t, err)
result := QueryResult{}
require.NoError(t, json.Unmarshal(mr.data, &result))
require.Equal(t, 3, len(result.Q))

res, _, err := queryWithTs(q1, "application/graphql+-", "", 0)
require.NoError(t, err)
Expand Down Expand Up @@ -2128,9 +2128,6 @@ upsert {
}`
mr, err := mutationWithTs(m2, "application/rdf", false, true, 0)
require.NoError(t, err)
// The uid variable is only used in the query and not in mutation
// and hence shouldn't be part of vars.

result := QueryResult{}
require.NoError(t, json.Unmarshal(mr.data, &result))
require.Equal(t, 0, len(result.Q))
Expand All @@ -2146,11 +2143,9 @@ func TestMultiMutationEmptyRequest(t *testing.T) {
}))
require.NoError(t, dg.Alter(context.Background(), &api.Operation{
Schema: `
name: string @index(exact) .
branch: string .
amount: float .
`,
}))
name: string @index(exact) .
branch: string .
amount: float .`}))

req := &api.Request{}
_, err = dg.NewTxn().Do(context.Background(), req)
Expand All @@ -2159,42 +2154,35 @@ func TestMultiMutationEmptyRequest(t *testing.T) {

// This mutation (upsert) has one independent query and one independent mutation.
func TestMultiMutationNoUpsert(t *testing.T) {
dg, err := testutil.DgraphClientWithGroot("localhost:9180")
require.NoError(t, err, "error while getting a dgraph client")
require.NoError(t, dropAll())
require.NoError(t, alterSchema(`
email: string @index(exact) .
works_for: string @index(exact) .
works_with: [uid] .`))

require.NoError(t, dg.Alter(context.Background(), &api.Operation{
DropOp: api.Operation_ALL,
}))
require.NoError(t, dg.Alter(context.Background(), &api.Operation{
Schema: `
email: string @index(exact) .
works_for: string @index(exact) .
works_with: [uid] .
`,
}))
m1 := `
upsert {
query {
q(func: eq(works_for, "company1")) {
uid
name
}
}
req := &api.Request{
Query: `
query {
empty(func: eq(works_for, "company1")) {
uid
name
}
}`,
Mutations: []*api.Mutation{
&api.Mutation{
SetNquads: []byte(`
_:user1 <name> "user1" .
_:user1 <email> "user1@company1.io" .
_:user1 <works_for> "company1" .`),
},
},
CommitNow: true,
}
resp, err := dg.NewTxn().Do(context.Background(), req)
mutation {
set {
_:user1 <name> "user1" .
_:user1 <email> "user1@company1.io" .
_:user1 <works_for> "company1" .
}
}
}`
mr, err := mutationWithTs(m1, "application/rdf", false, true, 0)
require.NoError(t, err)
require.Equal(t, []string{"1-email", "1-name", "1-works_for"}, resp.Txn.Preds)
testutil.CompareJSON(t, `{"empty": []}`, string(resp.Json))
result := QueryResult{}
require.NoError(t, json.Unmarshal(mr.data, &result))
require.Equal(t, 0, len(result.Q))
require.Equal(t, []string{"1-email", "1-name", "1-works_for"}, mr.preds)
}

func TestMultipleMutation(t *testing.T) {
Expand Down Expand Up @@ -2226,7 +2214,6 @@ upsert {
mr, err := mutationWithTs(m1, "application/rdf", false, true, 0)
require.NoError(t, err)
require.True(t, len(mr.keys) == 0)
sort.Strings(mr.preds)
require.Equal(t, []string{"1-email", "1-name"}, mr.preds)
result := QueryResult{}
require.NoError(t, json.Unmarshal(mr.data, &result))
Expand All @@ -2243,7 +2230,7 @@ upsert {
{
"data": {
"q": [{
"name": "name"
"name": "name"
}]
}
}`
Expand All @@ -2267,7 +2254,7 @@ upsert {
{
"data": {
"q": [{
"name": "not_name"
"name": "not_name"
}]
}
}`
Expand Down Expand Up @@ -2316,7 +2303,6 @@ upsert {
mr, err := mutationWithTs(m1, "application/rdf", false, true, 0)
require.NoError(t, err)
require.True(t, len(mr.keys) == 0)
sort.Strings(mr.preds)
require.Equal(t, []string{"1-email", "1-name"}, mr.preds)

q1 := `
Expand All @@ -2330,7 +2316,7 @@ upsert {
{
"data": {
"q": [{
"name": "name"
"name": "name"
},
{
"name": "other"
Expand Down Expand Up @@ -2374,7 +2360,6 @@ upsert {
mr, err := mutationWithTs(m1, "application/rdf", false, true, 0)
require.NoError(t, err)
require.True(t, len(mr.keys) == 0)
sort.Strings(mr.preds)
require.Equal(t, []string{"1-count", "1-email", "1-name"}, mr.preds)

q1 := `
Expand Down
34 changes: 16 additions & 18 deletions edgraph/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -462,9 +462,10 @@ type queryContext struct {
condVars []string
// uidRes stores mapping from variable names to UIDs for UID variables.
// These variables are either dummy variables used for Conditional
// Upsert or variables defined in the query in the incoming request.
// Upsert or variables used in the mutation block in the incoming request.
uidRes map[string][]string
// valRes stores mapping from variable names to values for value variables.
// valRes stores mapping from variable names to values for value
// variables used in the mutation block of incoming request.
valRes map[string]map[uint64]types.Val
// l stores latency numbers
latency *query.Latency
Expand All @@ -487,9 +488,6 @@ func (s *Server) doQuery(ctx context.Context, req *api.Request, authorize int) (
l := &query.Latency{}
l.Start = time.Now()

// let's divide the request in two categories:
// 1. query
// 2. mutation (upsert is also a mutation)
isMutation := len(req.Mutations) > 0
methodRequest := methodQuery
if isMutation {
Expand Down Expand Up @@ -521,7 +519,9 @@ func (s *Server) doQuery(ctx context.Context, req *api.Request, authorize int) (
}()

span.Annotatef(nil, "Request received: %v", req)
if len(req.Query) == 0 && len(req.Mutations) == 0 {
req.Query = strings.TrimSpace(req.Query)
isQuery := len(req.Query) != 0
if !isQuery && !isMutation {
span.Annotate(nil, "empty request")
return nil, errors.Errorf("empty request")
}
Expand All @@ -546,16 +546,19 @@ func (s *Server) doQuery(ctx context.Context, req *api.Request, authorize int) (

if qc.req.StartTs == 0 {
start := time.Now()
qc.req.StartTs = State.getTimestamp(qc.req.ReadOnly)
if qc.req.BestEffort {
qc.req.StartTs = posting.Oracle().MaxAssigned()
} else {
qc.req.StartTs = State.getTimestamp(qc.req.ReadOnly)
}
qc.latency.AssignTimestamp = time.Since(start)
}

if qc.req.Query != "" {
resp = &api.Response{}
if isQuery {
if resp, rerr = processQuery(ctx, qc); rerr != nil {
return
}
} else {
resp = &api.Response{}
}

if isMutation {
Expand Down Expand Up @@ -599,7 +602,7 @@ func parseRequest(qc *queryContext) error {
// updating queries to include dummy variables for conditional upsert
upsertQuery := buildUpsertQuery(qc)
needVars := findVars(qc)
if upsertQuery == "" {
if len(upsertQuery) == 0 {
if len(needVars) > 0 {
return errors.Errorf("variables %v not defined", needVars)
}
Expand All @@ -626,8 +629,7 @@ func parseRequest(qc *queryContext) error {
// buildUpsertQuery modifies the query to evaluate the
// @if condition defined in Conditional Upsert.
func buildUpsertQuery(qc *queryContext) string {
qc.req.Query = strings.TrimSpace(qc.req.Query)
if qc.req.Query == "" {
if len(qc.req.Query) == 0 {
return qc.req.Query
}

Expand Down Expand Up @@ -704,10 +706,6 @@ func processQuery(ctx context.Context, qc *queryContext) (*api.Response, error)
return nil, errors.Errorf("best effort query must be read-only")
}

if qc.req.StartTs == 0 {
qc.req.StartTs = posting.Oracle().MaxAssigned()
}

qr.Cache = worker.NoCache
}

Expand Down Expand Up @@ -780,7 +778,7 @@ func processQuery(ctx context.Context, qc *queryContext) (*api.Response, error)
func updateMutations(qc *queryContext) {
for i, gmu := range qc.gmuList {
condVar := qc.condVars[i]
if condVar != "" {
if len(condVar) != 0 {
uids, ok := qc.uidRes[condVar]
if !(ok && len(uids) == 1) {
gmu.Set = nil
Expand Down

0 comments on commit 24819a8

Please sign in to comment.