From 8fa78391fb1b05469018460fb3698f233872494a Mon Sep 17 00:00:00 2001 From: Rafael Chacon Date: Wed, 31 Jan 2018 13:22:19 -0800 Subject: [PATCH 1/5] Add support for trailing comments in begin/commit/rollback statements --- go/vt/sqlparser/analyzer.go | 3 ++- go/vt/sqlparser/analyzer_test.go | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/go/vt/sqlparser/analyzer.go b/go/vt/sqlparser/analyzer.go index 97bd8aeaa80..062f64394a1 100644 --- a/go/vt/sqlparser/analyzer.go +++ b/go/vt/sqlparser/analyzer.go @@ -52,6 +52,7 @@ const ( // textual comparison to identify the statement type. func Preview(sql string) int { trimmed := StripLeadingComments(sql) + trimmedNoComments, _ := SplitTrailingComments(trimmed) firstWord := trimmed if end := strings.IndexFunc(trimmed, unicode.IsSpace); end != -1 { @@ -72,7 +73,7 @@ func Preview(sql string) int { case "delete": return StmtDelete } - switch strings.ToLower(trimmed) { + switch strings.ToLower(trimmedNoComments) { case "begin", "start transaction": return StmtBegin case "commit": diff --git a/go/vt/sqlparser/analyzer_test.go b/go/vt/sqlparser/analyzer_test.go index c8f6ec0ed3d..c50955d2d47 100644 --- a/go/vt/sqlparser/analyzer_test.go +++ b/go/vt/sqlparser/analyzer_test.go @@ -45,9 +45,13 @@ func TestPreview(t *testing.T) { {"\n\t begin ", StmtBegin}, {"... begin ", StmtUnknown}, {"begin ...", StmtUnknown}, + {"begin /* ... */", StmtBegin}, + {"begin /* ... *//*test*/", StmtBegin}, {"start transaction", StmtBegin}, {"commit", StmtCommit}, + {"commit /*...*/", StmtCommit}, {"rollback", StmtRollback}, + {"rollback /*...*/", StmtRollback}, {"create", StmtDDL}, {"alter", StmtDDL}, {"rename", StmtDDL}, From e3baf62030b979f373cbd17c4928bd145b7b0d99 Mon Sep 17 00:00:00 2001 From: Rafael Chacon Date: Wed, 31 Jan 2018 20:29:59 -0800 Subject: [PATCH 2/5] nit - only get trimmed comments when needed --- go/vt/sqlparser/analyzer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/vt/sqlparser/analyzer.go b/go/vt/sqlparser/analyzer.go index 062f64394a1..f0a92d4dd23 100644 --- a/go/vt/sqlparser/analyzer.go +++ b/go/vt/sqlparser/analyzer.go @@ -52,7 +52,6 @@ const ( // textual comparison to identify the statement type. func Preview(sql string) int { trimmed := StripLeadingComments(sql) - trimmedNoComments, _ := SplitTrailingComments(trimmed) firstWord := trimmed if end := strings.IndexFunc(trimmed, unicode.IsSpace); end != -1 { @@ -73,6 +72,7 @@ func Preview(sql string) int { case "delete": return StmtDelete } + trimmedNoComments, _ := SplitTrailingComments(trimmed) switch strings.ToLower(trimmedNoComments) { case "begin", "start transaction": return StmtBegin From faf02dce2b64d11d8335b2e5fe5c8734fd8d1244 Mon Sep 17 00:00:00 2001 From: Rafael Chacon Date: Wed, 31 Jan 2018 20:49:28 -0800 Subject: [PATCH 3/5] Added comment per code review --- go/vt/sqlparser/analyzer.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/go/vt/sqlparser/analyzer.go b/go/vt/sqlparser/analyzer.go index f0a92d4dd23..52a369e5a7a 100644 --- a/go/vt/sqlparser/analyzer.go +++ b/go/vt/sqlparser/analyzer.go @@ -72,6 +72,11 @@ func Preview(sql string) int { case "delete": return StmtDelete } + // For the following statements, is not enough to just check + // loweredFirstWord. This is because they are not statements + // in the grammar and we are relying in Preview to parse them. + // For instance, we don't want: "BEGIN ..." to be parsed + // as StmtBegin. trimmedNoComments, _ := SplitTrailingComments(trimmed) switch strings.ToLower(trimmedNoComments) { case "begin", "start transaction": From bdc63720c16806814c130317f664a32cce811339 Mon Sep 17 00:00:00 2001 From: Rafael Chacon Date: Wed, 31 Jan 2018 20:50:23 -0800 Subject: [PATCH 4/5] nit - improve comment --- go/vt/sqlparser/analyzer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/vt/sqlparser/analyzer.go b/go/vt/sqlparser/analyzer.go index 52a369e5a7a..5e2fe28aada 100644 --- a/go/vt/sqlparser/analyzer.go +++ b/go/vt/sqlparser/analyzer.go @@ -72,7 +72,7 @@ func Preview(sql string) int { case "delete": return StmtDelete } - // For the following statements, is not enough to just check + // For the following statements is not enough to just check // loweredFirstWord. This is because they are not statements // in the grammar and we are relying in Preview to parse them. // For instance, we don't want: "BEGIN ..." to be parsed From 8d0e167fc2779e611215039c17b10f8cbf84d1cb Mon Sep 17 00:00:00 2001 From: Rafael Chacon Date: Wed, 31 Jan 2018 21:06:54 -0800 Subject: [PATCH 5/5] Rewrote comment to make it clearer --- go/vt/sqlparser/analyzer.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/go/vt/sqlparser/analyzer.go b/go/vt/sqlparser/analyzer.go index 5e2fe28aada..e76e6ead0e2 100644 --- a/go/vt/sqlparser/analyzer.go +++ b/go/vt/sqlparser/analyzer.go @@ -72,10 +72,10 @@ func Preview(sql string) int { case "delete": return StmtDelete } - // For the following statements is not enough to just check - // loweredFirstWord. This is because they are not statements - // in the grammar and we are relying in Preview to parse them. - // For instance, we don't want: "BEGIN ..." to be parsed + // For the following statements it is not sufficient to rely + // on loweredFirstWord. This is because they are not statements + // in the grammar and we are relying on Preview to parse them. + // For instance, we don't want: "BEGIN JUNK" to be parsed // as StmtBegin. trimmedNoComments, _ := SplitTrailingComments(trimmed) switch strings.ToLower(trimmedNoComments) {