Skip to content

Commit

Permalink
Remove unnecessary graph instantiations. The planner resolved repeate…
Browse files Browse the repository at this point in the history
…dly unnecessary graph instances that could be just fetched and cached.
  • Loading branch information
xllora committed Jan 6, 2017
1 parent 9014290 commit c92007e
Show file tree
Hide file tree
Showing 6 changed files with 84 additions and 25 deletions.
2 changes: 1 addition & 1 deletion bql/grammar/grammar_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ func TestAcceptOpsByParseAndSemantic(t *testing.T) {
if err := p.Parse(NewLLk(entry.query, 1), st); err != nil {
t.Errorf("Parser.consume: failed to accept entry %q with error %v", entry, err)
}
if got, want := len(st.Graphs()), entry.graphs; got != want {
if got, want := len(st.GraphNames()), entry.graphs; got != want {
t.Errorf("Parser.consume: failed to collect right number of graphs for case %v; got %d, want %d", entry, got, want)
}
if got, want := len(st.Data()), entry.triples; got != want {
Expand Down
31 changes: 12 additions & 19 deletions bql/planner/planner.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func (p *createPlan) Execute(ctx context.Context) (*table.Table, error) {
return nil, err
}
errs := []string{}
for _, g := range p.stm.Graphs() {
for _, g := range p.stm.GraphNames() {
if _, err := p.store.NewGraph(ctx, g); err != nil {
errs = append(errs, err.Error())
}
Expand Down Expand Up @@ -87,7 +87,7 @@ func (p *dropPlan) Execute(ctx context.Context) (*table.Table, error) {
return nil, err
}
errs := []string{}
for _, g := range p.stm.Graphs() {
for _, g := range p.stm.GraphNames() {
if err := p.store.DeleteGraph(ctx, g); err != nil {
errs = append(errs, err.Error())
}
Expand Down Expand Up @@ -124,7 +124,7 @@ func update(ctx context.Context, stm *semantic.Statement, store storage.Store, f
errs = append(errs, err.Error())
}

for _, graphBinding := range stm.Graphs() {
for _, graphBinding := range stm.GraphNames() {
wg.Add(1)
go func(graph string) {
defer wg.Done()
Expand Down Expand Up @@ -230,15 +230,20 @@ func newQueryPlan(ctx context.Context, store storage.Store, stm *semantic.Statem
if err != nil {
return nil, err
}
return &queryPlan{
qp := &queryPlan{
stm: stm,
store: store,
bndgs: bs,
grfsNames: stm.Graphs(),
grfsNames: stm.GraphNames(),
cls: stm.SortedGraphPatternClauses(),
tbl: t,
chanSize: chanSize,
}, nil
}
if err := stm.Init(ctx, store); err != nil {
return nil, err
}
qp.grfs = stm.Graphs()
return qp, nil
}

// processClause retrieves the triples for the provided triple given the
Expand Down Expand Up @@ -480,11 +485,7 @@ func (p *queryPlan) filterOnExistence(ctx context.Context, cls *semantic.GraphCl
if err != nil {
return err
}
gph, err := p.store.Graph(ctx, g)
if err != nil {
return err
}
b, err := gph.Exist(ctx, t)
b, err := g.Exist(ctx, t)
if err != nil {
return err
}
Expand Down Expand Up @@ -625,14 +626,6 @@ func (p *queryPlan) limit() {

// Execute queries the indicated graphs.
func (p *queryPlan) Execute(ctx context.Context) (*table.Table, error) {
// Collect the graph references.
for _, g := range p.stm.Graphs() {
ng, err := p.store.Graph(ctx, g)
if err != nil {
return nil, err
}
p.grfs = append(p.grfs, ng)
}
// Retrieve the data.
lo := p.stm.GlobalLookupOptions()
if err := p.processGraphPattern(ctx, lo); err != nil {
Expand Down
47 changes: 47 additions & 0 deletions bql/planner/planner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,53 @@ func TestTreeTraversalToRoot(t *testing.T) {
}
}

func TestChaining(t *testing.T) {
// Graph traversal data.
traversalTriples := `/u<joe> "parent_of"@[] /u<mary>
/u<joe> "parent_of"@[] /u<peter>
/u<peter> "parent_of"@[] /u<john>
/u<peter> "parent_of"@[] /u<eve>`

traversalQuery := `SELECT ?o FROM ?test
WHERE {
/u<joe> "parent_of"@[] ?o .
?o "parent_of"@[] /u<john>
};`

// Load traversing data
s, ctx := memory.NewStore(), context.Background()
g, gErr := s.NewGraph(ctx, "?test")
if gErr != nil {
t.Fatalf("memory.NewGraph failed to create \"?test\" with error %v", gErr)
}
b := bytes.NewBufferString(traversalTriples)
if _, err := io.ReadIntoGraph(ctx, g, b, literal.DefaultBuilder()); err != nil {
t.Fatalf("io.ReadIntoGraph failed to read test graph with error %v", err)
}
p, pErr := grammar.NewParser(grammar.SemanticBQL())
if pErr != nil {
t.Fatalf("grammar.NewParser: should have produced a valid BQL parser with error %v", pErr)
}
st := &semantic.Statement{}
if err := p.Parse(grammar.NewLLk(traversalQuery, 1), st); err != nil {
t.Errorf("Parser.consume: failed to parse query %q with error %v", traversalQuery, err)
}
plnr, err := New(ctx, s, st, 0)
if err != nil {
t.Errorf("planner.New failed to create a valid query plan with error %v", err)
}
tbl, err := plnr.Execute(ctx)
if err != nil {
t.Errorf("planner.Excecute failed for query %q with error %v", traversalQuery, err)
}
if got, want := len(tbl.Bindings()), 1; got != want {
t.Errorf("tbl.Bindings returned the wrong number of bindings for %q; got %d, want %d", traversalQuery, got, want)
}
if got, want := len(tbl.Rows()), 1; got != want {
t.Errorf("planner.Excecute failed to return the expected number of rows for query %q; got %d want %d\nGot:\n%v\n", traversalQuery, got, want, tbl)
}
}

// benchmarkQuery is a helper function that runs a specified query on the testing data set for benchmarking purposes.
func benchmarkQuery(query string, b *testing.B) {
ctx := context.Background()
Expand Down
2 changes: 1 addition & 1 deletion bql/semantic/hooks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ func TestSemanticAcceptInsertDelete(t *testing.T) {
t.Errorf("semantic.GraphAccumulator hook should have never failed for %v with error %v", ce, err)
}
}
data := st.Graphs()
data := st.GraphNames()
if len(data) != 2 {
t.Errorf("semantic.GraphAccumulator hook should have produced 2 graph bindings; instead produced %v", st.Graphs())
}
Expand Down
25 changes: 22 additions & 3 deletions bql/semantic/semantic.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package semantic

import (
"bytes"
"context"
"reflect"
"sort"
"time"
Expand Down Expand Up @@ -69,7 +70,8 @@ func (t StatementType) String() string {
// Statement contains all the semantic information extract from the parsing
type Statement struct {
sType StatementType
graphs []string
graphNames []string
graphs []storage.Graph
data []*triple.Triple
pattern []*GraphClause
workingClause *GraphClause
Expand Down Expand Up @@ -347,14 +349,31 @@ func (s *Statement) Type() StatementType {

// AddGraph adds a graph to a given https://critique.corp.google.com/#review/101398527statement.
func (s *Statement) AddGraph(g string) {
s.graphs = append(s.graphs, g)
s.graphNames = append(s.graphNames, g)
}

// Graphs returns the list of graphs listed on the statement.
func (s *Statement) Graphs() []string {
func (s *Statement) Graphs() []storage.Graph {
return s.graphs
}

// Init initialize the graphs givne the graph names.
func (s *Statement) Init(ctx context.Context, st storage.Store) error {
for _, gn := range s.graphNames {
g, err := st.Graph(ctx, gn)
if err != nil {
return err
}
s.graphs = append(s.graphs, g)
}
return nil
}

// GraphNames returns the list of graphs listed on the statement.
func (s *Statement) GraphNames() []string {
return s.graphNames
}

// AddData adds a triple to a given statement's data.
func (s *Statement) AddData(d *triple.Triple) {
s.data = append(s.data, d)
Expand Down
2 changes: 1 addition & 1 deletion bql/semantic/semantic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func TestStatementAddGraph(t *testing.T) {
st := &Statement{}
st.BindType(Query)
st.AddGraph("?foo")
if got, want := st.Graphs(), []string{"?foo"}; !reflect.DeepEqual(got, want) {
if got, want := st.GraphNames(), []string{"?foo"}; !reflect.DeepEqual(got, want) {
t.Errorf("semantic.AddGraph returned the wrong graphs avaiable; got %v, want %v", got, want)
}
}
Expand Down

0 comments on commit c92007e

Please sign in to comment.