Repository: vitessio/vitess
Pull request #7864 (2021-04-14 12:19:56)

Minor cleanups around errors on the vtgate
## Description
Removes redundant code and updates some error messages.

## Impacted Areas in Vitess
Components that this PR will affect:

- [x]  Query Serving


harshit-gangal: as the error message is changed, there are tests that needs to be fixed.

Patch:
diff --git a/go/vt/sqlparser/parser.go b/go/vt/sqlparser/parser.go
index 3cea4eeb59..b1e4c8086d 100644
--- a/go/vt/sqlparser/parser.go
+++ b/go/vt/sqlparser/parser.go
@@ -158,7 +158,7 @@ func parseNext(tokenizer *Tokenizer, strict bool) (Statement, error) {
 }
 
 // ErrEmpty is a sentinel error returned when parsing empty statements.
-var ErrEmpty = vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.EmptyQuery, "Query was empty")
+var ErrEmpty = vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.EmptyQuery, "query was empty")
 
 // SplitStatement returns the first sql statement up to either a ; or EOF
 // and the remainder from the given buffer
diff --git a/go/vt/vtgate/engine/set.go b/go/vt/vtgate/engine/set.go
index 51ee9934b9..ef27d88875 100644
--- a/go/vt/vtgate/engine/set.go
+++ b/go/vt/vtgate/engine/set.go
@@ -420,7 +420,7 @@ func (svss *SysVarSetAware) Execute(vcursor VCursor, env evalengine.ExpressionEn
 			// do nothing
 			break
 		default:
-			return vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.WrongValueForVar, "unexpected value for charset/names: %v", str)
+			return vterrors.Errorf(vtrpcpb.Code_UNIMPLEMENTED, "charset/name %v is not supported", str)
 		}
 	case sysvars.ReadAfterWriteGTID.Name:
 		str, err := svss.evalAsString(env)
@@ -445,10 +445,10 @@ func (svss *SysVarSetAware) Execute(vcursor VCursor, env evalengine.ExpressionEn
 		case "own_gtid":
 			vcursor.Session().SetSessionTrackGTIDs(true)
 		default:
-			return vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.WrongValueForVar, "Variable 'session_track_gtids' can't be set to the value of '%s'", str)
+			return vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.WrongValueForVar, "variable 'session_track_gtids' can't be set to the value of '%s'", str)
 		}
 	default:
-		return vterrors.NewErrorf(vtrpcpb.Code_NOT_FOUND, vterrors.UnknownSystemVariable, "Unknown system variable '%s'", svss.Name)
+		return vterrors.NewErrorf(vtrpcpb.Code_NOT_FOUND, vterrors.UnknownSystemVariable, "unknown system variable '%s'", svss.Name)
 	}
 
 	return err
@@ -462,7 +462,7 @@ func (svss *SysVarSetAware) evalAsInt64(env evalengine.ExpressionEnv) (int64, er
 
 	v := value.Value()
 	if !v.IsIntegral() {
-		return 0, vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.WrongTypeForVar, "Incorrect argument type to variable '%s': %s", svss.Name, value.Value().Type().String())
+		return 0, vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.WrongTypeForVar, "incorrect argument type to variable '%s': %s", svss.Name, value.Value().Type().String())
 	}
 	intValue, err := v.ToInt64()
 	if err != nil {
@@ -480,7 +480,7 @@ func (svss *SysVarSetAware) evalAsFloat(env evalengine.ExpressionEnv) (float64,
 	v := value.Value()
 	floatValue, err := v.ToFloat64()
 	if err != nil {
-		return 0, vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.WrongTypeForVar, "Incorrect argument type to variable '%s': %s", svss.Name, value.Value().Type().String())
+		return 0, vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.WrongTypeForVar, "incorrect argument type to variable '%s': %s", svss.Name, value.Value().Type().String())
 	}
 	return floatValue, nil
 }
@@ -492,7 +492,7 @@ func (svss *SysVarSetAware) evalAsString(env evalengine.ExpressionEnv) (string,
 	}
 	v := value.Value()
 	if !v.IsText() && !v.IsBinary() {
-		return "", vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.WrongTypeForVar, "Incorrect argument type to variable '%s': %s", svss.Name, value.Value().Type().String())
+		return "", vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.WrongTypeForVar, "incorrect argument type to variable '%s': %s", svss.Name, value.Value().Type().String())
 	}
 
 	return v.ToString(), nil
@@ -505,7 +505,7 @@ func (svss *SysVarSetAware) setBoolSysVar(env evalengine.ExpressionEnv, setter f
 	}
 	boolValue, err := value.ToBooleanStrict()
 	if err != nil {
-		return vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.WrongValueForVar, "Variable '%s' can't be set to the value: %s", svss.Name, err.Error())
+		return vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.WrongValueForVar, "variable '%s' can't be set to the value: %s", svss.Name, err.Error())
 	}
 	return setter(boolValue)
 }
