diff --git a/beeline/src/java/org/apache/hive/beeline/Commands.java b/beeline/src/java/org/apache/hive/beeline/Commands.java index 3b2d72ed79..67f27f6638 100644 --- a/beeline/src/java/org/apache/hive/beeline/Commands.java +++ b/beeline/src/java/org/apache/hive/beeline/Commands.java @@ -63,9 +63,10 @@ import org.apache.hive.jdbc.HiveStatement; import org.apache.hive.jdbc.Utils; import org.apache.hive.jdbc.Utils.JdbcConnectionParams; -import com.google.common.annotations.VisibleForTesting; import org.apache.hive.jdbc.logs.InPlaceUpdateStream; +import static org.apache.hive.common.util.HiveStringUtils.removeComments; + public class Commands { private final BeeLine beeLine; @@ -1068,32 +1069,6 @@ private boolean executeInternal(String sql, boolean call) { return true; } - //startQuote use array type in order to pass int type as input/output parameter. - //This method remove comment from current line of a query. - //It does not remove comment like strings inside quotes. - @VisibleForTesting - String removeComments(String line, int[] startQuote) { - if (line == null || line.isEmpty()) return line; - if (startQuote[0] == -1 && beeLine.isComment(line)) return ""; //assume # can only be used at the beginning of line. - StringBuilder builder = new StringBuilder(); - for (int index = 0; index < line.length(); index++) { - if (startQuote[0] == -1 && index < line.length() - 1 && line.charAt(index) == '-' && line.charAt(index + 1) =='-') { - return builder.toString().trim(); - } - - char letter = line.charAt(index); - if (startQuote[0] == letter && (index == 0 || line.charAt(index -1) != '\\') ) { - startQuote[0] = -1; // Turn escape off. - } else if (startQuote[0] == -1 && (letter == '\'' || letter == '"') && (index == 0 || line.charAt(index -1) != '\\')) { - startQuote[0] = letter; // Turn escape on. - } - - builder.append(letter); - } - - return builder.toString().trim(); - } - /* * Check if the input line is a multi-line command which needs to read further */ diff --git a/beeline/src/test/org/apache/hive/beeline/TestCommands.java b/beeline/src/test/org/apache/hive/beeline/TestCommands.java index 04c939a04c..80c01ff8d1 100644 --- a/beeline/src/test/org/apache/hive/beeline/TestCommands.java +++ b/beeline/src/test/org/apache/hive/beeline/TestCommands.java @@ -20,29 +20,28 @@ import org.junit.Test; +import static org.apache.hive.common.util.HiveStringUtils.removeComments; import static org.junit.Assert.assertEquals; public class TestCommands { @Test public void testLinesEndingWithComments() { - BeeLine beeline = new BeeLine(); - Commands commands = new Commands(beeline); int[] escape = {-1}; - assertEquals("show tables;", commands.removeComments("show tables;",escape)); - assertEquals("show tables;", commands.removeComments("show tables; --comments",escape)); - assertEquals("show tables;", commands.removeComments("show tables; -------comments",escape)); - assertEquals("show tables;", commands.removeComments("show tables; -------comments;one;two;three;;;;",escape)); - assertEquals("show", commands.removeComments("show-- tables; -------comments",escape)); - assertEquals("show", commands.removeComments("show --tables; -------comments",escape)); - assertEquals("s", commands.removeComments("s--how --tables; -------comments",escape)); - assertEquals("", commands.removeComments("-- show tables; -------comments",escape)); + 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\"", commands.removeComments("\"show tables\" --comments",escape)); - assertEquals("\"show --comments tables\"", commands.removeComments("\"show --comments tables\" --comments",escape)); - assertEquals("\"'show --comments' tables\"", commands.removeComments("\"'show --comments' tables\" --comments",escape)); - assertEquals("'show --comments tables'", commands.removeComments("'show --comments tables' --comments",escape)); - assertEquals("'\"show --comments tables\"'", commands.removeComments("'\"show --comments tables\"' --comments",escape)); + 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)); } } diff --git a/cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java b/cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java index 27fd66d35e..4ee4f65e5b 100644 --- a/cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java +++ b/cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java @@ -19,6 +19,7 @@ package org.apache.hadoop.hive.cli; import static org.apache.hadoop.util.StringUtils.stringifyException; +import static org.apache.hive.common.util.HiveStringUtils.removeComments; import java.io.BufferedReader; import java.io.File; @@ -122,7 +123,7 @@ public int processCmd(String cmd) { // Flush the print stream, so it doesn't include output from the last command ss.err.flush(); - String cmd_trimmed = cmd.trim(); + String cmd_trimmed = removeComments(cmd).trim(); String[] tokens = tokenizeCmd(cmd_trimmed); int ret = 0; @@ -181,7 +182,12 @@ public int processCmd(String cmd) { } else { // local mode try { CommandProcessor proc = CommandProcessorFactory.get(tokens, (HiveConf) conf); - ret = processLocalCmd(cmd, proc, ss); + if (proc instanceof Driver) { + // Let Driver strip comments using sql parser + ret = processLocalCmd(cmd, proc, ss); + } else { + ret = processLocalCmd(cmd_trimmed, proc, ss); + } } catch (SQLException e) { console.printError("Failed processing command " + tokens[0] + " " + e.getLocalizedMessage(), org.apache.hadoop.util.StringUtils.stringifyException(e)); diff --git a/common/src/java/org/apache/hive/common/util/HiveStringUtils.java b/common/src/java/org/apache/hive/common/util/HiveStringUtils.java index 4a6413a7c3..edf5edcefa 100644 --- a/common/src/java/org/apache/hive/common/util/HiveStringUtils.java +++ b/common/src/java/org/apache/hive/common/util/HiveStringUtils.java @@ -1073,4 +1073,71 @@ public static String getPartitionValWithInvalidCharacter(List partVals, return null; } + + /** + * Strip comments from a sql statement, tracking when the statement contains a string literal + * + * @param statement the input string + * @return a stripped statement + */ + public static String removeComments(String statement) { + if (statement == null) { + return null; + } + String[] split = statement.split("\n"); + int[] startQuote = { -1 }; + StringBuilder ret = new StringBuilder(statement.length()); + for (int i = 0; i < split.length; i++) { + String lineWithComments = split[i]; + String lineNoComments = removeComments(lineWithComments, startQuote); + ret.append(lineNoComments); + if (i != split.length - 1 && !lineNoComments.isEmpty()) { + ret.append("\n"); + } + } + return ret.toString(); + } + + //startQuote use array type in order to pass int type as input/output parameter. + //This method remove comment from current line of a query. + //It does not remove comment like strings inside quotes. + public static String removeComments(String line, int[] startQuote) { + if (line == null || line.isEmpty()) + return line; + if (startQuote[0] == -1 && isComment(line)) + return ""; //assume # can only be used at the beginning of line. + StringBuilder builder = new StringBuilder(); + for (int index = 0; index < line.length(); index++) { + if (startQuote[0] == -1 && index < line.length() - 1 && line.charAt(index) == '-' + && line.charAt(index + 1) == '-') { + return builder.toString().trim(); + } + + char letter = line.charAt(index); + if (startQuote[0] == letter && (index == 0 || line.charAt(index - 1) != '\\')) { + startQuote[0] = -1; // Turn escape off. + } else if (startQuote[0] == -1 && (letter == '\'' || letter == '"') && (index == 0 + || line.charAt(index - 1) != '\\')) { + startQuote[0] = letter; // Turn escape on. + } + + builder.append(letter); + } + + return builder.toString(); + } + + /** + * Test whether a line is a comment. + * + * @param line the line to be tested + * @return true if a comment + */ + private static boolean isComment(String line) { + // SQL92 comment prefix is "--" + // beeline also supports shell-style "#" prefix + String lineTrimmed = line.trim(); + return lineTrimmed.startsWith("#") || lineTrimmed.startsWith("--"); + } + } diff --git a/common/src/test/org/apache/hive/common/util/TestHiveStringUtils.java b/common/src/test/org/apache/hive/common/util/TestHiveStringUtils.java index 6bd7037152..022e72fe7d 100644 --- a/common/src/test/org/apache/hive/common/util/TestHiveStringUtils.java +++ b/common/src/test/org/apache/hive/common/util/TestHiveStringUtils.java @@ -18,6 +18,7 @@ package org.apache.hive.common.util; +import static org.apache.hive.common.util.HiveStringUtils.removeComments; import static org.junit.Assert.*; import java.util.Arrays; @@ -61,4 +62,55 @@ public void splitAndUnEscapeTestCase(String testValue, String[] expectedResults) assertTrue(Arrays.toString(expectedResults) + " == " + Arrays.toString(testResults), Arrays.equals(expectedResults, testResults)); } + + @Test + public void testStripComments() throws Exception { + assertNull(removeComments(null)); + assertUnchanged("foo"); + assertUnchanged("select 1"); + assertUnchanged("insert into foo (values('-----')"); + assertUnchanged("insert into foo (values('abc\n\'xyz')"); + assertUnchanged("create database if not exists testDB; set hive.cli.print.current.db=true;use\ntestDB;\nuse default;drop if exists testDB;"); + + assertEquals("foo", removeComments("foo\n")); + assertEquals("foo", removeComments("\nfoo")); + assertEquals("insert into foo (values('-----')", removeComments("--comment\ninsert into foo (values('-----')")); + assertEquals("insert into foo (values('----''-')", removeComments("--comment\ninsert into foo (values('----''-')")); + assertEquals("insert into foo (values(\"----''-\")", removeComments("--comment\ninsert into foo (values(\"----''-\")")); + assertEquals("insert into foo (values(\"----\"\"-\")", removeComments("--comment\ninsert into foo (values(\"----\"\"-\")")); + assertEquals("insert into foo (values('-\n--\n--')", removeComments("--comment\ninsert into foo (values('-\n--\n--')")); + assertEquals("insert into foo (values('-\n--\n--')", removeComments("--comment\n\ninsert into foo (values('-\n--\n--')")); + assertEquals("insert into foo (values(\"-\n--\n--\")", removeComments("--comment\n\ninsert into foo (values(\"-\n--\n--\")")); + assertEquals("insert into foo (values('abc');\ninsert into foo (values('def');", removeComments( "insert into foo (values('abc');\n--comment\ninsert into foo (values('def');")); + } + + @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\" --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)); + } + + /** + * check that statement is unchanged after stripping + */ + private void assertUnchanged(String statement) { + assertEquals("statement should not have been affected by stripping commnents", statement, + removeComments(statement)); + } + + + } diff --git a/ql/src/test/org/apache/hadoop/hive/ql/processors/TestCommandProcessorFactory.java b/ql/src/test/org/apache/hadoop/hive/ql/processors/TestCommandProcessorFactory.java index 21bdcf4443..50eaf18f97 100644 --- a/ql/src/test/org/apache/hadoop/hive/ql/processors/TestCommandProcessorFactory.java +++ b/ql/src/test/org/apache/hadoop/hive/ql/processors/TestCommandProcessorFactory.java @@ -46,6 +46,8 @@ public void testInvalidCommands() throws Exception { CommandProcessorFactory.getForHiveCommand(new String[]{" "}, conf)); Assert.assertNull("Set role should have returned null", CommandProcessorFactory.getForHiveCommand(new String[]{"set role"}, conf)); + Assert.assertNull("Set role should have returned null", + CommandProcessorFactory.getForHiveCommand(new String[]{"set", "role"}, conf)); Assert.assertNull("SQL should have returned null", CommandProcessorFactory.getForHiveCommand(new String[]{"SELECT * FROM TABLE"}, conf)); Assert.assertNull("Test only command should have returned null", diff --git a/ql/src/test/queries/clientpositive/set_processor_namespaces.q b/ql/src/test/queries/clientpositive/set_processor_namespaces.q index 612807f0c8..1f9e282c3b 100644 --- a/ql/src/test/queries/clientpositive/set_processor_namespaces.q +++ b/ql/src/test/queries/clientpositive/set_processor_namespaces.q @@ -30,3 +30,11 @@ set jar=${system:maven.local.repository}/org/apache/derby/derby/${system:derby.v add file ${hiveconf:jar}; delete file ${hiveconf:jar}; list file; + + +-- comment (will be removed by test driver) +set x=1; +set x; + -- an indented comment to test comment removal +set x=2; +set x; \ No newline at end of file diff --git a/ql/src/test/results/clientpositive/set_processor_namespaces.q.out b/ql/src/test/results/clientpositive/set_processor_namespaces.q.out index c05ce4d61d..c1c82704e9 100644 --- a/ql/src/test/results/clientpositive/set_processor_namespaces.q.out +++ b/ql/src/test/results/clientpositive/set_processor_namespaces.q.out @@ -51,3 +51,5 @@ POSTHOOK: Input: default@src 5 val_5 5 val_5 c=1 +x=1 +x=2 diff --git a/service/src/java/org/apache/hive/service/cli/operation/ExecuteStatementOperation.java b/service/src/java/org/apache/hive/service/cli/operation/ExecuteStatementOperation.java index 2dd90b69b3..84592edd96 100644 --- a/service/src/java/org/apache/hive/service/cli/operation/ExecuteStatementOperation.java +++ b/service/src/java/org/apache/hive/service/cli/operation/ExecuteStatementOperation.java @@ -26,6 +26,8 @@ import org.apache.hive.service.cli.OperationType; import org.apache.hive.service.cli.session.HiveSession; +import static org.apache.hive.common.util.HiveStringUtils.removeComments; + public abstract class ExecuteStatementOperation extends Operation { protected String statement = null; @@ -42,7 +44,9 @@ public String getStatement() { public static ExecuteStatementOperation newExecuteStatementOperation(HiveSession parentSession, String statement, Map confOverlay, boolean runAsync, long queryTimeout) throws HiveSQLException { - String[] tokens = statement.trim().split("\\s+"); + + String cleanStatement = removeComments(statement); + String[] tokens = cleanStatement.trim().split("\\s+"); CommandProcessor processor = null; try { processor = CommandProcessorFactory.getForHiveCommand(tokens, parentSession.getHiveConf()); @@ -51,8 +55,9 @@ public static ExecuteStatementOperation newExecuteStatementOperation(HiveSession } if (processor == null) { // runAsync, queryTimeout makes sense only for a SQLOperation + // Pass the original statement to SQLOperation as sql parser can remove comments by itself return new SQLOperation(parentSession, statement, confOverlay, runAsync, queryTimeout); } - return new HiveCommandOperation(parentSession, statement, processor, confOverlay); + return new HiveCommandOperation(parentSession, cleanStatement, processor, confOverlay); } }