Skip to content

Commit

Permalink
wip: Adhere to the review suggestions #1.
Browse files Browse the repository at this point in the history
  • Loading branch information
shahzadlone committed May 25, 2022
1 parent 5687fe5 commit c2e905e
Show file tree
Hide file tree
Showing 8 changed files with 52 additions and 37 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
6 changes: 3 additions & 3 deletions cli/defradb/examples/address.graphql
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
type address {
street: String
number: Int
city: String
street: String
number: Int
city: String
country: String
}
20 changes: 20 additions & 0 deletions cli/defradb/examples/bookauthpub.graphql
Original file line number Diff line number Diff line change
@@ -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]
}
3 changes: 1 addition & 2 deletions cli/defradb/examples/booksmany.graphql
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

type book {
name: String
rating: Float
Expand All @@ -10,4 +9,4 @@ type author {
age: Int
verified: Boolean
published: [book]
}
}
4 changes: 2 additions & 2 deletions cli/defradb/examples/booksone.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@ type author {
name: String
age: Int
verified: Boolean
published: book
}
published: book
}
8 changes: 4 additions & 4 deletions cli/defradb/examples/user.graphql
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
type user {
name: String
age: Int
verified: Boolean
points: Float
name: String
age: Int
verified: Boolean
points: Float
}
2 changes: 1 addition & 1 deletion query/graphql/planner/explain.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
44 changes: 20 additions & 24 deletions query/graphql/planner/planner.go
Original file line number Diff line number Diff line change
Expand Up @@ -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])

Expand All @@ -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) {
Expand All @@ -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)
}

}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -453,22 +449,22 @@ 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
}

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 {
Expand All @@ -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
Expand All @@ -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)
}

Expand Down

0 comments on commit c2e905e

Please sign in to comment.