diff --git a/dgraph/cmd/alpha/http_test.go b/dgraph/cmd/alpha/http_test.go index 8758df02d44..24dda9ca4c5 100644 --- a/dgraph/cmd/alpha/http_test.go +++ b/dgraph/cmd/alpha/http_test.go @@ -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 { diff --git a/dgraph/cmd/alpha/upsert_test.go b/dgraph/cmd/alpha/upsert_test.go index e44f62261c0..55c3d2ba112 100644 --- a/dgraph/cmd/alpha/upsert_test.go +++ b/dgraph/cmd/alpha/upsert_test.go @@ -20,7 +20,6 @@ import ( "context" "encoding/json" "fmt" - "sort" "strings" "sync" "testing" @@ -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)) @@ -493,7 +491,7 @@ upsert { m2 := ` upsert { query { - q(func: eq(name@en, "user1")) { + user1(func: eq(name@en, "user1")) { u1 as uid } } @@ -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 := ` { @@ -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)) @@ -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 @@ -1602,15 +1600,17 @@ upsert { uid(u) 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) @@ -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)) @@ -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) @@ -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 "user1" . - _:user1 "user1@company1.io" . - _:user1 "company1" .`), - }, - }, - CommitNow: true, - } - resp, err := dg.NewTxn().Do(context.Background(), req) + mutation { + set { + _:user1 "user1" . + _:user1 "user1@company1.io" . + _:user1 "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) { @@ -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)) @@ -2243,7 +2230,7 @@ upsert { { "data": { "q": [{ - "name": "name" + "name": "name" }] } }` @@ -2267,7 +2254,7 @@ upsert { { "data": { "q": [{ - "name": "not_name" + "name": "not_name" }] } }` @@ -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 := ` @@ -2330,7 +2316,7 @@ upsert { { "data": { "q": [{ - "name": "name" + "name": "name" }, { "name": "other" @@ -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 := ` diff --git a/edgraph/server.go b/edgraph/server.go index 0d05054c24f..dfa074a098a 100644 --- a/edgraph/server.go +++ b/edgraph/server.go @@ -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 @@ -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 { @@ -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") } @@ -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 { @@ -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) } @@ -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 } @@ -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 } @@ -776,7 +774,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