You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by GitBox <gi...@apache.org> on 2020/05/21 18:05:34 UTC
[GitHub] [cassandra] rgroothuijsen opened a new pull request #598: CASSANDRA-15802: Strip comment blocks from cqlsh input before processing statements
rgroothuijsen opened a new pull request #598:
URL: https://github.com/apache/cassandra/pull/598
It has been reported that including a semicolon in a CQL comment block causes an error, since cqlsh does not discern whether it is currently inside a comment block while preparing statements. As a result, the comment is parsed up to the semicolon and executed as if it were a statement.
The most straightforward solution, implemented in this PR, would be to simply remove everything inside a comment block from the input rather than keeping it as CQL to be parsed.
An alternative way would have been to restructure the parsing so that the comments are preserved, but this would be more complex and, to my knowledge, these comments would be ignored all the same.
The method implemented here removes, in order:
- Comment blocks that start and end on the same line
- The start of a comment block and anything following it
- The end of a comment block and anything preceding it
- Any line that is marked as being inside a comment block, but not having any start/end markers
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] rgroothuijsen commented on a change in pull request #598: CASSANDRA-15802: Strip comment blocks from cqlsh input before processing statements
Posted by GitBox <gi...@apache.org>.
rgroothuijsen commented on a change in pull request #598:
URL: https://github.com/apache/cassandra/pull/598#discussion_r471142974
##########
File path: pylib/cqlshlib/test/test_cql_parsing.py
##########
@@ -711,6 +711,14 @@ def test_parse_drop_index(self):
def test_parse_select_token(self):
pass
+ def test_strip_comment_blocks_from_input(self):
+ parsed = parse_cqlsh_statements('SELECT FROM /* \n comment block starts here; \n and continues here \n */ "MyTable";')
+ self.assertSequenceEqual(tokens_with_types(parsed),
+ [('SELECT', 'reserved_identifier'),
+ ('FROM', 'reserved_identifier'),
+ ('"MyTable"', 'quotedName'),
+ (';', 'endtoken')])
Review comment:
Good idea, thanks for the help in writing them out as well.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] michaelsembwever commented on a change in pull request #598: CASSANDRA-15802: Strip comment blocks from cqlsh input before processing statements
Posted by GitBox <gi...@apache.org>.
michaelsembwever commented on a change in pull request #598:
URL: https://github.com/apache/cassandra/pull/598#discussion_r471123210
##########
File path: pylib/cqlshlib/test/test_cql_parsing.py
##########
@@ -711,6 +711,14 @@ def test_parse_drop_index(self):
def test_parse_select_token(self):
pass
+ def test_strip_comment_blocks_from_input(self):
+ parsed = parse_cqlsh_statements('SELECT FROM /* \n comment block starts here; \n and continues here \n */ "MyTable";')
+ self.assertSequenceEqual(tokens_with_types(parsed),
+ [('SELECT', 'reserved_identifier'),
+ ('FROM', 'reserved_identifier'),
+ ('"MyTable"', 'quotedName'),
+ (';', 'endtoken')])
Review comment:
it would be great to ensure the different variations the code covers is tested, for example…
```suggestion
parsed = parse_cqlsh_statements('SELECT FROM /* comment block */ "MyTable";')
self.assertSequenceEqual(tokens_with_types(parsed),
[('SELECT', 'reserved_identifier'),
('FROM', 'reserved_identifier'),
('"MyTable"', 'quotedName'),
(';', 'endtoken')])
parsed = parse_cqlsh_statements('SELECT FROM /* \n comment block starts here; \n and continues here \n */ "MyTable";')
self.assertSequenceEqual(tokens_with_types(parsed),
[('SELECT', 'reserved_identifier'),
('FROM', 'reserved_identifier'),
('"MyTable"', 'quotedName'),
(';', 'endtoken')])
parsed = parse_cqlsh_statements('''
SELECT FROM /*
comment block starts here;
and continues here
*/ "MyTable";
''')
self.assertSequenceEqual(tokens_with_types(parsed),
[('SELECT', 'reserved_identifier'),
('FROM', 'reserved_identifier'),
('"MyTable"', 'quotedName'),
(';', 'endtoken')])
parsed = parse_cqlsh_statements('''
/* comment block */
SELECT FROM "MyTable";
''')
self.assertSequenceEqual(tokens_with_types(parsed),
[('SELECT', 'reserved_identifier'),
('FROM', 'reserved_identifier'),
('"MyTable"', 'quotedName'),
(';', 'endtoken')])
parsed = parse_cqlsh_statements('''
/* comment block */
/* another comment */ SELECT FROM /*
comment block starts here;
and continues here
*/ "MyTable";
''')
self.assertSequenceEqual(tokens_with_types(parsed),
[('SELECT', 'reserved_identifier'),
('FROM', 'reserved_identifier'),
('"MyTable"', 'quotedName'),
(';', 'endtoken')])
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] michaelsembwever commented on a change in pull request #598: CASSANDRA-15802: Strip comment blocks from cqlsh input before processing statements
Posted by GitBox <gi...@apache.org>.
michaelsembwever commented on a change in pull request #598:
URL: https://github.com/apache/cassandra/pull/598#discussion_r473803909
##########
File path: bin/cqlsh.py
##########
@@ -906,12 +907,30 @@ def cmdloop(self):
self.reset_statement()
print('')
+ def strip_comment_blocks(self, statementtext):
+ comment_block_in_literal_string = re.search('["].*[/][*].*[*][/].*["]', statementtext)
+ if not comment_block_in_literal_string:
+ result = re.sub('[/][*].*[*][/]', "", statementtext)
+ if '*/' in result and not '/*' in result and not self.in_comment:
Review comment:
this failed the pycodestyle test: https://ci-cassandra.apache.org/job/Cassandra-devbranch/254/testReport/junit/dtest.cqlsh_tests.test_cqlsh/TestCqlsh/test_pycodestyle_compliance/
have fixed it here: https://github.com/thelastpickle/cassandra/commit/b51117fb57948d9b4f67ae9f43cb7e88e4ec1f3f
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] rgroothuijsen commented on a change in pull request #598: CASSANDRA-15802: Strip comment blocks from cqlsh input before processing statements
Posted by GitBox <gi...@apache.org>.
rgroothuijsen commented on a change in pull request #598:
URL: https://github.com/apache/cassandra/pull/598#discussion_r471177344
##########
File path: pylib/cqlshlib/test/test_cql_parsing.py
##########
@@ -711,6 +711,14 @@ def test_parse_drop_index(self):
def test_parse_select_token(self):
pass
+ def test_strip_comment_blocks_from_input(self):
+ parsed = parse_cqlsh_statements('SELECT FROM /* \n comment block starts here; \n and continues here \n */ "MyTable";')
+ self.assertSequenceEqual(tokens_with_types(parsed),
+ [('SELECT', 'reserved_identifier'),
+ ('FROM', 'reserved_identifier'),
+ ('"MyTable"', 'quotedName'),
+ (';', 'endtoken')])
Review comment:
I realize I did mess up the commit message when I accepted your changes. Will that be okay, or is it worth rebasing my branch?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] smiklosovic closed pull request #598: CASSANDRA-15802: Strip comment blocks from cqlsh input before processing statements
Posted by GitBox <gi...@apache.org>.
smiklosovic closed pull request #598:
URL: https://github.com/apache/cassandra/pull/598
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org