Repository: zeppelin Updated Branches: refs/heads/branch-0.8 da277b9fb -> bbfae7c72
ZEPPELIN-3344. Revert comments in queries in JDBC interpreter ### What is this PR for? The original purpose of https://github.com/apache/zeppelin/pull/2158 was correct processing of ';'. This was done via full removing comments from code. Unfortunately Apache Phoenix uses hooks in comments https://forcedotcom.github.io/phoenix/#hintml. Thus we should not delete comments in scripts. There was discussion about comment rules for different databases (solr). The right way is keep style. Thus analysts can copy queries to another tool and can get same results and errors. ### What type of PR is it? [Bug Fix] ### What is the Jira issue? * https://issues.apache.org/jira/browse/ZEPPELIN-3344 ### How should this be tested? * Unit tests pass: testSplitSqlQueryWithComments and testSplitSqlQuery ### Questions: * Does the licenses files need update? No * Is there breaking changes for older versions? No * Does this needs documentation? No Author: mebelousov <mebelou...@ya.ru> Closes #2876 from mebelousov/ZEPPELIN-3344 and squashes the following commits: 6980400 [mebelousov] ZEPPELIN-3344 Fix checkstyle, add tests for comments 83c8e8f [mebelousov] ZEPPELIN-3344 Rename test eed54c8 [mebelousov] ZEPPELIN-3344 Revert comments in JDBC interpreter (cherry picked from commit 8238b711c7f6ff2c71ff807c6ee4ad52a29728a0) Signed-off-by: Jeff Zhang <zjf...@apache.org> Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/bbfae7c7 Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/bbfae7c7 Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/bbfae7c7 Branch: refs/heads/branch-0.8 Commit: bbfae7c725586e25eb1e71d60047b8804412208a Parents: da277b9 Author: mebelousov <mebelou...@ya.ru> Authored: Tue Mar 20 08:57:02 2018 +0300 Committer: Jeff Zhang <zjf...@apache.org> Committed: Sun Apr 1 11:29:13 2018 +0800 ---------------------------------------------------------------------- .../apache/zeppelin/jdbc/JDBCInterpreter.java | 21 ++++++-------------- .../zeppelin/jdbc/JDBCInterpreterTest.java | 11 +++++++--- 2 files changed, 14 insertions(+), 18 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/zeppelin/blob/bbfae7c7/jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java ---------------------------------------------------------------------- diff --git a/jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java b/jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java index 0265e2d..fa69165 100644 --- a/jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java +++ b/jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java @@ -588,18 +588,12 @@ public class JDBCInterpreter extends KerberosInterpreter { for (int item = 0; item < sql.length(); item++) { character = sql.charAt(item); - if ((singleLineComment && (character == '\n' || item == sql.length() - 1)) - || (multiLineComment && character == '/' && sql.charAt(item - 1) == '*')) { + if (singleLineComment && (character == '\n' || item == sql.length() - 1)) { singleLineComment = false; - multiLineComment = false; - if (item == sql.length() - 1 && query.length() > 0) { - queries.add(StringUtils.trim(query.toString())); - } - continue; } - if (singleLineComment || multiLineComment) { - continue; + if (multiLineComment && character == '/' && sql.charAt(item - 1) == '*') { + multiLineComment = false; } if (character == '\'') { @@ -622,16 +616,13 @@ public class JDBCInterpreter extends KerberosInterpreter { && sql.length() > item + 1) { if (character == '-' && sql.charAt(item + 1) == '-') { singleLineComment = true; - continue; - } - - if (character == '/' && sql.charAt(item + 1) == '*') { + } else if (character == '/' && sql.charAt(item + 1) == '*') { multiLineComment = true; - continue; } } - if (character == ';' && !quoteString && !doubleQuoteString) { + if (character == ';' && !quoteString && !doubleQuoteString && !multiLineComment + && !singleLineComment) { queries.add(StringUtils.trim(query.toString())); query = new StringBuilder(); } else if (item == sql.length() - 1) { http://git-wip-us.apache.org/repos/asf/zeppelin/blob/bbfae7c7/jdbc/src/test/java/org/apache/zeppelin/jdbc/JDBCInterpreterTest.java ---------------------------------------------------------------------- diff --git a/jdbc/src/test/java/org/apache/zeppelin/jdbc/JDBCInterpreterTest.java b/jdbc/src/test/java/org/apache/zeppelin/jdbc/JDBCInterpreterTest.java index 6441267..f1587d5 100644 --- a/jdbc/src/test/java/org/apache/zeppelin/jdbc/JDBCInterpreterTest.java +++ b/jdbc/src/test/java/org/apache/zeppelin/jdbc/JDBCInterpreterTest.java @@ -183,13 +183,16 @@ public class JDBCInterpreterTest extends BasicJDBCTestCaseAdapter { "select '\n', ';';" + "select replace('A\\;B', '\\', 'text');" + "select '\\', ';';" + - "select '''', ';'"; + "select '''', ';';" + + "select /*+ scan */ * from test_table;" + + "--singleLineComment\nselect * from test_table"; + Properties properties = new Properties(); JDBCInterpreter t = new JDBCInterpreter(properties); t.open(); List<String> multipleSqlArray = t.splitSqlQueries(sqlQuery); - assertEquals(8, multipleSqlArray.size()); + assertEquals(10, multipleSqlArray.size()); assertEquals("insert into test_table(id, name) values ('a', ';\"')", multipleSqlArray.get(0)); assertEquals("select * from test_table", multipleSqlArray.get(1)); assertEquals("select * from test_table WHERE ID = \";'\"", multipleSqlArray.get(2)); @@ -198,6 +201,8 @@ public class JDBCInterpreterTest extends BasicJDBCTestCaseAdapter { assertEquals("select replace('A\\;B', '\\', 'text')", multipleSqlArray.get(5)); assertEquals("select '\\', ';'", multipleSqlArray.get(6)); assertEquals("select '''', ';'", multipleSqlArray.get(7)); + assertEquals("select /*+ scan */ * from test_table", multipleSqlArray.get(8)); + assertEquals("--singleLineComment\nselect * from test_table", multipleSqlArray.get(9)); } @Test @@ -524,7 +529,7 @@ public class JDBCInterpreterTest extends BasicJDBCTestCaseAdapter { } @Test - public void testExcludingComments() throws SQLException, IOException { + public void testSplitSqlQueryWithComments() throws SQLException, IOException { Properties properties = new Properties(); properties.setProperty("common.max_count", "1000"); properties.setProperty("common.max_retry", "3");
