From commits-return-4301-archive-asf-public=cust-asf.ponee.io@zeppelin.apache.org Sun Apr 1 05:29:20 2018 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by mx-eu-01.ponee.io (Postfix) with SMTP id A2C6718064A for ; Sun, 1 Apr 2018 05:29:19 +0200 (CEST) Received: (qmail 84792 invoked by uid 500); 1 Apr 2018 03:29:18 -0000 Mailing-List: contact commits-help@zeppelin.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@zeppelin.apache.org Delivered-To: mailing list commits@zeppelin.apache.org Received: (qmail 84783 invoked by uid 99); 1 Apr 2018 03:29:18 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Sun, 01 Apr 2018 03:29:18 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 698BDF654D; Sun, 1 Apr 2018 03:29:18 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: zjffdu@apache.org To: commits@zeppelin.apache.org Message-Id: X-Mailer: ASF-Git Admin Mailer Subject: zeppelin git commit: ZEPPELIN-3344. Revert comments in queries in JDBC interpreter Date: Sun, 1 Apr 2018 03:29:18 +0000 (UTC) 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 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 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 Authored: Tue Mar 20 08:57:02 2018 +0300 Committer: Jeff Zhang 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 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");