Skip to content

Make StatementType a type alias instead of an int #5259

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sougou merged 1 commit into from
Oct 5, 2019
Merged

Make StatementType a type alias instead of an int #5259

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go/trace/trace.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func NewFromString(inCtx context.Context, parent, label string) (Span, context.C
// AnnotateSQL annotates information about a sql query in the span. This is done in a way
// so as to not leak personally identifying information (PII), or sensitive personal information (SPI)
func AnnotateSQL(span Span, sql string) {
span.Annotate("sql-statement-type", sqlparser.StmtType(sqlparser.Preview(sql)))
span.Annotate("sql-statement-type", sqlparser.Preview(sql).String())
}

// FromContext returns the Span from a Context if present. The bool return
Expand Down
12 changes: 7 additions & 5 deletions go/vt/sqlparser/analyzer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,13 @@ import (
vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc"
)

// StatementType encodes the type of a SQL statement
type StatementType int

// These constants are used to identify the SQL statement type.
// Changing this list will require reviewing all calls to Preview.
const (
StmtSelect = iota
StmtSelect StatementType = iota
StmtStream
StmtInsert
StmtReplace
Expand All @@ -53,7 +56,7 @@ const (

// Preview analyzes the beginning of the query using a simpler and faster
// textual comparison to identify the statement type.
func Preview(sql string) int {
func Preview(sql string) StatementType {
trimmed := StripLeadingComments(sql)

if strings.Index(trimmed, "/*!") == 0 {
Expand Down Expand Up @@ -111,9 +114,8 @@ func Preview(sql string) int {
return StmtUnknown
}

// StmtType returns the statement type as a string
func StmtType(stmtType int) string {
switch stmtType {
func (s StatementType) String() string {
switch s {
Copy link
Contributor

@sougou sougou Oct 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should ideally do an array lookup instead.

case StmtSelect:
return "SELECT"
case StmtStream:
Expand Down
2 changes: 1 addition & 1 deletion go/vt/sqlparser/analyzer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (
func TestPreview(t *testing.T) {
testcases := []struct {
sql string
want int
want StatementType
}{
{"select ...", StmtSelect},
{" select ...", StmtSelect},
Expand Down
8 changes: 4 additions & 4 deletions go/vt/vtgate/executor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ func (e *Executor) execute(ctx context.Context, safeSession *SafeSession, sql st
}

stmtType := sqlparser.Preview(sql)
logStats.StmtType = sqlparser.StmtType(stmtType)
logStats.StmtType = stmtType.String()

// Mysql warnings are scoped to the current session, but are
// cleared when a "non-diagnostic statement" is executed:
Expand Down Expand Up @@ -1126,7 +1126,7 @@ func (e *Executor) handleComment(sql string) (*sqltypes.Result, error) {
// StreamExecute executes a streaming query.
func (e *Executor) StreamExecute(ctx context.Context, method string, safeSession *SafeSession, sql string, bindVars map[string]*querypb.BindVariable, target querypb.Target, callback func(*sqltypes.Result) error) (err error) {
logStats := NewLogStats(ctx, method, sql, bindVars)
logStats.StmtType = sqlparser.StmtType(sqlparser.Preview(sql))
logStats.StmtType = sqlparser.Preview(sql).String()
defer logStats.Send()

if bindVars == nil {
Expand All @@ -1137,7 +1137,7 @@ func (e *Executor) StreamExecute(ctx context.Context, method string, safeSession

// check if this is a stream statement for messaging
// TODO: support keyRange syntax
if logStats.StmtType == sqlparser.StmtType(sqlparser.StmtStream) {
if logStats.StmtType == sqlparser.StmtStream.String() {
return e.handleMessageStream(ctx, safeSession, sql, target, callback, vcursor, logStats)
}

Expand Down Expand Up @@ -1521,7 +1521,7 @@ func (e *Executor) prepare(ctx context.Context, safeSession *SafeSession, sql st
}

stmtType := sqlparser.Preview(sql)
logStats.StmtType = sqlparser.StmtType(stmtType)
logStats.StmtType = stmtType.String()

// Mysql warnings are scoped to the current session, but are
// cleared when a "non-diagnostic statement" is executed:
Expand Down
2 changes: 1 addition & 1 deletion go/vt/vttablet/tabletserver/vstreamer/vstreamer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ func (vs *vstreamer) parseEvent(ev mysql.BinlogEvent) ([]*binlogdatapb.VEvent, e
case sqlparser.StmtOther:
// These are DBA statements like REPAIR that can be ignored.
default:
return nil, fmt.Errorf("unexpected statement type %s in row-based replication: %q", sqlparser.StmtType(cat), q.SQL)
return nil, fmt.Errorf("unexpected statement type %s in row-based replication: %q", cat, q.SQL)
}
case ev.IsTableMap():
// This is very frequent. It precedes every row event.
Expand Down