You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2020/12/25 03:04:28 UTC
[GitHub] [hive] ujc714 opened a new pull request #1814: HIVE-15820: comment at the head of beeline -e
ujc714 opened a new pull request #1814:
URL: https://github.com/apache/hive/pull/1814
### What changes were proposed in this pull request?
1) Don't check if a line is a comment in Beeline.dispatch(). Instead, remove the comments from the line.
2) Replace removeComments(String, int[]) with removeComments(String) in Commands.handleMultiLineCmd().
### Why are the changes needed?
1) The queries in '-e' parameter is passed to Beeline.dispatch() as a single line although there could be multiple lines. If the first line is a comment, the rest lines are ignored. We should pass the query strings to Commands.execute().
2) HiveStringUtils.removeComments(String, int[]) is used for a single line. In this method. If we use it to check a multiple line string and there is one comment line, the lines after this comment line will be discarded. In fact, HiveStringUtils.removeComments(String) splits a multiple line string to several single line strings and calls HiveStringUtils.removeComments(String, int[]) to process each single line. So HiveStringUtils.removeComments(String) is what we should use in Commands.handleMultiLineCmd().
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
1) org.apache.hive.beeline.testLinesEndingWithComments is used to test HiveStringUtils.removeComments(String).
2) org.apache.hive.beeline.cli.testSqlFromCmdWithComments* are used to test queries passed via '-e' option. And testSqlFromCmdWithComments2 is used to test the query after a comment line.
----------------------------------------------------------------
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: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] nrg4878 commented on a change in pull request #1814: HIVE-15820: comment at the head of beeline -e
Posted by GitBox <gi...@apache.org>.
nrg4878 commented on a change in pull request #1814:
URL: https://github.com/apache/hive/pull/1814#discussion_r552640050
##########
File path: beeline/src/test/org/apache/hive/beeline/TestCommands.java
##########
@@ -30,21 +30,35 @@
@Test
public void testLinesEndingWithComments() {
- int[] escape = {-1};
- assertEquals("show tables;", removeComments("show tables;",escape));
- assertEquals("show tables;", removeComments("show tables; --comments",escape));
- assertEquals("show tables;", removeComments("show tables; -------comments",escape));
- assertEquals("show tables;", removeComments("show tables; -------comments;one;two;three;;;;",escape));
- assertEquals("show", removeComments("show-- tables; -------comments",escape));
- assertEquals("show", removeComments("show --tables; -------comments",escape));
- assertEquals("s", removeComments("s--how --tables; -------comments",escape));
- assertEquals("", removeComments("-- show tables; -------comments",escape));
+ assertEquals("show tables;", removeComments("show tables;"));
+ assertEquals("show tables;", removeComments("show tables; --comments"));
+ assertEquals("show tables;", removeComments("show tables; -------comments"));
+ assertEquals("show tables;", removeComments("show tables; -------comments;one;two;three;;;;"));
+ assertEquals("show", removeComments("show-- tables; -------comments"));
+ assertEquals("show", removeComments("show --tables; -------comments"));
+ assertEquals("s", removeComments("s--how --tables; -------comments"));
+ assertEquals("", removeComments("-- show tables; -------comments"));
- assertEquals("\"show tables\"", removeComments("\"show tables\" --comments",escape));
- assertEquals("\"show --comments tables\"", removeComments("\"show --comments tables\" --comments",escape));
- assertEquals("\"'show --comments' tables\"", removeComments("\"'show --comments' tables\" --comments",escape));
- assertEquals("'show --comments tables'", removeComments("'show --comments tables' --comments",escape));
- assertEquals("'\"show --comments tables\"'", removeComments("'\"show --comments tables\"' --comments",escape));
+ assertEquals("\"show tables\"", removeComments("\"show tables\" --comments"));
+ assertEquals("\"show --comments tables\"", removeComments("\"show --comments tables\" --comments"));
+ assertEquals("\"'show --comments' tables\"", removeComments("\"'show --comments' tables\" --comments"));
+ assertEquals("'show --comments tables'", removeComments("'show --comments tables' --comments"));
+ assertEquals("'\"show --comments tables\"'", removeComments("'\"show --comments tables\"' --comments"));
+
+ assertEquals("show tables;", removeComments("--comments\nshow tables;"));
+ assertEquals("show tables;", removeComments("--comments\nshow tables; --comments"));
+ assertEquals("show tables;", removeComments("--comments\nshow tables; -------comments"));
+ assertEquals("show tables;", removeComments("--comments\nshow tables; -------comments;one;two;three;;;;"));
+ assertEquals("show", removeComments("--comments\nshow-- tables; -------comments"));
+ assertEquals("show", removeComments("--comments\nshow --tables; -------comments"));
+ assertEquals("s", removeComments("--comments\ns--how --tables; -------comments"));
+ assertEquals("", removeComments("--comments\n-- show tables; -------comments"));
+
+ assertEquals("\"show tables\"", removeComments("--comments\n\"show tables\" --comments"));
+ assertEquals("\"show --comments tables\"", removeComments("--comments\n\"show --comments tables\" --comments"));
+ assertEquals("\"'show --comments' tables\"", removeComments("--comments\n\"'show --comments' tables\" --comments"));
+ assertEquals("'show --comments tables'", removeComments("--comments\n'show --comments tables' --comments"));
+ assertEquals("'\"show --comments tables\"'", removeComments("--comments\n'\"show --comments tables\"' --comments"));
Review comment:
Could you please add a test scenario where a multiline query strings has comments in between fragments of query? just as an example
"select col1,
--partitioned year column
year,
--partitioned month column
month,
--partitioned date column
date
from test_table
where
--for a particular user
username = 'foo';"
should return something equivalent to
"select col1, year, month, date from test_table where username = 'foo';"
----------------------------------------------------------------
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: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] ujc714 commented on a change in pull request #1814: HIVE-15820: comment at the head of beeline -e
Posted by GitBox <gi...@apache.org>.
ujc714 commented on a change in pull request #1814:
URL: https://github.com/apache/hive/pull/1814#discussion_r553051915
##########
File path: beeline/src/test/org/apache/hive/beeline/TestCommands.java
##########
@@ -30,21 +30,35 @@
@Test
public void testLinesEndingWithComments() {
- int[] escape = {-1};
- assertEquals("show tables;", removeComments("show tables;",escape));
- assertEquals("show tables;", removeComments("show tables; --comments",escape));
- assertEquals("show tables;", removeComments("show tables; -------comments",escape));
- assertEquals("show tables;", removeComments("show tables; -------comments;one;two;three;;;;",escape));
- assertEquals("show", removeComments("show-- tables; -------comments",escape));
- assertEquals("show", removeComments("show --tables; -------comments",escape));
- assertEquals("s", removeComments("s--how --tables; -------comments",escape));
- assertEquals("", removeComments("-- show tables; -------comments",escape));
+ assertEquals("show tables;", removeComments("show tables;"));
+ assertEquals("show tables;", removeComments("show tables; --comments"));
+ assertEquals("show tables;", removeComments("show tables; -------comments"));
+ assertEquals("show tables;", removeComments("show tables; -------comments;one;two;three;;;;"));
+ assertEquals("show", removeComments("show-- tables; -------comments"));
+ assertEquals("show", removeComments("show --tables; -------comments"));
+ assertEquals("s", removeComments("s--how --tables; -------comments"));
+ assertEquals("", removeComments("-- show tables; -------comments"));
- assertEquals("\"show tables\"", removeComments("\"show tables\" --comments",escape));
- assertEquals("\"show --comments tables\"", removeComments("\"show --comments tables\" --comments",escape));
- assertEquals("\"'show --comments' tables\"", removeComments("\"'show --comments' tables\" --comments",escape));
- assertEquals("'show --comments tables'", removeComments("'show --comments tables' --comments",escape));
- assertEquals("'\"show --comments tables\"'", removeComments("'\"show --comments tables\"' --comments",escape));
+ assertEquals("\"show tables\"", removeComments("\"show tables\" --comments"));
+ assertEquals("\"show --comments tables\"", removeComments("\"show --comments tables\" --comments"));
+ assertEquals("\"'show --comments' tables\"", removeComments("\"'show --comments' tables\" --comments"));
+ assertEquals("'show --comments tables'", removeComments("'show --comments tables' --comments"));
+ assertEquals("'\"show --comments tables\"'", removeComments("'\"show --comments tables\"' --comments"));
+
+ assertEquals("show tables;", removeComments("--comments\nshow tables;"));
+ assertEquals("show tables;", removeComments("--comments\nshow tables; --comments"));
+ assertEquals("show tables;", removeComments("--comments\nshow tables; -------comments"));
+ assertEquals("show tables;", removeComments("--comments\nshow tables; -------comments;one;two;three;;;;"));
+ assertEquals("show", removeComments("--comments\nshow-- tables; -------comments"));
+ assertEquals("show", removeComments("--comments\nshow --tables; -------comments"));
+ assertEquals("s", removeComments("--comments\ns--how --tables; -------comments"));
+ assertEquals("", removeComments("--comments\n-- show tables; -------comments"));
+
+ assertEquals("\"show tables\"", removeComments("--comments\n\"show tables\" --comments"));
+ assertEquals("\"show --comments tables\"", removeComments("--comments\n\"show --comments tables\" --comments"));
+ assertEquals("\"'show --comments' tables\"", removeComments("--comments\n\"'show --comments' tables\" --comments"));
+ assertEquals("'show --comments tables'", removeComments("--comments\n'show --comments tables' --comments"));
+ assertEquals("'\"show --comments tables\"'", removeComments("--comments\n'\"show --comments tables\"' --comments"));
Review comment:
Thanks for having a look! I added your example. It contains both separate comment lines and inline comment.
----------------------------------------------------------------
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: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] ujc714 commented on pull request #1814: HIVE-15820: comment at the head of beeline -e
Posted by GitBox <gi...@apache.org>.
ujc714 commented on pull request #1814:
URL: https://github.com/apache/hive/pull/1814#issuecomment-754971040
I moved trim() from HiveStringUtils.removeComments(String, int[]) to HiveStringUtils.removeComments(String) so that I only need to change TestHiveStringUtils.java instead of 6 other test result files.
In my option, HiveStringUtils.removeComments(String, int[]) shouldn't trim the spaces. Otherwise, we'll lose the indents in a formatted SQL statement. Trimming the leading and trailing spaces of the whole statement should be enough.
----------------------------------------------------------------
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: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] kgyrtkirk merged pull request #1814: HIVE-15820: comment at the head of beeline -e
Posted by GitBox <gi...@apache.org>.
kgyrtkirk merged pull request #1814:
URL: https://github.com/apache/hive/pull/1814
----------------------------------------------------------------
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: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] kgyrtkirk merged pull request #1814: HIVE-15820: comment at the head of beeline -e
Posted by GitBox <gi...@apache.org>.
kgyrtkirk merged pull request #1814:
URL: https://github.com/apache/hive/pull/1814
----------------------------------------------------------------
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: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] ujc714 commented on pull request #1814: HIVE-15820: comment at the head of beeline -e
Posted by GitBox <gi...@apache.org>.
ujc714 commented on pull request #1814:
URL: https://github.com/apache/hive/pull/1814#issuecomment-753778414
The function trim() is called by HiveStringUtils.removeComments(String, int[]). As HiveStringUtils.removeComments(String, int[]) process a multiline statement as a single line string, only the leading spaces in the first line and the trailing spaces in the last line are removed. After I replaced HiveStringUtils.removeComments(String, int[]) with HiveStringUtils.removeComments(String), the leading spaces and trailing spaces in each line are trimmed. It's why 6 tests failed.
Rather than change HiveStringUtils.removeComments(String, int[]), I changed the test files as the new behaviour is expected.
----------------------------------------------------------------
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: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org