zeppelin-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From zjf...@apache.org
Subject zeppelin git commit: ZEPPELIN-3344. Revert comments in queries in JDBC interpreter
Date Sun, 01 Apr 2018 03:29:18 GMT
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 <mebelousov@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 <zjffdu@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 <mebelousov@ya.ru>
Authored: Tue Mar 20 08:57:02 2018 +0300
Committer: Jeff Zhang <zjffdu@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");


Mime
View raw message