From 0e789598383791fa5838c01927e789df092d71a3 Mon Sep 17 00:00:00 2001 From: shenli Date: Mon, 21 Sep 2015 10:47:05 +0800 Subject: [PATCH 1/3] *: Remove leading comment when check IsQuery IsQuery check the prefix of sql text, so we need to remove comments. --- parser/parser_test.go | 16 +++++++++++++--- tidb.go | 17 ++++++++++++++++- tidb_test.go | 14 ++++++++++++++ 3 files changed, 43 insertions(+), 4 deletions(-) diff --git a/parser/parser_test.go b/parser/parser_test.go index 1603edf55e23d..4789a986849cb 100644 --- a/parser/parser_test.go +++ b/parser/parser_test.go @@ -335,19 +335,21 @@ func (s *testParserSuite) TestParser0(c *C) { // For select with where clause {"SELECT * FROM t WHERE 1 = 1", true}, + + // For comment in query + {"/*comment*/ /*comment*/ select c /* this is a comment */ from t;", true}, } for _, t := range table { - fmt.Printf("%s\n", t.src) l := NewLexer(t.src) ok := yyParse(l) == 0 c.Assert(ok, Equals, t.ok, Commentf("source %v", t.src)) switch ok { case true: - c.Assert(len(l.errs), Equals, 0) + c.Assert(len(l.errs), Equals, 0, Commentf("src: %s", t.src)) case false: - c.Assert(len(l.errs), Not(Equals), 0) + c.Assert(len(l.errs), Not(Equals), 0, Commentf("src: %s", t.src)) } } @@ -391,4 +393,12 @@ func (s *testParserSuite) TestParser0(c *C) { cv, ok := ss.Fields[0].Expr.(*expressions.FunctionCast) c.Assert(ok, IsTrue) c.Assert(cv.FunctionType, Equals, expressions.ConvertFunction) + + // For query start with comment + src = "/* some comments */ /*comment*/ SELECT /*comment*/ CONVERT('111', /*comment*/ SIGNED) /*comment*/;" + l = NewLexer(src) + c.Assert(yyParse(l), Equals, 0) + st = l.Stmts()[0] + ss, ok = st.(*stmts.SelectStmt) + c.Assert(ok, IsTrue) } diff --git a/tidb.go b/tidb.go index c55dc9df311d7..b3c9f0458b5fb 100644 --- a/tidb.go +++ b/tidb.go @@ -243,9 +243,24 @@ func NewStore(uri string) (kv.Storage, error) { var queryStmtTable = []string{"explain", "select", "show", "execute", "describe", "desc"} +func trimSQL(sql string) string { + // Trim space. + sql = strings.TrimSpace(sql) + // Trim leading /*comment*/ + // There may be multiple comments + for strings.HasPrefix(sql, "/*") { + i := strings.Index(sql, "*/") + if i != -1 && i < len(sql)+1 { + sql = sql[i+2:] + } + sql = strings.TrimSpace(sql) + } + return sql +} + // IsQuery checks if a sql statement is a query statement. func IsQuery(sql string) bool { - sqlText := strings.ToLower(strings.TrimSpace(sql)) + sqlText := strings.ToLower(trimSQL(sql)) for _, key := range queryStmtTable { if strings.HasPrefix(sqlText, key) { return true diff --git a/tidb_test.go b/tidb_test.go index 06f0b317759dc..b4c1e26026712 100644 --- a/tidb_test.go +++ b/tidb_test.go @@ -292,6 +292,20 @@ func (s *testMainSuite) TestCheckArgs(c *C) { "abc", []byte("abc"), time.Now(), time.Hour, time.Local) } +func (s *testMainSuite) TestIsQuery(c *C) { + tbl := []struct { + sql string + ok bool + }{ + {"/*comment*/ select 1;", true}, + {"/*comment*/ /*comment*/ select 1;", true}, + {"select /*comment*/ 1 /*comment*/;", true}, + } + for _, t := range tbl { + c.Assert(IsQuery(t.sql), Equals, t.ok, Commentf(t.sql)) + } +} + func sessionExec(c *C, se Session, sql string) ([]rset.Recordset, error) { se.Execute("BEGIN;") r, err := se.Execute(sql) From e9ced8065a98c3e6e2288a79b1517213db0c2e40 Mon Sep 17 00:00:00 2001 From: shenli Date: Mon, 21 Sep 2015 11:17:47 +0800 Subject: [PATCH 2/3] *: Address comments --- parser/parser_test.go | 22 +++++++++++++++------- tidb_test.go | 14 ++++++++++++++ 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/parser/parser_test.go b/parser/parser_test.go index 4789a986849cb..04a4ecad2a81c 100644 --- a/parser/parser_test.go +++ b/parser/parser_test.go @@ -347,7 +347,7 @@ func (s *testParserSuite) TestParser0(c *C) { switch ok { case true: - c.Assert(len(l.errs), Equals, 0, Commentf("src: %s", t.src)) + c.Assert(l.errs, HasLen, 0, Commentf("src: %s", t.src)) case false: c.Assert(len(l.errs), Not(Equals), 0, Commentf("src: %s", t.src)) } @@ -395,10 +395,18 @@ func (s *testParserSuite) TestParser0(c *C) { c.Assert(cv.FunctionType, Equals, expressions.ConvertFunction) // For query start with comment - src = "/* some comments */ /*comment*/ SELECT /*comment*/ CONVERT('111', /*comment*/ SIGNED) /*comment*/;" - l = NewLexer(src) - c.Assert(yyParse(l), Equals, 0) - st = l.Stmts()[0] - ss, ok = st.(*stmts.SelectStmt) - c.Assert(ok, IsTrue) + srcs := []string{ + "/* some comments */ SELECT CONVERT('111', SIGNED) ;", + "/* some comments */ /*comment*/ SELECT CONVERT('111', SIGNED) ;", + "SELECT /*comment*/ CONVERT('111', SIGNED) ;", + "SELECT CONVERT('111', /*comment*/ SIGNED) ;", + "SELECT CONVERT('111', SIGNED) /*comment*/;", + } + for _, src := range srcs { + l = NewLexer(src) + c.Assert(yyParse(l), Equals, 0) + st = l.Stmts()[0] + ss, ok = st.(*stmts.SelectStmt) + c.Assert(ok, IsTrue) + } } diff --git a/tidb_test.go b/tidb_test.go index b4c1e26026712..990356559d57b 100644 --- a/tidb_test.go +++ b/tidb_test.go @@ -306,6 +306,20 @@ func (s *testMainSuite) TestIsQuery(c *C) { } } +func (s *testMainSuite) TestitrimSQL(c *C) { + tbl := []struct { + sql string + target string + }{ + {"/*comment*/ select 1; ", "select 1;"}, + {"/*comment*/ /*comment*/ select 1;", "select 1;"}, + {"select /*comment*/ 1 /*comment*/;", "select /*comment*/ 1 /*comment*/;"}, + } + for _, t := range tbl { + c.Assert(trimSQL(t.sql), Equals, t.target, Commentf(t.sql)) + } +} + func sessionExec(c *C, se Session, sql string) ([]rset.Recordset, error) { se.Execute("BEGIN;") r, err := se.Execute(sql) From 6c7a426b2ff6fd93673c56a7b4d9ab0cb5e80df5 Mon Sep 17 00:00:00 2001 From: shenli Date: Mon, 21 Sep 2015 11:25:04 +0800 Subject: [PATCH 3/3] tidb: Fix bug in trimSQL "/*comment select 1;" will be infinit loop --- tidb.go | 4 +++- tidb_test.go | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/tidb.go b/tidb.go index b3c9f0458b5fb..263c775fa6c39 100644 --- a/tidb.go +++ b/tidb.go @@ -252,8 +252,10 @@ func trimSQL(sql string) string { i := strings.Index(sql, "*/") if i != -1 && i < len(sql)+1 { sql = sql[i+2:] + sql = strings.TrimSpace(sql) + continue } - sql = strings.TrimSpace(sql) + break } return sql } diff --git a/tidb_test.go b/tidb_test.go index 990356559d57b..c9156e7a35bf7 100644 --- a/tidb_test.go +++ b/tidb_test.go @@ -314,6 +314,7 @@ func (s *testMainSuite) TestitrimSQL(c *C) { {"/*comment*/ select 1; ", "select 1;"}, {"/*comment*/ /*comment*/ select 1;", "select 1;"}, {"select /*comment*/ 1 /*comment*/;", "select /*comment*/ 1 /*comment*/;"}, + {"/*comment select 1; ", "/*comment select 1;"}, } for _, t := range tbl { c.Assert(trimSQL(t.sql), Equals, t.target, Commentf(t.sql))