Repository: vitessio/vitess
Pull request #5550 (2019-12-12 01:06:56)

Updating parser to handle more valid types of begin statements
Currently we don't accept `BEGIN;` which isn't something we normally get set, but it is within spec. This allows you to handle those. 

Signed-off-by: Dan Kozlowski <koz@planetscale.com>


Patch:
diff --git a/.dockerignore b/.dockerignore
index 58354ff421..5b123fd019 100644
--- a/.dockerignore
+++ b/.dockerignore
@@ -5,8 +5,3 @@ java/*/target
 java/*/bin
 php/vendor
 releases
-/dist/
-/py-vtdb/
-/vthook/
-/bin/
-/vtdataroot/
diff --git a/.gitignore b/.gitignore
index be8fe72756..2561bdd937 100644
--- a/.gitignore
+++ b/.gitignore
@@ -80,8 +80,9 @@ releases
 # Vagrant
 .vagrant
 
-/dist/
-/py-vtdb
-/vthook/
-/bin/
-/vtdataroot/
+dist/*
+py-vtdb*
+vthook*
+bin*
+
+vtdataroot*
diff --git a/go/vt/servenv/status.go b/go/vt/servenv/status.go
index c9fdbfb3fd..f391bf470a 100644
--- a/go/vt/servenv/status.go
+++ b/go/vt/servenv/status.go
@@ -104,7 +104,6 @@ Started: {{.StartTime}}<br>
 Running on {{.Hostname}}<br>
 View <a href=/debug/vars>variables</a>,
      <a href=/debug/pprof>debugging profiles</a>,
-     <a href=/debug/scatter_stats>scatter query statistics</a>,
 </div>
 </div>`
 
diff --git a/go/vt/sqlparser/comments.go b/go/vt/sqlparser/comments.go
index 26c664cf91..afffd326c8 100644
--- a/go/vt/sqlparser/comments.go
+++ b/go/vt/sqlparser/comments.go
@@ -123,7 +123,9 @@ func SplitMarginComments(sql string) (query string, comments MarginComments) {
 		Leading:  strings.TrimLeftFunc(sql[:leadingEnd], unicode.IsSpace),
 		Trailing: strings.TrimRightFunc(sql[trailingStart:], unicode.IsSpace),
 	}
-	return strings.TrimFunc(sql[leadingEnd:trailingStart], unicode.IsSpace), comments
+	return strings.TrimFunc(sql[leadingEnd:trailingStart], func(c rune) bool {
+		return unicode.IsSpace(c) || c == ';'
+	}), comments
 }
 
 // StripLeadingComments trims the SQL string and removes any leading comments
diff --git a/go/vt/vtgate/engine/delete.go b/go/vt/vtgate/engine/delete.go
index b0f06174c6..22e2fd61a5 100644
--- a/go/vt/vtgate/engine/delete.go
+++ b/go/vt/vtgate/engine/delete.go
@@ -67,9 +67,6 @@ type Delete struct {
 
 	// QueryTimeout contains the optional timeout (in milliseconds) to apply to this query
 	QueryTimeout int
-
-	// Delete does not take inputs
-	noInputs
 }
 
 // MarshalJSON serializes the Delete into a JSON representation.
diff --git a/go/vt/vtgate/engine/insert.go b/go/vt/vtgate/engine/insert.go
index ec2a54f106..bf75e6a446 100644
--- a/go/vt/vtgate/engine/insert.go
+++ b/go/vt/vtgate/engine/insert.go
@@ -80,9 +80,6 @@ type Insert struct {
 
 	// QueryTimeout contains the optional timeout (in milliseconds) to apply to this query
 	QueryTimeout int
-
-	// Insert does not take inputs
-	noInputs
 }
 
 // NewQueryInsert creates an Insert with a query string.
diff --git a/go/vt/vtgate/engine/join.go b/go/vt/vtgate/engine/join.go
index e577986605..3cfb25e7a1 100644
--- a/go/vt/vtgate/engine/join.go
+++ b/go/vt/vtgate/engine/join.go
@@ -171,11 +171,6 @@ func (jn *Join) GetFields(vcursor VCursor, bindVars map[string]*querypb.BindVari
 	return result, nil
 }
 
-// Inputs returns the input primitives for this join
-func (jn *Join) Inputs() []Primitive {
-	return []Primitive{jn.Left, jn.Right}
-}
-
 func joinFields(lfields, rfields []*querypb.Field, cols []int) []*querypb.Field {
 	fields := make([]*querypb.Field, len(cols))
 	for i, index := range cols {
diff --git a/go/vt/vtgate/engine/limit.go b/go/vt/vtgate/engine/limit.go
index 410a5af624..65b6b39da3 100644
--- a/go/vt/vtgate/engine/limit.go
+++ b/go/vt/vtgate/engine/limit.go
@@ -161,11 +161,6 @@ func (l *Limit) GetFields(vcursor VCursor, bindVars map[string]*querypb.BindVari
 	return l.Input.GetFields(vcursor, bindVars)
 }
 
-// Inputs returns the input to limit
-func (l *Limit) Inputs() []Primitive {
-	return []Primitive{l.Input}
-}
-
 func (l *Limit) fetchCount(bindVars map[string]*querypb.BindVariable) (int, error) {
 	resolved, err := l.Count.ResolveValue(bindVars)
 	if err != nil {
diff --git a/go/vt/vtgate/engine/memory_sort.go b/go/vt/vtgate/engine/memory_sort.go
index f426eb2039..b9832b34e5 100644
--- a/go/vt/vtgate/engine/memory_sort.go
+++ b/go/vt/vtgate/engine/memory_sort.go
@@ -78,7 +78,7 @@ func (ms *MemorySort) SetTruncateColumnCount(count int) {
 	ms.TruncateColumnCount = count
 }
 
-// Execute satisfies the Primitive interface.
+// Execute satisfies the Primtive interface.
 func (ms *MemorySort) Execute(vcursor VCursor, bindVars map[string]*querypb.BindVariable, wantfields bool) (*sqltypes.Result, error) {
 	count, err := ms.fetchCount(bindVars)
 	if err != nil {
@@ -105,7 +105,7 @@ func (ms *MemorySort) Execute(vcursor VCursor, bindVars map[string]*querypb.Bind
 	return result.Truncate(ms.TruncateColumnCount), nil
 }
 
-// StreamExecute satisfies the Primitive interface.
+// StreamExecute satisfies the Primtive interface.
 func (ms *MemorySort) StreamExecute(vcursor VCursor, bindVars map[string]*querypb.BindVariable, wantfields bool, callback func(*sqltypes.Result) error) error {
 	count, err := ms.fetchCount(bindVars)
 	if err != nil {
@@ -155,16 +155,11 @@ func (ms *MemorySort) StreamExecute(vcursor VCursor, bindVars map[string]*queryp
 	return cb(&sqltypes.Result{Rows: sh.rows})
 }
 
-// GetFields satisfies the Primitive interface.
+// GetFields satisfies the Primtive interface.
 func (ms *MemorySort) GetFields(vcursor VCursor, bindVars map[string]*querypb.BindVariable) (*sqltypes.Result, error) {
 	return ms.Input.GetFields(vcursor, bindVars)
 }
 
-// Inputs returns the input to memory sort
-func (ms *MemorySort) Inputs() []Primitive {
-	return []Primitive{ms.Input}
-}
-
 func (ms *MemorySort) fetchCount(bindVars map[string]*querypb.BindVariable) (int, error) {
 	resolved, err := ms.UpperLimit.ResolveValue(bindVars)
 	if err != nil {
diff --git a/go/vt/vtgate/engine/ordered_aggregate.go b/go/vt/vtgate/engine/ordered_aggregate.go
index 9ec98b35d2..9abe441921 100644
--- a/go/vt/vtgate/engine/ordered_aggregate.go
+++ b/go/vt/vtgate/engine/ordered_aggregate.go
@@ -309,11 +309,6 @@ func (oa *OrderedAggregate) GetFields(vcursor VCursor, bindVars map[string]*quer
 	return qr.Truncate(oa.TruncateColumnCount), nil
 }
 
-// Inputs returns the Primitive input for this aggregation
-func (oa *OrderedAggregate) Inputs() []Primitive {
-	return []Primitive{oa.Input}
-}
-
 func (oa *OrderedAggregate) keysEqual(row1, row2 []sqltypes.Value) (bool, error) {
 	for _, key := range oa.Keys {
 		cmp, err := sqltypes.NullsafeCompare(row1[key], row2[key])
diff --git a/go/vt/vtgate/engine/primitive.go b/go/vt/vtgate/engine/primitive.go
index b69d6ea373..cb9c916118 100644
--- a/go/vt/vtgate/engine/primitive.go
+++ b/go/vt/vtgate/engine/primitive.go
@@ -119,28 +119,6 @@ func (p *Plan) Stats() (execCount uint64, execTime time.Duration, shardQueries,
 	return
 }
 
-// Match is used to check if a Primitive matches
-type Match func(node Primitive) bool
-
-// Find will return the first Primitive that matches the evaluate function. If no match is found, nil will be returned
-func Find(isMatch Match, start Primitive) Primitive {
-	if isMatch(start) {
-		return start
-	}
-	for _, input := range start.Inputs() {
-		result := Find(isMatch, input)
-		if result != nil {
-			return result
-		}
-	}
-	return nil
-}
-
-// Exists traverses recursively down the Primitive tree structure, and returns true when Match returns true
-func Exists(m Match, p Primitive) bool {
-	return Find(m, p) != nil
-}
-
 // Size is defined so that Plan can be given to a cache.LRUCache.
 // VTGate needs to maintain a cache of plans. It uses LRUCache, which
 // in turn requires its objects to define a Size function.
@@ -148,10 +126,8 @@ func (p *Plan) Size() int {
 	return 1
 }
 
-// Primitive is the building block of the engine execution plan. They form a tree structure, where the leaves typically
-// issue queries to one or more vttablet.
-// During execution, the Primitive's pass Result objects up the tree structure, until reaching the root,
-// and its result is passed to the client.
+// Primitive is the interface that needs to be satisfied by
+// all primitives of a plan.
 type Primitive interface {
 	RouteType() string
 	GetKeyspaceName() string
@@ -159,14 +135,4 @@ type Primitive interface {
 	Execute(vcursor VCursor, bindVars map[string]*querypb.BindVariable, wantfields bool) (*sqltypes.Result, error)
 	StreamExecute(vcursor VCursor, bindVars map[string]*querypb.BindVariable, wantields bool, callback func(*sqltypes.Result) error) error
 	GetFields(vcursor VCursor, bindVars map[string]*querypb.BindVariable) (*sqltypes.Result, error)
-
-	// The inputs to this Primitive
-	Inputs() []Primitive
-}
-
-type noInputs struct{}
-
-// Inputs implements no inputs
-func (noInputs) Inputs() []Primitive {
-	return nil
 }
diff --git a/go/vt/vtgate/engine/pullout_subquery.go b/go/vt/vtgate/engine/pullout_subquery.go
index ade790599a..b632d37359 100644
--- a/go/vt/vtgate/engine/pullout_subquery.go
+++ b/go/vt/vtgate/engine/pullout_subquery.go
@@ -38,11 +38,6 @@ type PulloutSubquery struct {
 	Underlying     Primitive
 }
 
-// Inputs returns the input primitives for this join
-func (ps *PulloutSubquery) Inputs() []Primitive {
-	return []Primitive{ps.Subquery, ps.Underlying}
-}
-
 // RouteType returns a description of the query routing type used by the primitive
 func (ps *PulloutSubquery) RouteType() string {
 	return ps.Opcode.String()
diff --git a/go/vt/vtgate/engine/route.go b/go/vt/vtgate/engine/route.go
index 9d5b45471b..4576894f13 100644
--- a/go/vt/vtgate/engine/route.go
+++ b/go/vt/vtgate/engine/route.go
@@ -84,9 +84,6 @@ type Route struct {
 
 	// ScatterErrorsAsWarnings is true if results should be returned even if some shards have an error
 	ScatterErrorsAsWarnings bool
-
-	// Route does not take inputs
-	noInputs
 }
 
 // NewSimpleRoute creates a Route with the bare minimum of parameters.
diff --git a/go/vt/vtgate/engine/subquery.go b/go/vt/vtgate/engine/subquery.go
index fc37cd9b3e..b9e28885dd 100644
--- a/go/vt/vtgate/engine/subquery.go
+++ b/go/vt/vtgate/engine/subquery.go
@@ -71,11 +71,6 @@ func (sq *Subquery) GetFields(vcursor VCursor, bindVars map[string]*querypb.Bind
 	return &sqltypes.Result{Fields: sq.buildFields(inner)}, nil
 }
 
-// Inputs returns the input to this primitive
-func (sq *Subquery) Inputs() []Primitive {
-	return []Primitive{sq.Subquery}
-}
-
 // buildResult builds a new result by pulling the necessary columns from
 // the subquery in the requested order.
 func (sq *Subquery) buildResult(inner *sqltypes.Result) *sqltypes.Result {
diff --git a/go/vt/vtgate/engine/update.go b/go/vt/vtgate/engine/update.go
index 49ee8324b9..dc3c591546 100644
--- a/go/vt/vtgate/engine/update.go
+++ b/go/vt/vtgate/engine/update.go
@@ -70,9 +70,6 @@ type Update struct {
 
 	// QueryTimeout contains the optional timeout (in milliseconds) to apply to this query
 	QueryTimeout int
-
-	// Update does not take inputs
-	noInputs
 }
 
 // MarshalJSON serializes the Update into a JSON representation.
diff --git a/go/vt/vtgate/engine/vindex_func.go b/go/vt/vtgate/engine/vindex_func.go
index bc1aaf61da..8ffb591405 100644
--- a/go/vt/vtgate/engine/vindex_func.go
+++ b/go/vt/vtgate/engine/vindex_func.go
@@ -39,9 +39,6 @@ type VindexFunc struct {
 	// TODO(sougou): add support for MultiColumn.
 	Vindex vindexes.SingleColumn
 	Value  sqltypes.PlanValue
-
-	// VindexFunc does not take inputs
-	noInputs
 }
 
 // MarshalJSON serializes the VindexFunc into a JSON representation.
diff --git a/go/vt/vtgate/executor.go b/go/vt/vtgate/executor.go
index 7a91df5e65..b3c9e2c898 100644
--- a/go/vt/vtgate/executor.go
+++ b/go/vt/vtgate/executor.go
@@ -92,10 +92,6 @@ type Executor struct {
 
 var executorOnce sync.Once
 
-const pathQueryPlans = "/debug/query_plans"
-const pathScatterStats = "/debug/scatter_stats"
-const pathVSchema = "/debug/vschema"
-
 // NewExecutor creates a new Executor.
 func NewExecutor(ctx context.Context, serv srvtopo.Server, cell, statsName string, resolver *Resolver, normalize bool, streamSize int, queryPlanCacheSize int64) *Executor {
 	e := &Executor{
@@ -121,9 +117,8 @@ func NewExecutor(ctx context.Context, serv srvtopo.Server, cell, statsName strin
 		stats.Publish("QueryPlanCacheOldest", stats.StringFunc(func() string {
 			return fmt.Sprintf("%v", e.plans.Oldest())
 		}))
-		http.Handle(pathQueryPlans, e)
-		http.Handle(pathScatterStats, e)
-		http.Handle(pathVSchema, e)
+		http.Handle("/debug/query_plans", e)
+		http.Handle("/debug/vschema", e)
 	})
 	return e
 }
@@ -1424,31 +1419,31 @@ func (e *Executor) ServeHTTP(response http.ResponseWriter, request *http.Request
 		acl.SendError(response, err)
 		return
 	}
-
-	switch request.URL.Path {
-	case pathQueryPlans:
-		returnAsJSON(response, e.plans.Items())
-	case pathVSchema:
-		returnAsJSON(response, e.VSchema())
-	case pathScatterStats:
-		e.WriteScatterStats(response)
-	default:
+	if request.URL.Path == "/debug/query_plans" {
+		response.Header().Set("Content-Type", "application/json; charset=utf-8")
+		buf, err := json.MarshalIndent(e.plans.Items(), "", " ")
+		if err != nil {
+			response.Write([]byte(err.Error()))
+			return
+		}
+		ebuf := bytes.NewBuffer(nil)
+		json.HTMLEscape(ebuf, buf)
+		response.Write(ebuf.Bytes())
+	} else if request.URL.Path == "/debug/vschema" {
+		response.Header().Set("Content-Type", "application/json; charset=utf-8")
+		b, err := json.MarshalIndent(e.VSchema(), "", " ")
+		if err != nil {
+			response.Write([]byte(err.Error()))
+			return
+		}
+		buf := bytes.NewBuffer(nil)
+		json.HTMLEscape(buf, b)
+		response.Write(buf.Bytes())
+	} else {
 		response.WriteHeader(http.StatusNotFound)
 	}
 }
 
-func returnAsJSON(response http.ResponseWriter, stuff interface{}) {
-	response.Header().Set("Content-Type", "application/json; charset=utf-8")
-	buf, err := json.MarshalIndent(stuff, "", " ")
-	if err != nil {
-		_, _ = response.Write([]byte(err.Error()))
-		return
-	}
-	ebuf := bytes.NewBuffer(nil)
-	json.HTMLEscape(ebuf, buf)
-	_, _ = response.Write(ebuf.Bytes())
-}
-
 // Plans returns the LRU plan cache
 func (e *Executor) Plans() *cache.LRUCache {
 	return e.plans
diff --git a/go/vt/vtgate/executor_scatter_stats.go b/go/vt/vtgate/executor_scatter_stats.go
deleted file mode 100644
index 24858564a5..0000000000
--- a/go/vt/vtgate/executor_scatter_stats.go
+++ /dev/null
@@ -1,184 +0,0 @@
-/*
-Copyright 2019 The Vitess Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package vtgate
-
-import (
-	"fmt"
-	"html/template"
-	"net/http"
-	"time"
-
-	"vitess.io/vitess/go/vt/logz"
-
-	"vitess.io/vitess/go/vt/proto/vtrpc"
-	"vitess.io/vitess/go/vt/vterrors"
-	"vitess.io/vitess/go/vt/vtgate/engine"
-)
-
-type statsResults struct {
-	TotalReadOnlyTime  time.Duration
-	TotalScatterTime   time.Duration
-	PercentTimeScatter float64
-	Items              []*statsResultItem
-}
-
-// using this intermediate data structure to make testing easier
-type statsResultItem struct {
-	Query                  string
-	AvgTimePerQuery        time.Duration
-	PercentTimeOfReads     float64
-	PercentTimeOfScatters  float64
-	PercentCountOfReads    float64
-	PercentCountOfScatters float64
-	From                   string
-	Count                  uint64
-}
-
-func (e *Executor) gatherScatterStats() (statsResults, error) {
-	scatterExecTime := time.Duration(0)
-	readOnlyTime := time.Duration(0)
-	scatterCount := uint64(0)
-	readOnlyCount := uint64(0)
-	totalExecTime := time.Duration(0)
-	totalCount := uint64(0)
-
-	plans := make([]*engine.Plan, 0)
-	routes := make([]*engine.Route, 0)
-	// First we go over all plans and collect statistics and all query plans for scatter queries
-	for _, item := range e.plans.Items() {
-		plan := item.Value.(*engine.Plan)
-
-		scatter := engine.Find(findScatter, plan.Instructions)
-		readOnly := !engine.Exists(isUpdating, plan.Instructions)
-		isScatter := scatter != nil
-
-		if isScatter {
-			route, isRoute := scatter.(*engine.Route)
-			if !isRoute {
-				return statsResults{}, vterrors.Errorf(vtrpc.Code_INTERNAL, "expected a route, but found a %v", scatter)
-			}
-			plans = append(plans, plan)
-			routes = append(routes, route)
-			scatterExecTime += plan.ExecTime
-			scatterCount += plan.ExecCount
-		}
-		if readOnly {
-			readOnlyTime += plan.ExecTime
-			readOnlyCount += plan.ExecCount
-		}
-
-		totalExecTime += plan.ExecTime
-		totalCount += plan.ExecCount
-	}
-
-	// Now we'll go over all scatter queries we've found and produce result items for each
-	resultItems := make([]*statsResultItem, len(plans))
-	for i, plan := range plans {
-		route := routes[i]
-		resultItems[i] = &statsResultItem{
-			Query:                  plan.Original,
-			AvgTimePerQuery:        time.Duration(plan.ExecTime.Nanoseconds() / int64(plan.ExecCount)),
-			PercentTimeOfReads:     float64(100 * plan.ExecTime / readOnlyTime),
-			PercentTimeOfScatters:  float64(100 * plan.ExecTime / scatterExecTime),
-			PercentCountOfReads:    float64(100 * plan.ExecCount / readOnlyCount),
-			PercentCountOfScatters: float64(100 * plan.ExecCount / scatterCount),
-			From:                   route.Keyspace.Name + "." + route.TableName,
-			Count:                  plan.ExecCount,
-		}
-	}
-	result := statsResults{
-		TotalReadOnlyTime:  readOnlyTime,
-		TotalScatterTime:   scatterExecTime,
-		PercentTimeScatter: 100 * float64(scatterExecTime) / float64(readOnlyTime),
-		Items:              resultItems,
-	}
-	return result, nil
-}
-
-// WriteScatterStats will write an html report to the provided response writer
-func (e *Executor) WriteScatterStats(w http.ResponseWriter) {
-	logz.StartHTMLTable(w)
-	defer logz.EndHTMLTable(w)
-
-	results, err := e.gatherScatterStats()
-	if err != nil {
-		http.Error(w, err.Error(), 500)
-		return
-	}
-
-	t := template.New("template")
-	t, err = t.Parse(html)
-	if err != nil {
-		http.Error(w, err.Error(), 500)
-		return
-	}
-
-	err = t.Execute(w, results)
-	if err != nil {
-		http.Error(w, err.Error(), 500)
-	}
-
-	_, err = w.Write([]byte(fmt.Sprintf("Percentage of time spent on scatter queries: %2.2f%%", results.PercentTimeScatter)))
-
-	if err != nil {
-		http.Error(w, err.Error(), 500)
-	}
-
-}
-
-const html = `
-<thead>
-	<tr>
-		<th>Query</th>
-		<th># of executions</th>
-		<th>Avg time/query</th>
-		<th>% time of reads</th>
-		<th>% time of scatters</th>
-		<th>% of reads</th>
-		<th>% of scatters</th>
-	</tr>
-</thead>
-{{range .Items}}
-<tr class="medium">
-	<td>{{.Query}}</td>
-	<td>{{.Count}}</td>
-	<td>{{.AvgTimePerQuery}}</td>
-	<td>{{.PercentTimeOfReads}}</td>
-	<td>{{.PercentTimeOfScatters}}</td>
-	<td>{{.PercentCountOfReads}}</td>
-	<td>{{.PercentCountOfScatters}}</td>
-</tr>
-{{end}}
-`
-
-func findScatter(p engine.Primitive) bool {
-	switch v := p.(type) {
-	case *engine.Route:
-		return v.Opcode == engine.SelectScatter
-	default:
-		return false
-	}
-}
-
-func isUpdating(p engine.Primitive) bool {
-	switch p.(type) {
-	case *engine.Update, *engine.Delete, *engine.Insert:
-		return true
-	default:
-		return false
-	}
-}
diff --git a/go/vt/vtgate/executor_stats.go b/go/vt/vtgate/executor_stats.go
index 8bb5a40bef..ffbc8412c1 100644
--- a/go/vt/vtgate/executor_stats.go
+++ b/go/vt/vtgate/executor_stats.go
@@ -33,7 +33,6 @@ const (
   <a href="/debug/querylogz">Current Query Log</a><br>
   <a href="/debug/queryz">Query Plan Stats</a><br>
   <a href="/debug/query_plans">Query Plans</a><br>
-  <a href="/debug/scatter_stats">Scatter Query Statistics</a><br>
 </td>
 </tr>
 </table>


Test patch:
diff --git a/go/vt/sqlparser/analyzer_test.go b/go/vt/sqlparser/analyzer_test.go
index 8ecc3ce0db..f97f36ccbc 100644
--- a/go/vt/sqlparser/analyzer_test.go
+++ b/go/vt/sqlparser/analyzer_test.go
@@ -50,6 +50,9 @@ func TestPreview(t *testing.T) {
 		{"begin ...", StmtUnknown},
 		{"begin /* ... */", StmtBegin},
 		{"begin /* ... *//*test*/", StmtBegin},
