From c2e905ed1b76ee0d77e0dce3c97e6fb7f24ebda3 Mon Sep 17 00:00:00 2001 From: Shahzad Lone Date: Thu, 19 May 2022 07:16:08 -0400 Subject: [PATCH] wip: Adhere to the review suggestions #1. --- Makefile | 2 +- cli/defradb/examples/address.graphql | 6 ++-- cli/defradb/examples/bookauthpub.graphql | 20 +++++++++++ cli/defradb/examples/booksmany.graphql | 3 +- cli/defradb/examples/booksone.graphql | 4 +-- cli/defradb/examples/user.graphql | 8 ++--- query/graphql/planner/explain.go | 2 +- query/graphql/planner/planner.go | 44 +++++++++++------------- 8 files changed, 52 insertions(+), 37 deletions(-) create mode 100644 cli/defradb/examples/bookauthpub.graphql diff --git a/Makefile b/Makefile index d2893e5d5e..e60ee7ba3c 100644 --- a/Makefile +++ b/Makefile @@ -30,7 +30,7 @@ client\:dump: .PHONY: client\:add-schema client\:add-schema: - ./build/defradb client schema add -f cli/defradb/examples/user.graphql + ./build/defradb client schema add -f cli/defradb/examples/bookauthpub.graphql .PHONY: deps\:lint deps\:lint: diff --git a/cli/defradb/examples/address.graphql b/cli/defradb/examples/address.graphql index ddff423b58..096e7414ba 100644 --- a/cli/defradb/examples/address.graphql +++ b/cli/defradb/examples/address.graphql @@ -1,6 +1,6 @@ type address { - street: String - number: Int - city: String + street: String + number: Int + city: String country: String } diff --git a/cli/defradb/examples/bookauthpub.graphql b/cli/defradb/examples/bookauthpub.graphql new file mode 100644 index 0000000000..6f4eaaa624 --- /dev/null +++ b/cli/defradb/examples/bookauthpub.graphql @@ -0,0 +1,20 @@ +type book { + name: String + rating: Float + author: author + publisher: publisher +} + +type author { + name: String + age: Int + verified: Boolean + wrote: book @primary +} + +type publisher { + name: String + address: String + favouritePageNumbers: [Int] + published: [book] +} diff --git a/cli/defradb/examples/booksmany.graphql b/cli/defradb/examples/booksmany.graphql index 391653a885..c638aede1f 100644 --- a/cli/defradb/examples/booksmany.graphql +++ b/cli/defradb/examples/booksmany.graphql @@ -1,4 +1,3 @@ - type book { name: String rating: Float @@ -10,4 +9,4 @@ type author { age: Int verified: Boolean published: [book] -} \ No newline at end of file +} diff --git a/cli/defradb/examples/booksone.graphql b/cli/defradb/examples/booksone.graphql index 4462ed5c08..c63a981d3b 100644 --- a/cli/defradb/examples/booksone.graphql +++ b/cli/defradb/examples/booksone.graphql @@ -8,5 +8,5 @@ type author { name: String age: Int verified: Boolean - published: book -} \ No newline at end of file + published: book +} diff --git a/cli/defradb/examples/user.graphql b/cli/defradb/examples/user.graphql index 5b3c1b6dca..3cb0cbaa42 100644 --- a/cli/defradb/examples/user.graphql +++ b/cli/defradb/examples/user.graphql @@ -1,6 +1,6 @@ type user { - name: String - age: Int - verified: Boolean - points: Float + name: String + age: Int + verified: Boolean + points: Float } diff --git a/query/graphql/planner/explain.go b/query/graphql/planner/explain.go index 502bf2a499..5a524261f3 100644 --- a/query/graphql/planner/explain.go +++ b/query/graphql/planner/explain.go @@ -51,7 +51,7 @@ func styleAttribute(attributeName string) string { func buildExplainGraph(source planNode) map[string]interface{} { - var explainGraph map[string]interface{} = map[string]interface{}{} + explainGraph := map[string]interface{}{} if source == nil { return explainGraph diff --git a/query/graphql/planner/planner.go b/query/graphql/planner/planner.go index 29742f97b3..d7abb22865 100644 --- a/query/graphql/planner/planner.go +++ b/query/graphql/planner/planner.go @@ -103,12 +103,12 @@ func (p *Planner) newPlan(stmt parser.Statement) (planNode, error) { } else if len(n.Mutations) > 0 { return p.newPlan(n.Mutations[0]) // @todo: handle multiple mutation statements } else { - return nil, fmt.Errorf("Error: Query is missing query or mutation statements") + return nil, fmt.Errorf("Query is missing query or mutation statements") } case *parser.OperationDefinition: if len(n.Selections) == 0 { - return nil, fmt.Errorf("Error: OperationDefinition is missing selections") + return nil, fmt.Errorf("OperationDefinition is missing selections") } return p.newPlan(n.Selections[0]) @@ -122,7 +122,7 @@ func (p *Planner) newPlan(stmt parser.Statement) (planNode, error) { return p.newObjectMutationPlan(n) } - return nil, fmt.Errorf("Error: Unknown statement type %T", stmt) + return nil, fmt.Errorf("Unknown statement type %T", stmt) } func (p *Planner) newObjectMutationPlan(stmt *parser.Mutation) (planNode, error) { @@ -139,7 +139,7 @@ func (p *Planner) newObjectMutationPlan(stmt *parser.Mutation) (planNode, error) return p.DeleteDocs(stmt) default: - return nil, fmt.Errorf("Error: Unknown mutation action %T", stmt.Type) + return nil, fmt.Errorf("Unknown mutation action %T", stmt.Type) } } @@ -275,7 +275,7 @@ func (p *Planner) expandTypeIndexJoinPlan(plan *typeIndexJoin, parentPlan *selec case *typeJoinMany: return p.expandPlan(node.subType, parentPlan) } - return fmt.Errorf("Error: Unknown type index join plan") + return fmt.Errorf("Unknown type index join plan") } func (p *Planner) expandGroupNodePlan(plan *selectTopNode) error { @@ -400,7 +400,7 @@ func (p *Planner) walkAndReplacePlan(plan, target, replace planNode) error { /* Do nothing - pipe nodes should not be replaced */ // @todo: add more nodes that apply here default: - return fmt.Errorf("Error: Unknown plan node type to replace: %T", node) + return fmt.Errorf("Unknown plan node type to replace: %T", node) } return nil @@ -430,20 +430,16 @@ func (p *Planner) explainRequest( plan planNode, ) ([]map[string]interface{}, error) { if plan == nil { - if errToLog := (plan.Close()); errToLog != nil { - log.ErrorE(ctx, "Error: Failure closing plan while explaining `planNode`.", errToLog) - } - return nil, fmt.Errorf("Error: Can't explain an empty plan.") + return nil, fmt.Errorf("Can't explain an empty / nil plan.") } - var topExplainGraph []map[string]interface{} = []map[string]interface{}{ + topExplainGraph := []map[string]interface{}{ { parser.DirectiveLabel.ExplainLabel: buildExplainGraph(plan), }, } - err := plan.Close() - return topExplainGraph, err + return topExplainGraph, plan.Close() } // executeRequest executes the plan graph that represents the request that was made. @@ -453,8 +449,8 @@ func (p *Planner) executeRequest( ) ([]map[string]interface{}, error) { if err := plan.Start(); err != nil { - if loggingErr := (plan.Close()); loggingErr != nil { - log.ErrorE(ctx, "Error: Failure closing plan after `plan.Start()` got an error.", loggingErr) + if err := plan.Close(); err != nil { + log.ErrorE(ctx, "Failure closing plan after `plan.Start()` got an error.", err) } return nil, err } @@ -462,13 +458,13 @@ func (p *Planner) executeRequest( var next bool var err error if next, err = plan.Next(); err != nil { - if loggingErr := (plan.Close()); loggingErr != nil { - log.ErrorE(ctx, "Error: Failure closing plan after initial `plan.Next()` call.", loggingErr) + if err := plan.Close(); err != nil { + log.ErrorE(ctx, "Failure closing plan after initial `plan.Next()` call.", err) } return nil, err } - var docs []map[string]interface{} = []map[string]interface{}{} + docs := []map[string]interface{}{} for next { if values := plan.Value(); values != nil { @@ -477,15 +473,15 @@ func (p *Planner) executeRequest( next, err = plan.Next() if err != nil { - if loggingErr := (plan.Close()); loggingErr != nil { - log.ErrorE(ctx, "Error: Failure closing plan after `plan.Next()` got an error.", loggingErr) + if err := plan.Close(); err != nil { + log.ErrorE(ctx, "Failure closing plan after `plan.Next()` got an error.", err) } return nil, err } } - if loggingErr := (plan.Close()); loggingErr != nil { - log.ErrorE(ctx, "Error: Failure closing plan near the end of plan execution.", loggingErr) + if err := plan.Close(); err != nil { + log.ErrorE(ctx, "Failure closing plan near the end of plan execution.", err) } return docs, err @@ -503,11 +499,11 @@ func (p *Planner) runRequest( return nil, err } - thisIsAnExplainRequest := + isAnExplainRequest := (len(query.Queries) > 0 && query.Queries[0].IsExplain) || (len(query.Mutations) > 0 && query.Mutations[0].IsExplain) - if thisIsAnExplainRequest { + if isAnExplainRequest { return p.explainRequest(ctx, plan) }