*: Remove leading comment when check IsQuery #210
Conversation
IsQuery check the prefix of sql text, so we need to remove comments.
| @@ -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 { | |||
There was a problem hiding this comment.
If comments start with -- or # the entire line is comment. Only /**/ can mix with sql statement.
There was a problem hiding this comment.
We need to strip comments start with --
see #199
There was a problem hiding this comment.
what I mean is to check the trimming result, not only IsQuery true or false.
There was a problem hiding this comment.
@ngaut We can handle the sql in #199 now. For sql text start with --, we just need to drop the entire sql text and do nothing else.
➜ interpreter git:(shenli/comment-in-query) ✗ cat xx
/* mysql-connector-java-5.1.35 ( Revision: 5fb9c5849535c13917c2cf9baaece6ef9693ef27 ) */SHOW VARIABLES WHERE Variable_name ='language' OR Variable_name = 'net_write_timeout' OR Variable_name = 'interactive_timeout' OR Variable_name = 'wait_timeout' OR Variable_name = 'character_set_client' OR Variable_name = 'character_set_connection' OR Variable_name = 'character_set' OR Variable_name = 'character_set_server' OR Variable_name = 'tx_isolation' OR Variable_name = 'transaction_isolation' OR Variable_name = 'character_set_results' OR Variable_name = 'timezone' OR Variable_name = 'time_zone' OR Variable_name = 'system_time_zone' OR Variable_name = 'lower_case_table_names' OR Variable_name = 'max_allowed_packet' OR Variable_name = 'net_buffer_length' OR Variable_name = 'sql_mode' OR Variable_name = 'query_cache_type' OR Variable_name = 'query_cache_size' OR Variable_name = 'license' OR Variable_name = 'init_connect';
➜ interpreter git:(shenli/comment-in-query) ✗ cat xx | ./interpreter
Welcome to the TiDB.
Version:
Git Commit Hash: None
UTC Build Time: None
+--------------------------+------------------------+
| Variable_name | Value |
+--------------------------+------------------------+
| character_set_results | latin1 |
| init_connect | |
| character_set_client | latin1 |
| tx_isolation | REPEATABLE-READ |
| interactive_timeout | 28800 |
| max_allowed_packet | 4194304 |
| lower_case_table_names | 2 |
| character_set_connection | latin1 |
| time_zone | SYSTEM |
| sql_mode | NO_ENGINE_SUBSTITUTION |
| query_cache_type | OFF |
| net_write_timeout | 60 |
| system_time_zone | CST |
| query_cache_size | 1048576 |
| net_buffer_length | 16384 |
| license | GPL |
| wait_timeout | 28800 |
| character_set_server | latin1 |
+--------------------------+------------------------+
Bye
There was a problem hiding this comment.
Yes, it is my fault.
SQL like select * /* comment */ from t; is supported now.
Seems we only not support start with /* SQL, like /* comment */ select * from t;
There was a problem hiding this comment.
Looks like mp didn't handle comments start -- correctly
There was a problem hiding this comment.
I test tidb-server and it is ok now.
Database changed
mysql> select /comment/ 1;
+------+
| 1 |
+------+
| 1 |
+------+
1 row in set (0.00 sec)
mysql> /comment/ select 1;
+------+
| 1 |
+------+
| 1 |
+------+
1 row in set (0.00 sec)
mysql>
| @@ -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}, | |||
There was a problem hiding this comment.
It is better to add different types of comments.
like
/* xxx */ select * from t
select * /* xxx */ from t;
select * from t /* xxx */
| i := strings.Index(sql, "*/") | ||
| if i != -1 && i < len(sql)+1 { | ||
| sql = sql[i+2:] | ||
| } |
There was a problem hiding this comment.
If SQL like /* select * from t;, here seems will be an endless loop？
"/*comment select 1;" will be infinit loop
| case false: | ||
| c.Assert(len(l.errs), Not(Equals), 0) | ||
| c.Assert(len(l.errs), Not(Equals), 0, Commentf("src: %s", t.src)) |
|
LGTM |
|
PTAL @ngaut @siddontang |
|
LGTM |
*: Remove leading comment when check IsQuery
Signed-off-by: Lonng <[email protected]>
IsQuery check the prefix of sql text, so we need to remove comments.