Skip to content
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.

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

ignore mysql-specific comment statements #3576

Merged
merged 1 commit into from
Jan 23, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions go/vt/sqlparser/analyzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ const (
StmtUse
StmtOther
StmtUnknown
StmtComment
)

// Preview analyzes the beginning of the query using a simpler and faster
Expand Down Expand Up @@ -91,6 +92,9 @@ func Preview(sql string) int {
case "analyze", "describe", "desc", "explain", "repair", "optimize", "truncate":
return StmtOther
}
if strings.Index(trimmed, "/*!") == 0 {
return StmtComment
}
return StmtUnknown
}

Expand Down
2 changes: 2 additions & 0 deletions go/vt/sqlparser/analyzer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ func TestPreview(t *testing.T) {

{"/* leading comment */ select ...", StmtSelect},
{"/* leading comment */ /* leading comment 2 */ select ...", StmtSelect},
{"/*! MySQL-specific comment */", StmtComment},
{"/*!50708 MySQL-version comment */", StmtComment},
{"-- leading single line comment \n select ...", StmtSelect},
{"-- leading single line comment \n -- leading single line comment 2\n select ...", StmtSelect},

Expand Down
16 changes: 16 additions & 0 deletions go/vt/sqlparser/comments.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,10 @@ func StripLeadingComments(sql string) string {
if index <= 1 {
return sql
}
// don't strip /*! ... */ or /*!50700 ... */
if len(sql) > 2 && sql[2] == '!' {
return sql
}
sql = sql[index+2:]
case '-':
// Single line comment
Expand All @@ -140,3 +144,15 @@ func StripLeadingComments(sql string) string {
func hasCommentPrefix(sql string) bool {
return len(sql) > 1 && ((sql[0] == '/' && sql[1] == '*') || (sql[0] == '-' && sql[1] == '-'))
}

// ExtractMysqlComment extracts the version and SQL from a comment-only query
// such as /*!50708 sql here */
func ExtractMysqlComment(sql string) (version string, innerSQL string) {
sql = sql[3 : len(sql)-2]

endOfVersionIndex := strings.IndexFunc(sql, func(c rune) bool { return !unicode.IsDigit(c) })
version = sql[0:endOfVersionIndex]
innerSQL = strings.TrimFunc(sql[endOfVersionIndex:], unicode.IsSpace)

return version, innerSQL
}
34 changes: 34 additions & 0 deletions go/vt/sqlparser/comments_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,12 @@ func TestStripLeadingComments(t *testing.T) {
}, {
input: "/**/",
outSQL: "",
}, {
input: "/*!*/",
outSQL: "/*!*/",
}, {
input: "/*!a*/",
outSQL: "/*!a*/",
}, {
input: "/*b*/ /*a*/",
outSQL: "",
Expand Down Expand Up @@ -167,3 +173,31 @@ a`,
}
}
}

func TestExtractMysqlComment(t *testing.T) {
var testCases = []struct {
input, outSQL, outVersion string
}{{
input: "/*!50708SET max_execution_time=5000 */",
outSQL: "SET max_execution_time=5000",
outVersion: "50708",
}, {
input: "/*!50708 SET max_execution_time=5000*/",
outSQL: "SET max_execution_time=5000",
outVersion: "50708",
}, {
input: "/*! SET max_execution_time=5000*/",
outSQL: "SET max_execution_time=5000",
outVersion: "",
}}
for _, testCase := range testCases {
gotVersion, gotSQL := ExtractMysqlComment(testCase.input)

if gotVersion != testCase.outVersion {
t.Errorf("test input: '%s', got version\n%+v, want\n%+v", testCase.input, gotVersion, testCase.outVersion)
}
if gotSQL != testCase.outSQL {
t.Errorf("test input: '%s', got SQL\n%+v, want\n%+v", testCase.input, gotSQL, testCase.outSQL)
}
}
}
9 changes: 9 additions & 0 deletions go/vt/vtgate/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,8 @@ func (e *Executor) execute(ctx context.Context, safeSession *SafeSession, sql st
return e.handleUse(ctx, safeSession, sql, bindVars)
case sqlparser.StmtOther:
return e.handleOther(ctx, safeSession, sql, bindVars, target, logStats)
case sqlparser.StmtComment:
return e.handleComment(ctx, safeSession, sql, bindVars, target, logStats)
}
return nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "unrecognized statement: %s", sql)
}
Expand Down Expand Up @@ -705,6 +707,13 @@ func (e *Executor) handleOther(ctx context.Context, safeSession *SafeSession, sq
return result, err
}

func (e *Executor) handleComment(ctx context.Context, safeSession *SafeSession, sql string, bindVars map[string]*querypb.BindVariable, target querypb.Target, logStats *LogStats) (*sqltypes.Result, error) {
_, sql = sqlparser.ExtractMysqlComment(sql)

// Not sure if this is a good idea.
return &sqltypes.Result{}, nil
}

// 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)
Expand Down
20 changes: 20 additions & 0 deletions go/vt/vtgate/executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -734,6 +734,26 @@ func TestExecutorUse(t *testing.T) {
}
}

func TestExecutorComment(t *testing.T) {
executor, _, _, _ := createExecutorEnv()

stmts := []string{
"/*! SET max_execution_time=5000*/",
"/*!50708 SET max_execution_time=5000*/",
}
wantResult := &sqltypes.Result{}

for _, stmt := range stmts {
gotResult, err := executor.Execute(context.Background(), "TestExecute", NewSafeSession(&vtgatepb.Session{TargetString: KsTestUnsharded}), stmt, nil)
if err != nil {
t.Error(err)
}
if !reflect.DeepEqual(gotResult, wantResult) {
t.Errorf("Exec %s: %v, want %v", stmt, gotResult, wantResult)
}
}
}

func TestExecutorOther(t *testing.T) {
executor, sbc1, sbc2, sbclookup := createExecutorEnv()

Expand Down