diff --git a/go/vt/vtgate/executor.go b/go/vt/vtgate/executor.go
index d95dce136d..0496d1d5ae 100644
--- a/go/vt/vtgate/executor.go
+++ b/go/vt/vtgate/executor.go
@@ -22,7 +22,6 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
-	"regexp"
 	"sort"
 	"strconv"
 	"strings"
@@ -77,10 +76,6 @@ var (
 )
 
 const (
-	utf8          = "utf8"
-	utf8mb4       = "utf8mb4"
-	both          = "both"
-	charset       = "charset"
 	bindVarPrefix = "__vt"
 )
 
@@ -480,8 +475,7 @@ func (e *Executor) handleSet(ctx context.Context, sql string, logStats *LogStats
 	for _, expr := range set.Exprs {
 		// This is what correctly allows us to handle queries such as "set @@session.`autocommit`=1"
 		// it will remove backticks and double quotes that might surround the part after the first period
-		_, out := sqlparser.NewStringTokenizer(expr.Name.Lowered()).Scan()
-		name := string(out)
+		_, name := sqlparser.NewStringTokenizer(expr.Name.Lowered()).Scan()
 		switch expr.Scope {
 		case sqlparser.VitessMetadataScope:
 			value, err = getValueFor(expr)
@@ -510,15 +504,15 @@ func getValueFor(expr *sqlparser.SetExpr) (interface{}, error) {
 	case *sqlparser.Literal:
 		switch expr.Type {
 		case sqlparser.StrVal:
-			return strings.ToLower(string(expr.Val)), nil
+			return strings.ToLower(expr.Val), nil
 		case sqlparser.IntVal:
-			num, err := strconv.ParseInt(string(expr.Val), 0, 64)
+			num, err := strconv.ParseInt(expr.Val, 0, 64)
 			if err != nil {
 				return nil, err
 			}
 			return num, nil
 		case sqlparser.FloatVal:
-			num, err := strconv.ParseFloat(string(expr.Val), 64)
+			num, err := strconv.ParseFloat(expr.Val, 64)
 			if err != nil {
 				return nil, err
 			}
@@ -1391,98 +1385,6 @@ func buildVarCharRow(values ...string) []sqltypes.Value {
 	return row
 }
 
-func generateCharsetRows(showFilter *sqlparser.ShowFilter, colNames []string) ([][]sqltypes.Value, error) {
-	if showFilter == nil {
-		return buildCharsetRows(both), nil
-	}
-
-	var filteredColName string
-	var err error
-
-	if showFilter.Like != "" {
-		filteredColName, err = checkLikeOpt(showFilter.Like, colNames)
-		if err != nil {
-			return nil, err
-		}
-
-	} else {
-		cmpExp, ok := showFilter.Filter.(*sqlparser.ComparisonExpr)
-		if !ok {
-			return nil, vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.SyntaxError, "expect a 'LIKE' or '=' expression")
-		}
-
-		left, ok := cmpExp.Left.(*sqlparser.ColName)
-		if !ok {
-			return nil, vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.SyntaxError, "expect left side to be 'column'")
-		}
-		leftOk := left.Name.EqualString(charset)
-
-		if leftOk {
-			literal, ok := cmpExp.Right.(*sqlparser.Literal)
-			if !ok {
-				return nil, vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.SyntaxError, "expect right side to be string")
-			}
-			rightString := string(literal.Val)
-
-			switch cmpExp.Operator {
-			case sqlparser.EqualOp:
-				for _, colName := range colNames {
-					if rightString == colName {
-						filteredColName = colName
-					}
-				}
-			case sqlparser.LikeOp:
-				filteredColName, err = checkLikeOpt(rightString, colNames)
-				if err != nil {
-					return nil, err
-				}
-			}
-		}
-
-	}
-
-	return buildCharsetRows(filteredColName), nil
-}
-
-func buildCharsetRows(colName string) [][]sqltypes.Value {
-	row0 := buildVarCharRow(
-		"utf8",
-		"UTF-8 Unicode",
-		"utf8_general_ci")
-	row0 = append(row0, sqltypes.NewInt32(3))
-	row1 := buildVarCharRow(
-		"utf8mb4",
-		"UTF-8 Unicode",
-		"utf8mb4_general_ci")
-	row1 = append(row1, sqltypes.NewInt32(4))
-
-	switch colName {
-	case utf8:
-		return [][]sqltypes.Value{row0}
-	case utf8mb4:
-		return [][]sqltypes.Value{row1}
-	case both:
-		return [][]sqltypes.Value{row0, row1}
-	}
-
-	return [][]sqltypes.Value{}
-}
-
-func checkLikeOpt(likeOpt string, colNames []string) (string, error) {
-	likeRegexp := strings.ReplaceAll(likeOpt, "%", ".*")
-	for _, v := range colNames {
-		match, err := regexp.MatchString(likeRegexp, v)
-		if err != nil {
-			return "", err
-		}
-		if match {
-			return v, nil
-		}
-	}
-
-	return "", nil
-}
-
 // isValidPayloadSize validates whether a query payload is above the
 // configured MaxPayloadSize threshold. The WarnPayloadSizeExceeded will increment
 // if the payload size exceeds the warnPayloadSize.
diff --git a/go/vt/vtgate/planbuilder/bypass.go b/go/vt/vtgate/planbuilder/bypass.go
index 765fbaa0c0..abc42459ab 100644
--- a/go/vt/vtgate/planbuilder/bypass.go
+++ b/go/vt/vtgate/planbuilder/bypass.go
@@ -28,7 +28,7 @@ func buildPlanForBypass(stmt sqlparser.Statement, _ *sqlparser.ReservedVars, vsc
 	switch vschema.Destination().(type) {
 	case key.DestinationExactKeyRange:
 		if _, ok := stmt.(*sqlparser.Insert); ok {
-			return nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "range queries are not allowed for insert statement: %s", vschema.TargetString())
+			return nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "INSERT not supported when targeting a key range: %s", vschema.TargetString())
 		}
 	}
 
diff --git a/go/vt/vtgate/planbuilder/grouping.go b/go/vt/vtgate/planbuilder/grouping.go
index 49324a0e8b..a1b8614a64 100644
--- a/go/vt/vtgate/planbuilder/grouping.go
+++ b/go/vt/vtgate/planbuilder/grouping.go
@@ -56,7 +56,7 @@ func planGroupBy(pb *primitiveBuilder, input logicalPlan, groupBy sqlparser.Grou
 			case *sqlparser.ColName:
 				c := e.Metadata.(*column)
 				if c.Origin() == node {
-					return nil, vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.WrongGroupField, "group by expression cannot reference an aggregate function: %v", sqlparser.String(e))
+					return nil, vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.WrongGroupField, "Can't group on '%s'", sqlparser.String(e))
 				}
 				for i, rc := range node.resultColumns {
 					if rc.column == c {
diff --git a/go/vt/vtgate/planbuilder/ordering.go b/go/vt/vtgate/planbuilder/ordering.go
index 8350a5ed80..57f3b268db 100644
--- a/go/vt/vtgate/planbuilder/ordering.go
+++ b/go/vt/vtgate/planbuilder/ordering.go
@@ -48,7 +48,7 @@ func planOrdering(pb *primitiveBuilder, input logicalPlan, orderBy sqlparser.Ord
 	case *orderedAggregate:
 		return planOAOrdering(pb, orderBy, node)
 	case *mergeSort:
-		return nil, vterrors.Errorf(vtrpc.Code_INVALID_ARGUMENT, "can't do ORDER BY on top of ORDER BY")
+		return nil, vterrors.Errorf(vtrpc.Code_UNIMPLEMENTED, "can't do ORDER BY on top of ORDER BY")
 	}
 	if orderBy == nil {
 		return input, nil
diff --git a/go/vt/vtgate/planbuilder/queryprojection.go b/go/vt/vtgate/planbuilder/queryprojection.go
index e638d651e1..032fe0e283 100644
--- a/go/vt/vtgate/planbuilder/queryprojection.go
+++ b/go/vt/vtgate/planbuilder/queryprojection.go
@@ -52,7 +52,7 @@ func createQPFromSelect(sel *sqlparser.Select) (*queryProjection, error) {
 		fExpr, ok := exp.Expr.(*sqlparser.FuncExpr)
 		if ok && fExpr.IsAggregate() {
 			if len(fExpr.Exprs) != 1 {
-				return nil, vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.SyntaxError, "multiple arguments inside the function '%s'", sqlparser.String(fExpr))
+				return nil, vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.SyntaxError, "aggregate functions take a single argument '%s'", sqlparser.String(fExpr))
 			}
 			qp.aggrExprs = append(qp.aggrExprs, exp)
 			continue
diff --git a/go/vt/vtgate/planbuilder/set.go b/go/vt/vtgate/planbuilder/set.go
index d34e9397ab..a65059895e 100644
--- a/go/vt/vtgate/planbuilder/set.go
+++ b/go/vt/vtgate/planbuilder/set.go
@@ -104,13 +104,13 @@ func buildSetPlan(stmt *sqlparser.Set, vschema ContextVSchema) (engine.Primitive
 
 func buildSetOpReadOnly(s setting) planFunc {
 	return func(expr *sqlparser.SetExpr, schema ContextVSchema, _ *expressionConverter) (engine.SetOp, error) {
-		return nil, vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.IncorrectGlobalLocalVar, "Variable '%s' is a read only variable", expr.Name)
+		return nil, vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.IncorrectGlobalLocalVar, "variable '%s' is a read only variable", expr.Name)
 	}
 }
 
 func buildNotSupported(setting) planFunc {
 	return func(expr *sqlparser.SetExpr, schema ContextVSchema, _ *expressionConverter) (engine.SetOp, error) {
-		return nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "%s: system setting is not supported", expr.Name)
+		return nil, vterrors.Errorf(vtrpcpb.Code_UNIMPLEMENTED, "%s: system setting is not supported", expr.Name)
 	}
 }
 
diff --git a/go/vt/vtgate/planbuilder/show.go b/go/vt/vtgate/planbuilder/show.go
index a4b166fdc4..452e143c95 100644
--- a/go/vt/vtgate/planbuilder/show.go
+++ b/go/vt/vtgate/planbuilder/show.go
@@ -311,21 +311,21 @@ func generateCharsetRows(showFilter *sqlparser.ShowFilter, colNames []string) ([
 	} else {
 		cmpExp, ok := showFilter.Filter.(*sqlparser.ComparisonExpr)
 		if !ok {
-			return nil, vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.SyntaxError, "expect a 'LIKE' or '=' expression")
+			return nil, vterrors.Errorf(vtrpcpb.Code_UNIMPLEMENTED, "expect a 'LIKE' or '=' expression")
 		}
 
 		left, ok := cmpExp.Left.(*sqlparser.ColName)
 		if !ok {
-			return nil, vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.SyntaxError, "expect left side to be 'charset'")
+			return nil, vterrors.Errorf(vtrpcpb.Code_UNIMPLEMENTED, "expect left side to be 'charset'")
 		}
 		leftOk := left.Name.EqualString(charset)
 
 		if leftOk {
 			literal, ok := cmpExp.Right.(*sqlparser.Literal)
 			if !ok {
-				return nil, vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.SyntaxError, "we expect the right side to be a string")
+				return nil, vterrors.Errorf(vtrpcpb.Code_UNIMPLEMENTED, "we expect the right side to be a string")
 			}
-			rightString := string(literal.Val)
+			rightString := literal.Val
 
 			switch cmpExp.Operator {
 			case sqlparser.EqualOp:
diff --git a/go/vt/vtgate/vcursor_impl.go b/go/vt/vtgate/vcursor_impl.go
index f60af6dca2..8ba52d45cf 100644
--- a/go/vt/vtgate/vcursor_impl.go
+++ b/go/vt/vtgate/vcursor_impl.go
@@ -525,11 +525,11 @@ func (vc *vcursorImpl) SetTarget(target string) error {
 		return err
 	}
 	if _, ok := vc.vschema.Keyspaces[keyspace]; !ignoreKeyspace(keyspace) && !ok {
-		return vterrors.NewErrorf(vtrpcpb.Code_NOT_FOUND, vterrors.BadDb, "Unknown database '%s'", keyspace)
+		return vterrors.NewErrorf(vtrpcpb.Code_NOT_FOUND, vterrors.BadDb, "unknown database '%s'", keyspace)
 	}
 
 	if vc.safeSession.InTransaction() && tabletType != topodatapb.TabletType_MASTER {
-		return vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.LockOrActiveTransaction, "Can't execute the given command because you have an active transaction")
+		return vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.LockOrActiveTransaction, "can't execute the given command because you have an active transaction")
 	}
 	vc.safeSession.SetTargetString(target)
 	return nil


