Fix filter that was still adding empty clauses. This led the query pl…

…anner to do more work that really needed.
google · Jan 10, 2017 · 11266c7 · 11266c7
1 parent a522b77
commit 11266c7
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 4 deletions.
diff --git a/bql/grammar/grammar_test.go b/bql/grammar/grammar_test.go
@@ -343,3 +343,36 @@ func TestRejectByParseAndSemantic(t *testing.T) {
 		}
 	}
 }
+
+func TestSemanticStatementGraphClausesLenghtCorrectness(t *testing.T) {
+	table := []struct {
+		query string
+		want  int
+	}{
+		{
+			query: `SELECT ?o,?l FROM ?bbacl WHERE { ?o "some_id"@[,] ?l } LIMIT "20"^^type:int64;`,
+			want:  1,
+		},
+		{
+			query: `SELECT ?o,?l FROM ?bbacl WHERE { ?o "some_id"@[,] ?x . ?x "some_id"@[,] ?l } LIMIT "20"^^type:int64;`,
+			want:  2,
+		},
+		{
+			query: `SELECT ?o,?l FROM ?bbacl WHERE { ?o "some_id"@[,] ?x . ?x "some_id"@[,] ?y . ?y "some_id"@[,] ?l } LIMIT "20"^^type:int64;`,
+			want:  3,
+		},
+	}
+	p, err := NewParser(SemanticBQL())
+	if err != nil {
+		t.Errorf("grammar.NewParser: should have produced a valid BQL parser, %v", err)
+	}
+	for _, entry := range table {
+		st := &semantic.Statement{}
+		if err := p.Parse(NewLLk(entry.query, 1), st); err != nil {
+			t.Errorf("Parser.consume: failed to accept valid semantic entry %q", entry.query)
+		}
+		if got, want := len(st.GraphPatternClauses()), entry.want; got != want {
+			t.Errorf("Invalid number of graph pattern clauses for query %q; got %d, want %d; %v", entry.query, got, want, st.GraphPatternClauses())
+		}
+	}
+}
diff --git a/bql/semantic/hooks_test.go b/bql/semantic/hooks_test.go
@@ -155,8 +155,8 @@ func TestWhereWorkingClauseHook(t *testing.T) {
 	st.ResetWorkingGraphClause()
 	f(st, Symbol("FOO"))
 	f(st, Symbol("FOO"))
-	if len(st.GraphPatternClauses()) != 2 {
-		t.Errorf("semantic.whereNextWorkingClause should have returned two clauses for statement %v", st)
+	if got, want := len(st.GraphPatternClauses()), 0; got != want {
+		t.Errorf("semantic.whereNextWorkingClause should have returned two clauses for statement %v; got %d, want %d", st, got, want)
 	}
 }
 

diff --git a/bql/semantic/semantic.go b/bql/semantic/semantic.go
@@ -402,7 +402,7 @@ func (s *Statement) WorkingClause() *GraphClause {
 // AddWorkingGraphClause adds the current working graph clause to the set of
 // clauses that form the graph pattern.
 func (s *Statement) AddWorkingGraphClause() {
-	if s.workingClause != nil || !s.workingClause.IsEmpty() {
+	if s.workingClause != nil && !s.workingClause.IsEmpty() {
 		s.pattern = append(s.pattern, s.workingClause)
 	}
 	s.ResetWorkingGraphClause()

diff --git a/bql/semantic/semantic_test.go b/bql/semantic/semantic_test.go
@@ -81,7 +81,7 @@ func TestGraphClauseManipulation(t *testing.T) {
 		t.Fatalf("semantic.GraphClause.WorkingClause should return a working clause after initilization in %v", st)
 	}
 	st.AddWorkingGraphClause()
-	if got, want := len(st.GraphPatternClauses()), 1; got != want {
+	if got, want := len(st.GraphPatternClauses()), 0; got != want {
 		t.Fatalf("semantic.GraphClause.Clauses return wrong number of clauses in %v; got %d, want %d", st, got, want)
 	}
 }