You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zeppelin.apache.org by zj...@apache.org on 2020/06/17 03:34:47 UTC
[zeppelin] branch branch-0.9 updated: [ZEPPELIN-4877]. text between
2 sql comment is mistaken as comment
This is an automated email from the ASF dual-hosted git repository.
zjffdu pushed a commit to branch branch-0.9
in repository https://gitbox.apache.org/repos/asf/zeppelin.git
The following commit(s) were added to refs/heads/branch-0.9 by this push:
new 939b38b [ZEPPELIN-4877]. text between 2 sql comment is mistaken as comment
939b38b is described below
commit 939b38b5fad9db9fd089e3338e159085c2500ad9
Author: Jeff Zhang <zj...@apache.org>
AuthorDate: Tue Jun 16 15:10:37 2020 +0800
[ZEPPELIN-4877]. text between 2 sql comment is mistaken as comment
### What is this PR for?
This PR fix the bug in Splitter that text between 2 sql comment is mistaken as comment. Unit test is added in this PR as well. Besides that, this PR also remove the comment in the middle of sql. e.g.
```
select a -- comment 1
from table_1
```
will be converted to
```
select a
from table_1
```
### What type of PR is it?
[Bug Fix]
### Todos
* [ ] - Task
### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-4877
### How should this be tested?
* CI pass
### Screenshots (if appropriate)
### Questions:
* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No
Author: Jeff Zhang <zj...@apache.org>
Closes #3796 from zjffdu/ZEPPELIN-4877 and squashes the following commits:
0b0b72802 [Jeff Zhang] address comment
6e9287e16 [Jeff Zhang] address comment
63afd6ad3 [Jeff Zhang] [ZEPPELIN-4877]. text between 2 sql comment is mistaken as comment
(cherry picked from commit badcaa0ca939733721bd603effac69959dbd6ac3)
Signed-off-by: Jeff Zhang <zj...@apache.org>
---
.../zeppelin/interpreter/util/SqlSplitter.java | 39 +++++++++-------------
.../zeppelin/interpreter/util/SqlSplitterTest.java | 31 ++++++++++++++---
2 files changed, 42 insertions(+), 28 deletions(-)
diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/util/SqlSplitter.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/util/SqlSplitter.java
index 95af0e0..14e6cec 100644
--- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/util/SqlSplitter.java
+++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/util/SqlSplitter.java
@@ -19,6 +19,8 @@
package org.apache.zeppelin.interpreter.util;
+import org.apache.commons.lang3.StringUtils;
+
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
@@ -79,11 +81,9 @@ public class SqlSplitter {
}
// end of multiple line comment
- if (multiLineComment && character == '/' && text.charAt(index - 1) == '*') {
+ if (multiLineComment && (index - 1) >= 0 && text.charAt(index - 1) == '/'
+ && (index - 2) >= 0 && text.charAt(index - 2) == '*') {
multiLineComment = false;
- if (query.toString().trim().isEmpty()) {
- continue;
- }
}
if (character == '\'') {
@@ -106,39 +106,30 @@ public class SqlSplitter {
&& text.length() > (index + 1)) {
if (isSingleLineComment(text.charAt(index), text.charAt(index + 1))) {
singleLineComment = true;
- } else if (text.charAt(index) == '/' && text.charAt(index + 1) == '*') {
+ } else if (text.charAt(index) == '/' && text.length() > (index + 2)
+ && text.charAt(index + 1) == '*' && text.charAt(index + 2) != '+') {
multiLineComment = true;
}
}
- if (character == ';' && !singleQuoteString && !doubleQuoteString && !multiLineComment
- && !singleLineComment) {
- // meet semicolon
- queries.add(query.toString().trim());
- query = new StringBuilder();
+ if (character == ';' && !singleQuoteString && !doubleQuoteString && !multiLineComment && !singleLineComment) {
+ // meet the end of semicolon
+ if (!query.toString().trim().isEmpty()) {
+ queries.add(query.toString().trim());
+ query = new StringBuilder();
+ }
} else if (index == (text.length() - 1)) {
// meet the last character
if (!singleLineComment && !multiLineComment) {
query.append(character);
+ }
+ if (!query.toString().trim().isEmpty()) {
queries.add(query.toString().trim());
+ query = new StringBuilder();
}
} else if (!singleLineComment && !multiLineComment) {
// normal case, not in single line comment and not in multiple line comment
query.append(character);
- } else if (singleLineComment && !query.toString().trim().isEmpty()) {
- // in single line comment, only add it to query when the single line comment is
- // in the middle of sql statement
- // e.g.
- // select a -- comment
- // from table_1
- query.append(character);
- } else if (multiLineComment && !query.toString().trim().isEmpty()) {
- // in multiple line comment, only add it to query when the multiple line comment
- // is in the middle of sql statement.
- // e.g.
- // select a /* comment */
- // from table_1
- query.append(character);
}
}
diff --git a/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/util/SqlSplitterTest.java b/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/util/SqlSplitterTest.java
index afb6289..f112002 100644
--- a/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/util/SqlSplitterTest.java
+++ b/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/util/SqlSplitterTest.java
@@ -100,7 +100,16 @@ public class SqlSplitterTest {
sqls = sqlSplitter.splitSql("select a -- comment\n from table_1");
assertEquals(1, sqls.size());
- assertEquals("select a -- comment\n from table_1", sqls.get(0));
+ assertEquals("select a \n from table_1", sqls.get(0));
+
+ sqls = sqlSplitter.splitSql("--comment 1\nselect a from table_1\n--comment 2");
+ assertEquals(1, sqls.size());
+ assertEquals("select a from table_1", sqls.get(0));
+
+ sqls = sqlSplitter.splitSql("--comment 1\nselect a from table_1;\n--comment 2\nselect b from table_1");
+ assertEquals(2, sqls.size());
+ assertEquals("select a from table_1", sqls.get(0));
+ assertEquals("select b from table_1", sqls.get(1));
}
@Test
@@ -145,7 +154,21 @@ public class SqlSplitterTest {
sqls = sqlSplitter.splitSql("select a /*comment*/ from table_1");
assertEquals(1, sqls.size());
- assertEquals("select a /*comment*/ from table_1", sqls.get(0));
+ assertEquals("select a from table_1", sqls.get(0));
+
+ sqls = sqlSplitter.splitSql("/*comment 1*/\nselect a from table_1\n/*comment 2*/");
+ assertEquals(1, sqls.size());
+ assertEquals("select a from table_1", sqls.get(0));
+
+ sqls = sqlSplitter.splitSql("/*comment 1*/\nselect a from table_1;\n/*comment 2*/select b from table_1");
+ assertEquals(2, sqls.size());
+ assertEquals("select a from table_1", sqls.get(0));
+ assertEquals("select b from table_1", sqls.get(1));
+
+ sqls = sqlSplitter.splitSql("/*comment 1*/\nselect a /*+ hint*/ from table_1;\n/*comment 2*/select b from table_1");
+ assertEquals(2, sqls.size());
+ assertEquals("select a /*+ hint*/ from table_1", sqls.get(0));
+ assertEquals("select b from table_1", sqls.get(1));
}
@Test
@@ -231,7 +254,7 @@ public class SqlSplitterTest {
sqls = sqlSplitter.splitSql("select a // comment\n from table_1");
assertEquals(1, sqls.size());
- assertEquals("select a // comment\n from table_1", sqls.get(0));
+ assertEquals("select a \n from table_1", sqls.get(0));
}
@Test
@@ -281,6 +304,6 @@ public class SqlSplitterTest {
sqls = sqlSplitter.splitSql("select a # comment\n from table_1");
assertEquals(1, sqls.size());
- assertEquals("select a # comment\n from table_1", sqls.get(0));
+ assertEquals("select a \n from table_1", sqls.get(0));
}
}