From 63afd6ad332841f1ca20a6c96309702968934c77 Mon Sep 17 00:00:00 2001 From: Jeff Zhang Date: Thu, 11 Jun 2020 11:00:02 +0800 Subject: [PATCH 1/3] [ZEPPELIN-4877]. text between 2 sql comment is mistaken as comment --- .../interpreter/util/SqlSplitter.java | 39 +++++++------------ .../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 95af0e02456..f4f5f675732 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 List splitSql(String text) { } // 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 List splitSql(String text) { && 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.charAt(index + 1) == '*' + && text.length() > (index + 2) && 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 afb62890892..f112002c1d0 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 void testSingleLineComment() { 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 void testMultipleLineComment() { 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 void testCustomSplitter_1() { 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 void testCustomSplitter_2() { 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)); } } From 6e9287e16fdfaf59e33d6d65178fbb2e80a83b1b Mon Sep 17 00:00:00 2001 From: Jeff Zhang Date: Mon, 15 Jun 2020 15:25:36 +0800 Subject: [PATCH 2/3] address comment --- .../org/apache/zeppelin/interpreter/util/SqlSplitter.java | 4 ++-- 1 file changed, 2 insertions(+), 2 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 f4f5f675732..bbbb2371a38 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 @@ -106,8 +106,8 @@ public List splitSql(String text) { && text.length() > (index + 1)) { if (isSingleLineComment(text.charAt(index), text.charAt(index + 1))) { singleLineComment = true; - } else if (text.charAt(index) == '/' && text.charAt(index + 1) == '*' - && text.length() > (index + 2) && text.charAt(index + 2) != '+') { + } else if (text.length() > (index + 2) && text.charAt(index + 2) != '+' && + text.charAt(index) == '/' && text.charAt(index + 1) == '*') { multiLineComment = true; } } From 0b0b728020bbb445537eacf7ab48dd8802c28a33 Mon Sep 17 00:00:00 2001 From: Jeff Zhang Date: Tue, 16 Jun 2020 15:10:37 +0800 Subject: [PATCH 3/3] address comment --- .../org/apache/zeppelin/interpreter/util/SqlSplitter.java | 4 ++-- 1 file changed, 2 insertions(+), 2 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 bbbb2371a38..14e6cec30ea 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 @@ -106,8 +106,8 @@ public List splitSql(String text) { && text.length() > (index + 1)) { if (isSingleLineComment(text.charAt(index), text.charAt(index + 1))) { singleLineComment = true; - } else if (text.length() > (index + 2) && text.charAt(index + 2) != '+' && - 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; } }