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.

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

Jump to bottom

Fix Vtexplains StripComments #5984

Merged
merged 4 commits into from
Apr 2, 2020
Merged

Fix Vtexplains StripComments #5984

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1ddeee0
Fix Vtexplains StripComments
saifalharthi Mar 27, 2020
e907bdb
Use Parser to handle comments if parseable
saifalharthi Mar 30, 2020
9b2f210
Try to use SplitMarginComments instead of having StripComments() sepa…
systay Apr 1, 2020
309c888
Remove Partition comment from test to match current behaviour for Vitess
saifalharthi Apr 1, 2020
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
28 changes: 3 additions & 25 deletions go/vt/sqlparser/comments.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func leadingCommentEnd(text string) (end int) {

// Found visible characters. Look for '/*' at the beginning
// and '*/' somewhere after that.
if len(remainingText) < 4 || remainingText[:2] != "/*" {
if len(remainingText) < 4 || remainingText[:2] != "/*" || remainingText[2] == '!' {
break
}
commentLength := 4 + strings.Index(remainingText[2:], "*/")
Expand Down Expand Up @@ -93,8 +93,8 @@ func trailingCommentStart(text string) (start int) {

// Find the beginning of the comment
startCommentPos := strings.LastIndex(text[:reducedLen-2], "/*")
if startCommentPos < 0 {
// Badly formatted sql :/
if startCommentPos < 0 || text[startCommentPos+2] == '!' {
// Badly formatted sql, or a special /*! comment
break
}

Expand Down Expand Up @@ -164,28 +164,6 @@ func hasCommentPrefix(sql string) bool {
return len(sql) > 1 && ((sql[0] == '/' && sql[1] == '*') || (sql[0] == '-' && sql[1] == '-'))
}

// StripComments removes all comments from the string regardless
// of where they occur
func StripComments(sql string) string {
sql = StripLeadingComments(sql) // handle -- or /* ... */ at the beginning

for {
start := strings.Index(sql, "/*")
if start == -1 {
break
}
end := strings.Index(sql, "*/")
if end <= 1 {
break
}
sql = sql[:start] + sql[end+2:]
}

sql = strings.TrimFunc(sql, unicode.IsSpace)

return sql
}

// ExtractMysqlComment extracts the version and SQL from a comment-only query
// such as /*!50708 sql here */
func ExtractMysqlComment(sql string) (version string, innerSQL string) {
Expand Down
113 changes: 23 additions & 90 deletions go/vt/sqlparser/comments_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,20 +119,32 @@ func TestSplitComments(t *testing.T) {
outSQL: "foo",
outLeadingComments: "",
outTrailingComments: "",
}, {
input: "select 1 from t where col = '*//*'",
outSQL: "select 1 from t where col = '*//*'",
outLeadingComments: "",
outTrailingComments: "",
}, {
input: "/*! select 1 */",
outSQL: "/*! select 1 */",
outLeadingComments: "",
outTrailingComments: "",
}}
for _, testCase := range testCases {
gotSQL, gotComments := SplitMarginComments(testCase.input)
gotLeadingComments, gotTrailingComments := gotComments.Leading, gotComments.Trailing
t.Run(testCase.input, func(t *testing.T) {
gotSQL, gotComments := SplitMarginComments(testCase.input)
gotLeadingComments, gotTrailingComments := gotComments.Leading, gotComments.Trailing

if gotSQL != testCase.outSQL {
t.Errorf("test input: '%s', got SQL\n%+v, want\n%+v", testCase.input, gotSQL, testCase.outSQL)
}
if gotLeadingComments != testCase.outLeadingComments {
t.Errorf("test input: '%s', got LeadingComments\n%+v, want\n%+v", testCase.input, gotLeadingComments, testCase.outLeadingComments)
}
if gotTrailingComments != testCase.outTrailingComments {
t.Errorf("test input: '%s', got TrailingComments\n%+v, want\n%+v", testCase.input, gotTrailingComments, testCase.outTrailingComments)
}
if gotSQL != testCase.outSQL {
t.Errorf("test input: '%s', got SQL\n%+v, want\n%+v", testCase.input, gotSQL, testCase.outSQL)
}
if gotLeadingComments != testCase.outLeadingComments {
t.Errorf("test input: '%s', got LeadingComments\n%+v, want\n%+v", testCase.input, gotLeadingComments, testCase.outLeadingComments)
}
if gotTrailingComments != testCase.outTrailingComments {
t.Errorf("test input: '%s', got TrailingComments\n%+v, want\n%+v", testCase.input, gotTrailingComments, testCase.outTrailingComments)
}
})
}
}

Expand Down Expand Up @@ -212,85 +224,6 @@ a`,
}
}

func TestRemoveComments(t *testing.T) {
var testCases = []struct {
input, outSQL string
}{{
input: "/",
outSQL: "/",
}, {
input: "*/",
outSQL: "*/",
}, {
input: "/*/",
outSQL: "/*/",
}, {
input: "/*a",
outSQL: "/*a",
}, {
input: "/*a*",
outSQL: "/*a*",
}, {
input: "/*a**",
outSQL: "/*a**",
}, {
input: "/*b**a*/",
outSQL: "",
}, {
input: "/*a*/",
outSQL: "",
}, {
input: "/**/",
outSQL: "",
}, {
input: "/*!*/",
outSQL: "",
}, {
input: "/*!a*/",
outSQL: "",
}, {
input: "/*b*/ /*a*/",
outSQL: "",
}, {
input: `/*b*/ --foo
bar`,
outSQL: "bar",
}, {
input: "foo /* bar */",
outSQL: "foo",
}, {
input: "foo /* bar */ baz",
outSQL: "foo baz",
}, {
input: "/* foo */ bar",
outSQL: "bar",
}, {
input: "-- /* foo */ bar",
outSQL: "",
}, {
input: "foo -- bar */",
outSQL: "foo -- bar */",
}, {
input: `/*
foo */ bar`,
outSQL: "bar",
}, {
input: `-- foo bar
a`,
outSQL: "a",
}, {
input: `-- foo bar`,
outSQL: "",
}}
for _, testCase := range testCases {
gotSQL := StripComments(testCase.input)

if gotSQL != testCase.outSQL {
t.Errorf("test input: '%s', got SQL\n%+v, want\n%+v", testCase.input, gotSQL, testCase.outSQL)
}
}
}

func TestExtractMysqlComment(t *testing.T) {
var testCases = []struct {
input, outSQL, outVersion string
Expand Down
6 changes: 0 additions & 6 deletions go/vt/vtexplain/testdata/test-schema.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,4 @@ create table test_partitioned (
date_create int,
primary key(id)
) Engine=InnoDB
/*!50100 PARTITION BY RANGE (date_create)
(PARTITION p2018_06_14 VALUES LESS THAN (1528959600) ENGINE = InnoDB,
PARTITION p2018_06_15 VALUES LESS THAN (1529046000) ENGINE = InnoDB,
PARTITION p2018_06_16 VALUES LESS THAN (1529132400) ENGINE = InnoDB,
PARTITION p2018_06_17 VALUES LESS THAN (1529218800) ENGINE = InnoDB)
*/
;
2 changes: 1 addition & 1 deletion go/vt/vtexplain/vtexplain.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ func parseSchema(sqlSchema string, opts *Options) ([]*sqlparser.DDL, error) {
if sql == "" {
break
}
sql = sqlparser.StripComments(sql)
sql, _ = sqlparser.SplitMarginComments(sql)
if sql == "" {
continue
}
Expand Down
2 changes: 1 addition & 1 deletion go/vt/vtexplain/vtexplain_flaky_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func initTest(mode string, opts *Options, t *testing.T) {

opts.ExecutionMode = mode
err = Init(string(vSchema), string(schema), opts)
require.NoError(t, err, "vtexplain Init error")
require.NoError(t, err, "vtexplain Init error\n%s", string(schema))
}

func testExplain(testcase string, opts *Options, t *testing.T) {
Expand Down