+		{"begin;", StmtBegin},
+		{"begin ;", StmtBegin},
+		{"begin; /*...*/", StmtBegin},
 		{"start transaction", StmtBegin},
 		{"commit", StmtCommit},
 		{"commit /*...*/", StmtCommit},
diff --git a/go/vt/sqlparser/parse_test.go b/go/vt/sqlparser/parse_test.go
index 564edccefc..8ae27a8594 100644
--- a/go/vt/sqlparser/parse_test.go
+++ b/go/vt/sqlparser/parse_test.go
@@ -1442,6 +1442,9 @@ var (
 		input: "stream /* comment */ * from t",
 	}, {
 		input: "begin",
+	}, {
+		input:  "begin;",
+		output: "begin",
 	}, {
 		input:  "start transaction",
 		output: "begin",
diff --git a/go/vt/vtgate/engine/fake_primitive_test.go b/go/vt/vtgate/engine/fake_primitive_test.go
index 2c0641a979..2d00e3e735 100644
--- a/go/vt/vtgate/engine/fake_primitive_test.go
+++ b/go/vt/vtgate/engine/fake_primitive_test.go
@@ -40,12 +40,6 @@ type fakePrimitive struct {
 	log []string
 }
 
-func (f *fakePrimitive) Inputs() []Primitive {
-	return []Primitive{}
-}
-
-var _ Primitive = (*fakePrimitive)(nil)
-
 func (f *fakePrimitive) rewind() {
 	f.curResult = 0
 	f.log = nil
diff --git a/go/vt/vtgate/executor_scatter_stats_test.go b/go/vt/vtgate/executor_scatter_stats_test.go
deleted file mode 100644
index 333a06be91..0000000000
--- a/go/vt/vtgate/executor_scatter_stats_test.go
+++ /dev/null
@@ -1,78 +0,0 @@
-/*
-Copyright 2019 The Vitess Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package vtgate
-
-import (
-	"context"
-	"testing"
-
-	"net/http/httptest"
-
-	"github.com/stretchr/testify/assert"
-
-	vtgatepb "vitess.io/vitess/go/vt/proto/vtgate"
-)
-
-func TestScatterStatsWithNoScatterQuery(t *testing.T) {
-	executor, _, _, _ := createExecutorEnv()
-	session := NewSafeSession(&vtgatepb.Session{TargetString: "@master"})
-
-	_, err := executor.Execute(context.Background(), "TestExecutorResultsExceeded", session, "select * from main1", nil)
-	assert.NoError(t, err)
-
-	result, err := executor.gatherScatterStats()
-	assert.NoError(t, err)
-	assert.Equal(t, 0, len(result.Items))
-}
-
-func TestScatterStatsWithSingleScatterQuery(t *testing.T) {
-	executor, _, _, _ := createExecutorEnv()
-	session := NewSafeSession(&vtgatepb.Session{TargetString: "@master"})
-
-	_, err := executor.Execute(context.Background(), "TestExecutorResultsExceeded", session, "select * from user", nil)
-	assert.NoError(t, err)
-
-	result, err := executor.gatherScatterStats()
-	assert.NoError(t, err)
-	assert.Equal(t, 1, len(result.Items))
-}
-
-func TestScatterStatsHttpWriting(t *testing.T) {
-	executor, _, _, _ := createExecutorEnv()
-	session := NewSafeSession(&vtgatepb.Session{TargetString: "@master"})
-
-	_, err := executor.Execute(context.Background(), "TestExecutorResultsExceeded", session, "select * from user", nil)
-	assert.NoError(t, err)
-
-	_, err = executor.Execute(context.Background(), "TestExecutorResultsExceeded", session, "select * from user where Id = 15", nil)
-	assert.NoError(t, err)
-
-	_, err = executor.Execute(context.Background(), "TestExecutorResultsExceeded", session, "select * from user where Id > 15", nil)
-	assert.NoError(t, err)
-
-	query4 := "select * from user as u1 join  user as u2 on u1.Id = u2.Id"
-	_, err = executor.Execute(context.Background(), "TestExecutorResultsExceeded", session, query4, nil)
-	assert.NoError(t, err)
-
-	recorder := httptest.NewRecorder()
-	executor.WriteScatterStats(recorder)
-
-	// Here we are checking that the template was executed correctly.
-	// If it wasn't, instead of html, we'll get an error message
-	assert.Contains(t, recorder.Body.String(), query4)
-	assert.NoError(t, err)
-}