Skip to content

Commit

Permalink
removed usage of generating mysql error inside vtgate code, instead g…
Browse files Browse the repository at this point in the history
…enerate vterrors errors that will be converted later to mysql errors

Signed-off-by: Harshit Gangal <[email protected]>
  • Loading branch information
harshit-gangal committed Feb 25, 2021
1 parent 7c3de7d commit dfbf58d
Show file tree
Hide file tree
Showing 12 changed files with 53 additions and 40 deletions.
1 change: 1 addition & 0 deletions go/mysql/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,7 @@ const (
ERQueryInterrupted = 1317
ERTruncatedWrongValueForField = 1366
ERDataTooLong = 1406
ERForbidSchemaChange = 1450
ERDataOutOfRange = 1690
)

Expand Down
36 changes: 13 additions & 23 deletions go/mysql/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1160,15 +1160,15 @@ func TestErrorCodes(t *testing.T) {
vtrpcpb.Code_DEADLINE_EXCEEDED,
"connection deadline exceeded"),
code: ERQueryInterrupted,
sqlState: SSUnknownSQLState,
sqlState: SSQueryInterrupted,
text: "deadline exceeded",
},
{
err: vterrors.Errorf(
vtrpcpb.Code_RESOURCE_EXHAUSTED,
"query pool timeout"),
code: ERTooManyUserConnections,
sqlState: SSUnknownSQLState,
sqlState: SSClientError,
text: "resource exhausted",
},
{
Expand All @@ -1180,27 +1180,17 @@ func TestErrorCodes(t *testing.T) {
}

for _, test := range tests {
th.SetErr(NewSQLErrorFromError(test.err))
result, err := client.ExecuteFetch("error", 100, false)
if err == nil {
t.Fatalf("mysql should have failed but returned: %v", result)
}
serr, ok := err.(*SQLError)
if !ok {
t.Fatalf("mysql should have returned a SQLError")
}

if serr.Number() != test.code {
t.Errorf("error in %s: want code %v got %v", test.text, test.code, serr.Number())
}

if serr.SQLState() != test.sqlState {
t.Errorf("error in %s: want sqlState %v got %v", test.text, test.sqlState, serr.SQLState())
}

if !strings.Contains(serr.Error(), test.err.Error()) {
t.Errorf("error in %s: want err %v got %v", test.text, test.err.Error(), serr.Error())
}
t.Run(test.err.Error(), func(t *testing.T) {
th.SetErr(NewSQLErrorFromError(test.err))
rs, err := client.ExecuteFetch("error", 100, false)
require.Error(t, err, "mysql should have failed but returned: %v", rs)
serr, ok := err.(*SQLError)
require.True(t, ok, "mysql should have returned a SQLError")

assert.Equal(t, test.code, serr.Number(), "error in %s: want code %v got %v", test.text, test.code, serr.Number())
assert.Equal(t, test.sqlState, serr.SQLState(), "error in %s: want sqlState %v got %v", test.text, test.sqlState, serr.SQLState())
assert.Contains(t, serr.Error(), test.err.Error())
})
}
}

Expand Down
12 changes: 12 additions & 0 deletions go/mysql/sql_error.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,14 +153,26 @@ var stateToMysqlCode = map[vterrors.State]struct {
num int
state string
}{
vterrors.Undefined: {num: ERUnknownError, state: SSUnknownSQLState},
vterrors.DataOutOfRange: {num: ERDataOutOfRange, state: SSDataOutOfRange},
vterrors.NoDB: {num: ERNoDb, state: SSNoDB},
vterrors.WrongNumberOfColumnsInSelect: {num: ERWrongNumberOfColumnsInSelect, state: SSWrongNumberOfColumns},
vterrors.BadFieldError: {num: ERBadFieldError, state: SSBadFieldError},
vterrors.DbCreateExists: {num: ERDbCreateExists, state: SSUnknownSQLState},
vterrors.DbDropExists: {num: ERDbDropExists, state: SSUnknownSQLState},
vterrors.ForbidSchemaChange: {num: ERForbidSchemaChange, state: SSUnknownSQLState},
vterrors.NetPacketTooLarge: {num: ERNetPacketTooLarge, state: SSNetError},
vterrors.SPDoesNotExist: {num: ERSPDoesNotExist, state: SSClientError},
vterrors.QueryInterrupted: {num: ERQueryInterrupted, state: SSQueryInterrupted},
vterrors.CantUseOptionHere: {num: ERCantUseOptionHere, state: SSClientError},
vterrors.NonUniqTable: {num: ERNonUniqTable, state: SSClientError},
vterrors.BadDb: {num: ERBadDb, state: SSClientError},
}