Test patch:
diff --git a/go/test/endtoend/vtgate/reservedconn/sysvar_test.go b/go/test/endtoend/vtgate/reservedconn/sysvar_test.go
index 6953178108..f14aa287e5 100644
--- a/go/test/endtoend/vtgate/reservedconn/sysvar_test.go
+++ b/go/test/endtoend/vtgate/reservedconn/sysvar_test.go
@@ -393,5 +393,5 @@ func TestSysvarSocket(t *testing.T) {
 	require.True(t, ok, "not a mysql error: %T", err)
 	assert.Equal(t, mysql.ERIncorrectGlobalLocalVar, sqlErr.Number())
 	assert.Equal(t, mysql.SSUnknownSQLState, sqlErr.SQLState())
-	assert.Equal(t, "Variable 'socket' is a read only variable (errno 1238) (sqlstate HY000) during query: set socket = '/any/path'", sqlErr.Error())
+	assert.Equal(t, "variable 'socket' is a read only variable (errno 1238) (sqlstate HY000) during query: set socket = '/any/path'", sqlErr.Error())
 }
diff --git a/go/vt/sqlparser/parse_test.go b/go/vt/sqlparser/parse_test.go
index 937a22265a..24ea51b316 100644
--- a/go/vt/sqlparser/parse_test.go
+++ b/go/vt/sqlparser/parse_test.go
@@ -2004,7 +2004,7 @@ func TestInvalid(t *testing.T) {
 		err:   "syntax error",
 	}, {
 		input: "/*!*/",
-		err:   "Query was empty",
+		err:   "query was empty",
 	}}
 
 	for _, tcase := range invalidSQL {
@@ -2316,7 +2316,7 @@ func TestConvert(t *testing.T) {
 		output: "syntax error at position 33",
 	}, {
 		input:  "/* a comment */",
-		output: "Query was empty",
+		output: "query was empty",
 	}, {
 		input:  "set transaction isolation level 12345",
 		output: "syntax error at position 38 near '12345'",
diff --git a/go/vt/vtgate/executor_dml_test.go b/go/vt/vtgate/executor_dml_test.go
index bce657e799..6af655507c 100644
--- a/go/vt/vtgate/executor_dml_test.go
+++ b/go/vt/vtgate/executor_dml_test.go
@@ -1605,7 +1605,7 @@ func TestKeyDestRangeQuery(t *testing.T) {
 	masterSession.TargetString = "TestExecutor[-]"
 	_, err := executorExec(executor, insertInput, nil)
 
-	require.EqualError(t, err, "range queries are not allowed for insert statement: TestExecutor[-]")
+	require.EqualError(t, err, "INSERT not supported when targeting a key range: TestExecutor[-]")
 
 	masterSession.TargetString = ""
 }
diff --git a/go/vt/vtgate/executor_set_test.go b/go/vt/vtgate/executor_set_test.go
index aa177d77cd..47537ab10c 100644
--- a/go/vt/vtgate/executor_set_test.go
+++ b/go/vt/vtgate/executor_set_test.go
@@ -89,10 +89,10 @@ func TestExecutorSet(t *testing.T) {
 		out: &vtgatepb.Session{},
 	}, {
 		in:  "set AUTOCOMMIT = 'aa'",
-		err: "Variable 'autocommit' can't be set to the value: 'aa' is not a boolean",
+		err: "variable 'autocommit' can't be set to the value: 'aa' is not a boolean",
 	}, {
 		in:  "set autocommit = 2",
-		err: "Variable 'autocommit' can't be set to the value: 2 is not a boolean",
+		err: "variable 'autocommit' can't be set to the value: 2 is not a boolean",
 	}, {
 		in:  "set client_found_rows = 1",
 		out: &vtgatepb.Session{Autocommit: true, Options: &querypb.ExecuteOptions{ClientFoundRows: true}},
@@ -110,10 +110,10 @@ func TestExecutorSet(t *testing.T) {
 		err: "cannot use scope and @@",
 	}, {
 		in:  "set client_found_rows = 'aa'",
-		err: "Variable 'client_found_rows' can't be set to the value: 'aa' is not a boolean",
+		err: "variable 'client_found_rows' can't be set to the value: 'aa' is not a boolean",
 	}, {
 		in:  "set client_found_rows = 2",
-		err: "Variable 'client_found_rows' can't be set to the value: 2 is not a boolean",
+		err: "variable 'client_found_rows' can't be set to the value: 2 is not a boolean",
 	}, {
 		in:  "set transaction_mode = 'unspecified'",
 		out: &vtgatepb.Session{Autocommit: true, TransactionMode: vtgatepb.TransactionMode_UNSPECIFIED},
@@ -134,7 +134,7 @@ func TestExecutorSet(t *testing.T) {
 		err: "invalid transaction_mode: aa",
 	}, {
 		in:  "set transaction_mode = 1",
-		err: "Incorrect argument type to variable 'transaction_mode': INT64",
+		err: "incorrect argument type to variable 'transaction_mode': INT64",
 	}, {
 		in:  "set workload = 'unspecified'",
 		out: &vtgatepb.Session{Autocommit: true, Options: &querypb.ExecuteOptions{Workload: querypb.ExecuteOptions_UNSPECIFIED}},
@@ -152,7 +152,7 @@ func TestExecutorSet(t *testing.T) {
 		err: "invalid workload: aa",
 	}, {
 		in:  "set workload = 1",
-		err: "Incorrect argument type to variable 'workload': INT64",
+		err: "incorrect argument type to variable 'workload': INT64",
 	}, {
 		in:  "set transaction_mode = 'twopc', autocommit=1",
 		out: &vtgatepb.Session{Autocommit: true, TransactionMode: vtgatepb.TransactionMode_TWOPC},
@@ -164,10 +164,10 @@ func TestExecutorSet(t *testing.T) {
 		out: &vtgatepb.Session{Autocommit: true, Options: &querypb.ExecuteOptions{SqlSelectLimit: 0}},
 	}, {
 		in:  "set sql_select_limit = 'asdfasfd'",
-		err: "Incorrect argument type to variable 'sql_select_limit': VARBINARY",
+		err: "incorrect argument type to variable 'sql_select_limit': VARBINARY",
 	}, {
 		in:  "set autocommit = 1+1",
-		err: "Variable 'autocommit' can't be set to the value: 2 is not a boolean",
+		err: "variable 'autocommit' can't be set to the value: 2 is not a boolean",
 	}, {
 		in:  "set autocommit = 1+0",
 		out: &vtgatepb.Session{Autocommit: true},
@@ -182,7 +182,7 @@ func TestExecutorSet(t *testing.T) {
 		out: &vtgatepb.Session{Autocommit: true},
 	}, {
 		in:  "set names ascii",
-		err: "unexpected value for charset/names: ascii",
+		err: "charset/name ascii is not supported",
 	}, {
 		in:  "set charset utf8",
 		out: &vtgatepb.Session{Autocommit: true},
@@ -191,7 +191,7 @@ func TestExecutorSet(t *testing.T) {
 		out: &vtgatepb.Session{Autocommit: true},
 	}, {
 		in:  "set character set ascii",
-		err: "unexpected value for charset/names: ascii",
+		err: "charset/name ascii is not supported",
 	}, {
 		in:  "set skip_query_plan_cache = 1",
 		out: &vtgatepb.Session{Autocommit: true, Options: &querypb.ExecuteOptions{SkipQueryPlanCache: true}},
@@ -200,10 +200,10 @@ func TestExecutorSet(t *testing.T) {
 		out: &vtgatepb.Session{Autocommit: true, Options: &querypb.ExecuteOptions{}},
 	}, {
 		in:  "set tx_read_only = 2",
-		err: "Variable 'tx_read_only' can't be set to the value: 2 is not a boolean",
+		err: "variable 'tx_read_only' can't be set to the value: 2 is not a boolean",
 	}, {
 		in:  "set transaction_read_only = 2",
-		err: "Variable 'transaction_read_only' can't be set to the value: 2 is not a boolean",
+		err: "variable 'transaction_read_only' can't be set to the value: 2 is not a boolean",
 	}, {
 		in:  "set session transaction isolation level repeatable read",
 		out: &vtgatepb.Session{Autocommit: true},
@@ -248,7 +248,7 @@ func TestExecutorSet(t *testing.T) {
 		out: &vtgatepb.Session{Autocommit: true, EnableSystemSettings: false},
 	}, {
 		in:  "set @@socket = '/tmp/change.sock'",
-		err: "Variable 'socket' is a read only variable",
+		err: "variable 'socket' is a read only variable",
 	}}
 	for i, tcase := range testcases {
 		t.Run(fmt.Sprintf("%d-%s", i, tcase.in), func(t *testing.T) {
diff --git a/go/vt/vtgate/executor_test.go b/go/vt/vtgate/executor_test.go
index 2f9fd2f80a..4a38de511f 100644
--- a/go/vt/vtgate/executor_test.go
+++ b/go/vt/vtgate/executor_test.go
@@ -195,7 +195,7 @@ func TestLegacyExecutorTransactionsNoAutoCommit(t *testing.T) {
 	// Prevent use of non-master if in_transaction is on.
 	session = NewSafeSession(&vtgatepb.Session{TargetString: "@master", InTransaction: true})
 	_, err = executor.Execute(ctx, "TestExecute", session, "use @replica", nil)
-	require.EqualError(t, err, `Can't execute the given command because you have an active transaction`)
+	require.EqualError(t, err, `can't execute the given command because you have an active transaction`)
 }
 
 func TestDirectTargetRewrites(t *testing.T) {
@@ -943,10 +943,7 @@ func TestExecutorUse(t *testing.T) {
 	}
 
 	_, err = executor.Execute(ctx, "TestExecute", NewSafeSession(&vtgatepb.Session{}), "use UnexistentKeyspace", nil)
-	wantErr = "Unknown database 'UnexistentKeyspace'"
-	if err == nil || err.Error() != wantErr {
-		t.Errorf("got: %v, want %v", err, wantErr)
-	}
+	require.EqualError(t, err, "unknown database 'UnexistentKeyspace'")
 }
 
 func TestExecutorComment(t *testing.T) {
@@ -1806,63 +1803,6 @@ func TestDebugVSchema(t *testing.T) {
 	}
 }
 
-func TestGenerateCharsetRows(t *testing.T) {
-	rows := make([][]sqltypes.Value, 0, 4)
-	rows0 := [][]sqltypes.Value{
-		append(buildVarCharRow(
-			"utf8",
-			"UTF-8 Unicode",
-			"utf8_general_ci"),
-			sqltypes.NewInt32(3)),
-	}
-	rows1 := [][]sqltypes.Value{
-		append(buildVarCharRow(
-			"utf8mb4",
-			"UTF-8 Unicode",
-			"utf8mb4_general_ci"),
-			sqltypes.NewInt32(4)),
-	}
-	rows2 := [][]sqltypes.Value{
-		append(buildVarCharRow(
-			"utf8",
-			"UTF-8 Unicode",
-			"utf8_general_ci"),
-			sqltypes.NewInt32(3)),
-		append(buildVarCharRow(
-			"utf8mb4",
-			"UTF-8 Unicode",
-			"utf8mb4_general_ci"),
-			sqltypes.NewInt32(4)),
-	}
-
-	testcases := []struct {
-		input    string
-		expected [][]sqltypes.Value
-	}{
-		{input: "show charset", expected: rows2},
-		{input: "show character set", expected: rows2},
-		{input: "show charset where charset like 'foo%'", expected: rows},
-		{input: "show charset where charset like 'utf8%'", expected: rows0},
-		{input: "show charset where charset = 'utf8'", expected: rows0},
-		{input: "show charset where charset = 'foo%'", expected: rows},
-		{input: "show charset where charset = 'utf8mb4'", expected: rows1},
-	}
-
-	charsets := []string{"utf8", "utf8mb4"}
-
-	for _, tc := range testcases {
-		t.Run(tc.input, func(t *testing.T) {
-			stmt, err := sqlparser.Parse(tc.input)
-			require.NoError(t, err)
-			match := stmt.(*sqlparser.Show).Internal.(*sqlparser.ShowBasic)
-			filter := match.Filter
-			actual, err := generateCharsetRows(filter, charsets)
-			require.NoError(t, err)
-			require.Equal(t, tc.expected, actual)
-		})
-	}
-}
-
 func TestExecutorMaxPayloadSizeExceeded(t *testing.T) {
 	saveMax := *maxPayloadSize
 	saveWarn := *warnPayloadSize
diff --git a/go/vt/vtgate/mysql_protocol_test.go b/go/vt/vtgate/mysql_protocol_test.go
index 33e1a814f5..8cce6b3c61 100644
--- a/go/vt/vtgate/mysql_protocol_test.go
+++ b/go/vt/vtgate/mysql_protocol_test.go
@@ -120,7 +120,7 @@ func TestMySQLProtocolExecuteUseStatement(t *testing.T) {
 	// No such keyspace this will fail
 	_, err = c.ExecuteFetch("use InvalidKeyspace", 0, false)
 	require.Error(t, err)
-	assert.Contains(t, err.Error(), "Unknown database 'InvalidKeyspace' (errno 1049) (sqlstate 42000)")
+	assert.Contains(t, err.Error(), "unknown database 'InvalidKeyspace' (errno 1049) (sqlstate 42000)")
 
 	// That doesn't reset the vitess_target
 	qr, err = c.ExecuteFetch("show vitess_target", 1, false)
@@ -138,7 +138,7 @@ func TestMySQLProtocolExecuteUseStatement(t *testing.T) {
 
 func TestMysqlProtocolInvalidDB(t *testing.T) {
 	_, err := mysqlConnect(&mysql.ConnParams{DbName: "invalidDB"})
-	require.EqualError(t, err, "Unknown database 'invalidDB' (errno 1049) (sqlstate 42000)")
+	require.EqualError(t, err, "unknown database 'invalidDB' (errno 1049) (sqlstate 42000)")
 }
 
 func TestMySQLProtocolClientFoundRows(t *testing.T) {
diff --git a/go/vt/vtgate/planbuilder/show_test.go b/go/vt/vtgate/planbuilder/show_test.go
new file mode 100644
index 0000000000..fc614d7026
--- /dev/null
+++ b/go/vt/vtgate/planbuilder/show_test.go
@@ -0,0 +1,83 @@
+/*
+Copyright 2021 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 planbuilder
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"vitess.io/vitess/go/sqltypes"
+	"vitess.io/vitess/go/vt/sqlparser"
+)
+
+func TestGenerateCharsetRows(t *testing.T) {
+	rows := make([][]sqltypes.Value, 0, 4)
+	rows0 := [][]sqltypes.Value{
+		append(buildVarCharRow(
+			"utf8",
+			"UTF-8 Unicode",
+			"utf8_general_ci"),
+			sqltypes.NewInt32(3)),
+	}
+	rows1 := [][]sqltypes.Value{
+		append(buildVarCharRow(
+			"utf8mb4",
+			"UTF-8 Unicode",
+			"utf8mb4_general_ci"),
+			sqltypes.NewInt32(4)),
+	}
+	rows2 := [][]sqltypes.Value{
+		append(buildVarCharRow(
+			"utf8",
+			"UTF-8 Unicode",
+			"utf8_general_ci"),
+			sqltypes.NewInt32(3)),
+		append(buildVarCharRow(
+			"utf8mb4",
+			"UTF-8 Unicode",
+			"utf8mb4_general_ci"),
+			sqltypes.NewInt32(4)),
+	}
+
+	testcases := []struct {
+		input    string
+		expected [][]sqltypes.Value
+	}{
+		{input: "show charset", expected: rows2},
+		{input: "show character set", expected: rows2},
+		{input: "show charset where charset like 'foo%'", expected: rows},
+		{input: "show charset where charset like 'utf8%'", expected: rows0},
+		{input: "show charset where charset = 'utf8'", expected: rows0},
+		{input: "show charset where charset = 'foo%'", expected: rows},
+		{input: "show charset where charset = 'utf8mb4'", expected: rows1},
+	}
+
+	charsets := []string{"utf8", "utf8mb4"}
+
+	for _, tc := range testcases {
+		t.Run(tc.input, func(t *testing.T) {
+			stmt, err := sqlparser.Parse(tc.input)
+			require.NoError(t, err)
+			match := stmt.(*sqlparser.Show).Internal.(*sqlparser.ShowBasic)
+			filter := match.Filter
+			actual, err := generateCharsetRows(filter, charsets)
+			require.NoError(t, err)
+			require.Equal(t, tc.expected, actual)
+		})
+	}
+}
diff --git a/go/vt/vtgate/planbuilder/testdata/aggr_cases.txt b/go/vt/vtgate/planbuilder/testdata/aggr_cases.txt
index 59d635dc85..cec49eeb0a 100644
--- a/go/vt/vtgate/planbuilder/testdata/aggr_cases.txt
+++ b/go/vt/vtgate/planbuilder/testdata/aggr_cases.txt
@@ -880,7 +880,7 @@ Gen4 plan same as above
 
 # scatter aggregate group by aggregate function
 " select count(*) b from user group by b"
-"group by expression cannot reference an aggregate function: b"
+"Can't group on 'b'"
 
 # scatter aggregate multiple group by (columns)
 "select a, b, count(*) from user group by b, a"
diff --git a/go/vt/vtgate/planbuilder/testdata/set_cases.txt b/go/vt/vtgate/planbuilder/testdata/set_cases.txt
index 3a29de7278..d691e8162c 100644
--- a/go/vt/vtgate/planbuilder/testdata/set_cases.txt
+++ b/go/vt/vtgate/planbuilder/testdata/set_cases.txt
@@ -524,5 +524,5 @@ Gen4 plan same as above
 
 # change read only variable
 "set socket = ''"
-"Variable 'socket' is a read only variable"
+"variable 'socket' is a read only variable"
 Gen4 plan same as above
diff --git a/go/vt/vtgate/vcursor_impl_test.go b/go/vt/vtgate/vcursor_impl_test.go
index 418c53d00e..70493008bc 100644
--- a/go/vt/vtgate/vcursor_impl_test.go
+++ b/go/vt/vtgate/vcursor_impl_test.go
@@ -234,11 +234,11 @@ func TestSetTarget(t *testing.T) {
 	}, {
 		vschema:       vschemaWith2KS,
 		targetString:  "ks3",
-		expectedError: "Unknown database 'ks3'",
+		expectedError: "unknown database 'ks3'",
 	}, {
 		vschema:       vschemaWith2KS,
 		targetString:  "ks2@replica",
-		expectedError: "Can't execute the given command because you have an active transaction",
+		expectedError: "can't execute the given command because you have an active transaction",
 	}}
 
 	for i, tc := range tests {
diff --git a/go/vt/vttablet/tabletserver/query_engine_test.go b/go/vt/vttablet/tabletserver/query_engine_test.go
index d74dfbb90b..685e784cb7 100644
--- a/go/vt/vttablet/tabletserver/query_engine_test.go
+++ b/go/vt/vttablet/tabletserver/query_engine_test.go
@@ -109,7 +109,7 @@ func TestGetPlanPanicDuetoEmptyQuery(t *testing.T) {
 	ctx := context.Background()
 	logStats := tabletenv.NewLogStats(ctx, "GetPlanStats")
 	_, err := qe.GetPlan(ctx, logStats, "", false, false /* inReservedConn */)
-	require.EqualError(t, err, "Query was empty")
+	require.EqualError(t, err, "query was empty")
 }
 
 func addSchemaEngineQueries(db *fakesqldb.DB) {