func init() {
if len(stateToMysqlCode) != int(vterrors.NumOfStates) {
panic("all vterrors states are not mapped to mysql errors")
}
}

func convertToMysqlError(err error) error {
Expand Down
7 changes: 7 additions & 0 deletions go/vt/vterrors/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,11 @@ const (
ForbidSchemaChange
NetPacketTooLarge
SPDoesNotExist
QueryInterrupted
CantUseOptionHere
NonUniqTable
BadDb

// No state should be added below NumOfStates
NumOfStates
)
9 changes: 6 additions & 3 deletions go/vt/vtgate/engine/route_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ import (
"errors"
"testing"

vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc"
"vitess.io/vitess/go/vt/vterrors"

"vitess.io/vitess/go/vt/sqlparser"

"vitess.io/vitess/go/vt/vtgate/evalengine"
Expand Down Expand Up @@ -1002,14 +1005,14 @@ func TestExecFail(t *testing.T) {
"dummy_select_field",
)

vc := &loggingVCursor{shards: []string{"0"}, resultErr: mysql.NewSQLError(mysql.ERQueryInterrupted, "", "query timeout")}
vc := &loggingVCursor{shards: []string{"0"}, resultErr: vterrors.NewErrorf(vtrpcpb.Code_CANCELED, vterrors.QueryInterrupted, "query timeout")}
_, err := sel.Execute(vc, map[string]*querypb.BindVariable{}, false)
require.EqualError(t, err, `query timeout (errno 1317) (sqlstate HY000)`)
require.EqualError(t, err, `query timeout`)
vc.ExpectWarnings(t, nil)

vc.Rewind()
_, err = wrapStreamExecute(sel, vc, map[string]*querypb.BindVariable{}, false)
require.EqualError(t, err, `query timeout (errno 1317) (sqlstate HY000)`)
require.EqualError(t, err, `query timeout`)

// Scatter fails if one of N fails without ScatterErrorsAsWarnings
sel = NewRoute(
Expand Down
2 changes: 1 addition & 1 deletion go/vt/vtgate/executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1858,7 +1858,7 @@ func TestExecutorMaxPayloadSizeExceeded(t *testing.T) {
for _, query := range testMaxPayloadSizeExceeded {
_, err := executor.Execute(context.Background(), "TestExecutorMaxPayloadSizeExceeded", session, query, nil)
require.NotNil(t, err)
assert.EqualError(t, err, "query payload size above threshold (errno 1153) (sqlstate HY000)")
assert.EqualError(t, err, "query payload size above threshold")
}
assert.Equal(t, warningCount, warnings.Counts()["WarnPayloadSizeExceeded"], "warnings count")

Expand Down
9 changes: 5 additions & 4 deletions go/vt/vtgate/planbuilder/select.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ import (

"vitess.io/vitess/go/vt/vtgate/semantics"

"vitess.io/vitess/go/mysql"

"vitess.io/vitess/go/vt/key"

vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc"
Expand Down Expand Up @@ -164,6 +162,9 @@ func dependsOnRoute(solves semantics.TableSet, expr sqlparser.Expr, semTable *se
return !sqlparser.IsValue(expr)
}

var errSQLCalcFoundRows = vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.CantUseOptionHere, "Incorrect usage/placement of 'SQL_CALC_FOUND_ROWS'")
var errInto = vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.CantUseOptionHere, "Incorrect usage/placement of 'INTO'")

// processSelect builds a primitive tree for the given query or subquery.
// The tree built by this function has the following general structure:
//
Expand Down Expand Up @@ -208,7 +209,7 @@ func (pb *primitiveBuilder) processSelect(sel *sqlparser.Select, outer *symtab,
}
if sel.SQLCalcFoundRows {
if outer != nil || query == "" {
return mysql.NewSQLError(mysql.ERCantUseOptionHere, mysql.SSClientError, "Incorrect usage/placement of 'SQL_CALC_FOUND_ROWS'")
return errSQLCalcFoundRows
}
sel.SQLCalcFoundRows = false
if sel.Limit != nil {
Expand All @@ -223,7 +224,7 @@ func (pb *primitiveBuilder) processSelect(sel *sqlparser.Select, outer *symtab,

// Into is not supported in subquery.
if sel.Into != nil && (outer != nil || query == "") {
return mysql.NewSQLError(mysql.ERCantUseOptionHere, mysql.SSClientError, "Incorrect usage/placement of 'INTO'")
return errInto
}

var where sqlparser.Expr
Expand Down
4 changes: 2 additions & 2 deletions go/vt/vtgate/planbuilder/testdata/ddl_cases.txt
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@

# create db main
"create database main"
"Can't create database 'main'; database exists (errno 1007) (sqlstate HY000)"
"Can't create database 'main'; database exists"

# create db if not exists main
"create database if not exists main"
Expand All @@ -124,7 +124,7 @@

# drop db foo
"drop database foo"
"Can't drop database 'foo'; database doesn't exists (errno 1008) (sqlstate HY000)"
"Can't drop database 'foo'; database doesn't exists"

# drop db main
"drop database main"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -568,4 +568,4 @@

# Rename table with change in keyspace name
"rename table user_extra to main.b"
"Changing schema from 'user' to 'main' is not allowed (errno 1450) (sqlstate HY000)"
"Changing schema from 'user' to 'main' is not allowed"
2 changes: 1 addition & 1 deletion go/vt/vtgate/scatter_conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ func (stc *ScatterConn) ExecuteMultiShard(
)

if !ignoreMaxMemoryRows && len(qr.Rows) > *maxMemoryRows {
return nil, []error{mysql.NewSQLError(mysql.ERNetPacketTooLarge, "", "in-memory row count exceeded allowed limit of %d", *maxMemoryRows)}
return nil, []error{vterrors.NewErrorf(vtrpcpb.Code_RESOURCE_EXHAUSTED, vterrors.NetPacketTooLarge, "in-memory row count exceeded allowed limit of %d", *maxMemoryRows)}
}

return qr, allErrors.GetErrors()
Expand Down
5 changes: 3 additions & 2 deletions go/vt/vtgate/semantics/semantic_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ limitations under the License.
package semantics

import (
"vitess.io/vitess/go/mysql"
vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc"
"vitess.io/vitess/go/vt/vterrors"

"vitess.io/vitess/go/vt/sqlparser"
)
Expand Down Expand Up @@ -85,7 +86,7 @@ func newScope(parent *scope) *scope {
func (s *scope) addTable(name string, table *sqlparser.AliasedTableExpr) error {
_, found := s.tables[name]
if found {
return mysql.NewSQLError(mysql.ERNonUniqTable, mysql.SSClientError, "Not unique table/alias: '%s'", name)
return vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.NonUniqTable, "Not unique table/alias: '%s'", name)
}
s.tables[name] = table
return nil
Expand Down
4 changes: 1 addition & 3 deletions go/vt/vtgate/vcursor_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ import (

"golang.org/x/sync/errgroup"

"vitess.io/vitess/go/mysql"

"vitess.io/vitess/go/vt/callerid"
vschemapb "vitess.io/vitess/go/vt/proto/vschema"
"vitess.io/vitess/go/vt/schema"
Expand Down Expand Up @@ -497,7 +495,7 @@ func (vc *vcursorImpl) SetTarget(target string) error {
return err
}
if _, ok := vc.vschema.Keyspaces[keyspace]; !ignoreKeyspace(keyspace) && !ok {
return mysql.NewSQLError(mysql.ERBadDb, mysql.SSClientError, "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 {
Expand Down

0 comments on commit dfbf58d

Please sign in to comment.