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/11/25 04:26:11 UTC
[zeppelin] branch master updated: [ZEPPELIN-5138]. Incorrect line
number in error message of hive interpreter
This is an automated email from the ASF dual-hosted git repository.
zjffdu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zeppelin.git
The following commit(s) were added to refs/heads/master by this push:
new 8730945 [ZEPPELIN-5138]. Incorrect line number in error message of hive interpreter
8730945 is described below
commit 8730945dba100583f106cb44d58a58e3a90ecd2f
Author: Jeff Zhang <zj...@apache.org>
AuthorDate: Tue Nov 17 10:22:07 2020 +0800
[ZEPPELIN-5138]. Incorrect line number in error message of hive interpreter
### What is this PR for?
In the current hive interpreter (jdbc), the line number in the error message is not correct, this is due to the logic of how we split sql. In this PR, we fix it via
1. Revert the changes of ZEPPELIN-5135. Because the purpose of ZEPPELIN-5135 is to get the correct the line number in error message, but it only works in single sql statement. This ticket can resolve the issue in all cases (including single sql & multiple sql)
2. Change the logic of `SqlSplitter`, see the SqlSplitterTest for more details.
3. Don't do trim in all the places, otherwise we would get wrong line number.
### What type of PR is it?
[ Improvement ]
### Todos
* [ ] - Task
### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-5138
### How should this be tested?
* CI pass & manually tested
https://travis-ci.com/github/zjffdu/zeppelin/builds/203247703
### 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 #3979 from zjffdu/ZEPPELIN-5138 and squashes the following commits:
63c3d6980 [Jeff Zhang] [ZEPPELIN-5138]. Incorrect line number in error message of hive interpreter
---
.../apache/zeppelin/flink/FlinkSqlInterrpeter.java | 2 +-
.../org/apache/zeppelin/jdbc/JDBCInterpreter.java | 18 +----
jdbc/src/main/resources/interpreter-setting.json | 7 --
.../apache/zeppelin/jdbc/JDBCInterpreterTest.java | 47 ++---------
.../java/org/apache/zeppelin/client/ZSession.java | 4 +-
.../integration/InterpreterModeActionsIT.java | 2 +-
.../integration/ZSessionIntegrationTest.java | 4 +-
.../zeppelin/interpreter/util/SqlSplitter.java | 72 +++++++++++++++--
.../zeppelin/interpreter/util/SqlSplitterTest.java | 92 ++++++++++++++--------
.../apache/zeppelin/socket/NotebookServerTest.java | 2 +-
.../zeppelin/notebook/ParagraphTextParser.java | 24 +++++-
.../interpreter/mock/MockInterpreter1.java | 2 +-
.../interpreter/mock/MockInterpreter2.java | 2 +-
.../apache/zeppelin/notebook/ParagraphTest.java | 20 ++---
.../zeppelin/notebook/ParagraphTextParserTest.java | 6 +-
15 files changed, 173 insertions(+), 131 deletions(-)
diff --git a/flink/interpreter/src/main/java/org/apache/zeppelin/flink/FlinkSqlInterrpeter.java b/flink/interpreter/src/main/java/org/apache/zeppelin/flink/FlinkSqlInterrpeter.java
index 3d00bc8..e7f6a3b 100644
--- a/flink/interpreter/src/main/java/org/apache/zeppelin/flink/FlinkSqlInterrpeter.java
+++ b/flink/interpreter/src/main/java/org/apache/zeppelin/flink/FlinkSqlInterrpeter.java
@@ -127,7 +127,7 @@ public abstract class FlinkSqlInterrpeter extends AbstractInterpreter {
try {
boolean runAsOne = Boolean.parseBoolean(context.getStringLocalProperty("runAsOne", "false"));
- List<String> sqls = sqlSplitter.splitSql(st);
+ List<String> sqls = sqlSplitter.splitSql(st).stream().map(String::trim).collect(Collectors.toList());
boolean isFirstInsert = true;
for (String sql : sqls) {
Optional<SqlCommandParser.SqlCommandCall> sqlCommand = sqlCommandParser.parse(sql);
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 282d256..916fe5e 100644
--- a/jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java
+++ b/jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java
@@ -50,7 +50,6 @@ import java.sql.SQLException;
import java.sql.Statement;
import java.util.ArrayList;
import java.util.Arrays;
-import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
@@ -109,7 +108,6 @@ public class JDBCInterpreter extends KerberosInterpreter {
static final String DRIVER_KEY = "driver";
static final String URL_KEY = "url";
static final String USER_KEY = "user";
- static final String SPLIT_QURIES_KEY = "splitQueries";
static final String PASSWORD_KEY = "password";
static final String PRECODE_KEY = "precode";
static final String STATEMENT_PRECODE_KEY = "statementPrecode";
@@ -716,16 +714,7 @@ public class JDBCInterpreter extends KerberosInterpreter {
}
try {
- boolean splitSql = Boolean.parseBoolean(getJDBCConfiguration(user)
- .getPropertyMap(dbPrefix)
- .getProperty(SPLIT_QURIES_KEY, "true"));
- List<String> sqlArray = null;
- if (splitSql) {
- sqlArray = sqlSplitter.splitSql(sql);
- } else {
- sqlArray = Collections.singletonList(sql);
- }
-
+ List<String> sqlArray = sqlSplitter.splitSql(sql);
for (String sqlToExecute : sqlArray) {
statement = connection.createStatement();
@@ -884,10 +873,9 @@ public class JDBCInterpreter extends KerberosInterpreter {
String dbPrefix = getDBPrefix(context);
LOGGER.debug("DBPrefix: {}, SQL command: '{}'", dbPrefix, cmd);
if (!isRefreshMode(context)) {
- return executeSql(dbPrefix, cmd.trim(), context);
+ return executeSql(dbPrefix, cmd, context);
} else {
int refreshInterval = Integer.parseInt(context.getLocalProperties().get("refreshInterval"));
- final String code = cmd.trim();
paragraphCancelMap.put(context.getParagraphId(), false);
ScheduledExecutorService refreshExecutor = Executors.newSingleThreadScheduledExecutor();
refreshExecutorServices.put(context.getParagraphId(), refreshExecutor);
@@ -896,7 +884,7 @@ public class JDBCInterpreter extends KerberosInterpreter {
refreshExecutor.scheduleAtFixedRate(() -> {
context.out.clear(false);
try {
- InterpreterResult result = executeSql(dbPrefix, code, context);
+ InterpreterResult result = executeSql(dbPrefix, cmd, context);
context.out.flush();
interpreterResultRef.set(result);
if (result.code() != Code.SUCCESS) {
diff --git a/jdbc/src/main/resources/interpreter-setting.json b/jdbc/src/main/resources/interpreter-setting.json
index 20c72cf..41fecd0 100644
--- a/jdbc/src/main/resources/interpreter-setting.json
+++ b/jdbc/src/main/resources/interpreter-setting.json
@@ -32,13 +32,6 @@
"description": "JDBC Driver Name",
"type": "string"
},
- "default.splitQueries": {
- "envName": null,
- "propertyName": "default.splitQueries",
- "defaultValue": "true",
- "description": "Whether split the whole text into multiple sql statements",
- "type": "checkbox"
- },
"default.completer.ttlInSeconds": {
"envName": null,
"propertyName": "default.completer.ttlInSeconds",
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 b43eb7c..c49f1ee 100644
--- a/jdbc/src/test/java/org/apache/zeppelin/jdbc/JDBCInterpreterTest.java
+++ b/jdbc/src/test/java/org/apache/zeppelin/jdbc/JDBCInterpreterTest.java
@@ -305,11 +305,11 @@ public class JDBCInterpreterTest extends BasicJDBCTestCaseAdapter {
assertEquals("select * from test_table WHERE ID = \";'\"", multipleSqlArray.get(2));
assertEquals("select * from test_table WHERE ID = ';'", multipleSqlArray.get(3));
assertEquals("select '\n', ';'", multipleSqlArray.get(4));
- 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("select * from test_table", multipleSqlArray.get(9));
+ assertEquals("\nselect replace('A\\;B', '\\', 'text')", multipleSqlArray.get(5));
+ assertEquals("\nselect '\\', ';'", multipleSqlArray.get(6));
+ assertEquals("\nselect '''', ';'", multipleSqlArray.get(7));
+ assertEquals("\nselect /*+ scan */ * from test_table", multipleSqlArray.get(8));
+ assertEquals("\n\nselect * from test_table", multipleSqlArray.get(9));
}
@Test
@@ -818,43 +818,6 @@ public class JDBCInterpreterTest extends BasicJDBCTestCaseAdapter {
assertEquals(3, resultMessages.size());
}
- @Test
- public void testSqlWithoutSplit() throws IOException,
- InterpreterException {
- Properties properties = new Properties();
- properties.setProperty("common.max_count", "1000");
- properties.setProperty("common.max_retry", "3");
- properties.setProperty("default.driver", "org.h2.Driver");
- properties.setProperty("default.url", getJdbcConnection());
- properties.setProperty("default.user", "");
- properties.setProperty("default.password", "");
- properties.setProperty("default.splitQueries", "false");
- JDBCInterpreter t = new JDBCInterpreter(properties);
- t.open();
-
- String sqlQuery = "-- comment\n" +
- "--select * from test_table\n" +
- "select * from test_table;";
-
- InterpreterResult interpreterResult = t.interpret(sqlQuery, context);
- assertEquals(InterpreterResult.Code.SUCCESS, interpreterResult.code());
- List<InterpreterResultMessage> resultMessages = context.out.toInterpreterResultMessage();
- assertEquals(1, resultMessages.size());
- assertEquals(InterpreterResult.Type.TABLE, resultMessages.get(0).getType());
-
-
- // the second sql is skipped.
- context = getInterpreterContext();
- sqlQuery = "select * from test_table;" +
- "select name from test_table";
- interpreterResult = t.interpret(sqlQuery, context);
- assertEquals(InterpreterResult.Code.SUCCESS, interpreterResult.code());
- resultMessages = context.out.toInterpreterResultMessage();
- assertEquals(1, resultMessages.size());
- assertEquals(InterpreterResult.Type.TABLE, resultMessages.get(0).getType());
- assertTrue(resultMessages.get(0).getData(),
- resultMessages.get(0).getData().startsWith("ID\tNAME"));
- }
private InterpreterContext getInterpreterContext() {
return InterpreterContext.builder()
.setAuthenticationInfo(new AuthenticationInfo("testUser"))
diff --git a/zeppelin-client/src/main/java/org/apache/zeppelin/client/ZSession.java b/zeppelin-client/src/main/java/org/apache/zeppelin/client/ZSession.java
index f3aac50..45bef26 100644
--- a/zeppelin-client/src/main/java/org/apache/zeppelin/client/ZSession.java
+++ b/zeppelin-client/src/main/java/org/apache/zeppelin/client/ZSession.java
@@ -275,7 +275,7 @@ public class ZSession {
builder.append(StringUtils.join(propertyString, ","));
builder.append(")");
}
- builder.append("\n" + code);
+ builder.append(" " + code);
String text = builder.toString();
@@ -367,7 +367,7 @@ public class ZSession {
}
builder.append(")");
}
- builder.append("\n" + code);
+ builder.append(" " + code);
String text = builder.toString();
String nextParagraphId = zeppelinClient.nextSessionParagraph(getNoteId(), maxStatement);
diff --git a/zeppelin-integration/src/test/java/org/apache/zeppelin/integration/InterpreterModeActionsIT.java b/zeppelin-integration/src/test/java/org/apache/zeppelin/integration/InterpreterModeActionsIT.java
index 1b2f727..8b219b7 100644
--- a/zeppelin-integration/src/test/java/org/apache/zeppelin/integration/InterpreterModeActionsIT.java
+++ b/zeppelin-integration/src/test/java/org/apache/zeppelin/integration/InterpreterModeActionsIT.java
@@ -155,7 +155,7 @@ public class InterpreterModeActionsIT extends AbstractZeppelinIT {
}
private void setPythonParagraph(int num, String text) {
- setTextOfParagraph(num, "%python\\n " + text);
+ setTextOfParagraph(num, "%python\\n" + text);
runParagraph(num);
try {
waitForParagraph(num, "FINISHED");
diff --git a/zeppelin-interpreter-integration/src/test/java/org/apache/zeppelin/integration/ZSessionIntegrationTest.java b/zeppelin-interpreter-integration/src/test/java/org/apache/zeppelin/integration/ZSessionIntegrationTest.java
index f086f78..98a833d 100644
--- a/zeppelin-interpreter-integration/src/test/java/org/apache/zeppelin/integration/ZSessionIntegrationTest.java
+++ b/zeppelin-interpreter-integration/src/test/java/org/apache/zeppelin/integration/ZSessionIntegrationTest.java
@@ -107,7 +107,7 @@ public class ZSessionIntegrationTest extends AbstractTestRestApi {
assertTrue(result.getResults().get(1).getData(), result.getResults().get(1).getData().contains("ExitValue"));
assertEquals(4, note.getParagraphCount());
- assertEquals("%sh\ninvalid_command", note.getParagraph(3).getText());
+ assertEquals("%sh invalid_command", note.getParagraph(3).getText());
} finally {
session.stop();
@@ -147,7 +147,7 @@ public class ZSessionIntegrationTest extends AbstractTestRestApi {
assertTrue(result.getResults().get(1).getData(), result.getResults().get(1).getData().contains("ExitValue"));
assertEquals(4, note.getParagraphCount());
- assertEquals("%sh\ninvalid_command", note.getParagraph(3).getText());
+ assertEquals("%sh invalid_command", note.getParagraph(3).getText());
} finally {
session.stop();
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 14e6cec..1fee950 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
@@ -22,6 +22,7 @@ package org.apache.zeppelin.interpreter.util;
import org.apache.commons.lang3.StringUtils;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
@@ -58,8 +59,24 @@ public class SqlSplitter {
}
}
+ /**
+ * Split whole text into multiple sql statements.
+ * Two Steps:
+ * Step 1, split the whole text into multiple sql statements.
+ * Step 2, refine the results. Replace the preceding sql statements with empty lines, so that
+ * we can get the correct line number in the parsing error message.
+ *
+ * e.g.
+ * select a from table_1;
+ * select a from table_2;
+ * The above text will be splitted into:
+ * sql_1: select a from table_1
+ * sql_2: \nselect a from table_2
+ *
+ * @param text
+ * @return
+ */
public List<String> splitSql(String text) {
- text = text.trim();
List<String> queries = new ArrayList<>();
StringBuilder query = new StringBuilder();
char character;
@@ -75,9 +92,8 @@ public class SqlSplitter {
// end of single line comment
if (singleLineComment && (character == '\n')) {
singleLineComment = false;
- if (query.toString().trim().isEmpty()) {
- continue;
- }
+ query.append(character);
+ continue;
}
// end of multiple line comment
@@ -115,25 +131,65 @@ public class SqlSplitter {
if (character == ';' && !singleQuoteString && !doubleQuoteString && !multiLineComment && !singleLineComment) {
// meet the end of semicolon
if (!query.toString().trim().isEmpty()) {
- queries.add(query.toString().trim());
+ queries.add(query.toString());
query = new StringBuilder();
}
} else if (index == (text.length() - 1)) {
// meet the last character
- if (!singleLineComment && !multiLineComment) {
+ if ((!singleLineComment && !multiLineComment)) {
query.append(character);
}
+
if (!query.toString().trim().isEmpty()) {
- queries.add(query.toString().trim());
+ queries.add(query.toString());
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 (character == '\n') {
+ query.append(character);
}
}
- return queries;
+ List<String> refinedQueries = new ArrayList<>();
+ for (int i = 0; i < queries.size(); ++i) {
+ String emptyLine = "";
+ if (i > 0) {
+ emptyLine = createEmptyLine(refinedQueries.get(i-1));
+ }
+ if (isSingleLineComment(queries.get(i)) || isMultipleLineComment(queries.get(i))) {
+ // refine the last refinedQuery
+ if (refinedQueries.size() > 0) {
+ String lastRefinedQuery = refinedQueries.get(refinedQueries.size() - 1);
+ refinedQueries.set(refinedQueries.size() - 1,
+ lastRefinedQuery + createEmptyLine(queries.get(i)));
+ }
+ } else {
+ String refinedQuery = emptyLine + queries.get(i);
+ refinedQueries.add(refinedQuery);
+ }
+ }
+
+ return refinedQueries;
+ }
+
+ private boolean isSingleLineComment(String text) {
+ return text.trim().startsWith("--");
+ }
+
+ private boolean isMultipleLineComment(String text) {
+ return text.trim().startsWith("/*") && text.trim().endsWith("*/");
+ }
+
+ private String createEmptyLine(String text) {
+ StringBuilder builder = new StringBuilder();
+ for (int i = 0; i < text.length(); ++i) {
+ if (text.charAt(i) == '\n') {
+ builder.append('\n');
+ }
+ }
+ return builder.toString();
}
private boolean isSingleLineComment(char curChar, char nextChar) {
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 f112002..00fa5fd 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
@@ -41,15 +41,24 @@ public class SqlSplitterTest {
assertEquals(1, sqls.size());
assertEquals("show tables", sqls.get(0));
+ sqls = sqlSplitter.splitSql("\nshow tables;");
+ assertEquals(1, sqls.size());
+ assertEquals("\nshow tables", sqls.get(0));
+
sqls = sqlSplitter.splitSql("show tables;\nselect * from table_1");
assertEquals(2, sqls.size());
assertEquals("show tables", sqls.get(0));
- assertEquals("select * from table_1", sqls.get(1));
+ assertEquals("\nselect * from table_1", sqls.get(1));
+
+ sqls = sqlSplitter.splitSql("show tables;\n\nselect * from table_1");
+ assertEquals(2, sqls.size());
+ assertEquals("show tables", sqls.get(0));
+ assertEquals("\n\nselect * from table_1", sqls.get(1));
sqls = sqlSplitter.splitSql("show\ntables;\nselect * \nfrom table_1");
assertEquals(2, sqls.size());
assertEquals("show\ntables", sqls.get(0));
- assertEquals("select * \nfrom table_1", sqls.get(1));
+ assertEquals("\n\nselect * \nfrom table_1", sqls.get(1));
}
@@ -74,26 +83,26 @@ public class SqlSplitterTest {
sqls = sqlSplitter.splitSql("--comment_1;\nshow tables");
assertEquals(1, sqls.size());
- assertEquals("show tables", sqls.get(0));
+ assertEquals("\nshow tables", sqls.get(0));
sqls = sqlSplitter.splitSql("--comment_1;\nshow tables;\n--comment_2");
assertEquals(1, sqls.size());
- assertEquals("show tables", sqls.get(0));
+ assertEquals("\nshow tables", sqls.get(0));
sqls = sqlSplitter.splitSql("show tables;\ndescribe table_1");
assertEquals(2, sqls.size());
assertEquals("show tables", sqls.get(0));
- assertEquals("describe table_1", sqls.get(1));
+ assertEquals("\ndescribe table_1", sqls.get(1));
sqls = sqlSplitter.splitSql("show tables;\n--comment_1;\ndescribe table_1");
assertEquals(2, sqls.size());
assertEquals("show tables", sqls.get(0));
- assertEquals("describe table_1", sqls.get(1));
+ assertEquals("\n\ndescribe table_1", sqls.get(1));
sqls = sqlSplitter.splitSql("select a\nfrom table_1;\ndescribe table_1;--comment_1");
assertEquals(2, sqls.size());
assertEquals("select a\nfrom table_1", sqls.get(0));
- assertEquals("describe table_1", sqls.get(1));
+ assertEquals("\n\ndescribe table_1", sqls.get(1));
sqls = sqlSplitter.splitSql("--comment_1;\n--comment_2\n");
assertEquals(0, sqls.size());
@@ -104,12 +113,12 @@ public class SqlSplitterTest {
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));
+ assertEquals("\nselect a from table_1\n", 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));
+ assertEquals("\nselect a from table_1", sqls.get(0));
+ assertEquals("\n\n\nselect b from table_1", sqls.get(1));
}
@Test
@@ -133,21 +142,21 @@ public class SqlSplitterTest {
sqls = sqlSplitter.splitSql("/*comment_1;*/\nshow tables");
assertEquals(1, sqls.size());
- assertEquals("show tables", sqls.get(0));
+ assertEquals("\nshow tables", sqls.get(0));
sqls = sqlSplitter.splitSql("/*comment_1*;*/\nshow tables;\n/*--comment_2*/");
assertEquals(1, sqls.size());
- assertEquals("show tables", sqls.get(0));
+ assertEquals("\nshow tables", sqls.get(0));
sqls = sqlSplitter.splitSql("show tables;\n/*comment_1;*/\ndescribe table_1");
assertEquals(2, sqls.size());
assertEquals("show tables", sqls.get(0));
- assertEquals("describe table_1", sqls.get(1));
+ assertEquals("\n\ndescribe table_1", sqls.get(1));
sqls = sqlSplitter.splitSql("select a\nfrom table_1;\ndescribe table_1;/*comment_1*/");
assertEquals(2, sqls.size());
assertEquals("select a\nfrom table_1", sqls.get(0));
- assertEquals("describe table_1", sqls.get(1));
+ assertEquals("\n\ndescribe table_1", sqls.get(1));
sqls = sqlSplitter.splitSql("/*comment_1;*/\n/*comment_2*/\n");
assertEquals(0, sqls.size());
@@ -158,17 +167,17 @@ public class SqlSplitterTest {
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));
+ assertEquals("\nselect a from table_1\n", 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));
+ assertEquals("\nselect a from table_1", sqls.get(0));
+ assertEquals("\n\nselect 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));
+ assertEquals("\nselect a /*+ hint*/ from table_1", sqls.get(0));
+ assertEquals("\n\nselect b from table_1", sqls.get(1));
}
@Test
@@ -180,7 +189,7 @@ public class SqlSplitterTest {
sqls = sqlSplitter.splitSql("--comment_1;\nselect a from table_1 where a=' and b=1");
assertEquals(1, sqls.size());
- assertEquals("select a from table_1 where a=' and b=1", sqls.get(0));
+ assertEquals("\nselect a from table_1 where a=' and b=1", sqls.get(0));
sqls = sqlSplitter.splitSql("select a from table_1 where a=' and b=1;\n--comment_1");
assertEquals(1, sqls.size());
@@ -193,7 +202,7 @@ public class SqlSplitterTest {
String text = "/* ; */\n" +
"-- /* comment\n" +
"--select * from test_table\n" +
- "select * from test_table; /* some comment ; */\n" +
+ "select * from test_table;/* some comment ; */\n" +
"/*\n" +
"select * from test_table;\n" +
"*/\n" +
@@ -202,9 +211,24 @@ public class SqlSplitterTest {
"select * from test_table WHERE ID = '/*'; -- test";
List<String> sqls = sqlSplitter.splitSql(text);
assertEquals(3, sqls.size());
- assertEquals("select * from test_table", sqls.get(0));
- assertEquals("select * from test_table WHERE ID = ';--'", sqls.get(1));
- assertEquals("select * from test_table WHERE ID = '/*'", sqls.get(2));
+ assertEquals("\n\n\nselect * from test_table", sqls.get(0));
+ assertEquals("\n\n\n\n\n\n\n\nselect * from test_table WHERE ID = ';--'", sqls.get(1));
+ assertEquals("\n\n\n\n\n\n\n\n\nselect * from test_table WHERE ID = '/*'", sqls.get(2));
+
+ text = "\n" +
+ "\n" +
+ "-- comment 1;\n" +
+ "-- comment 2;\n" +
+ "show databases;\n" +
+ "\n" +
+ "-- another comment\n" +
+ "-- another comment2\n" +
+ "show \n" +
+ "table\n";
+ sqls = sqlSplitter.splitSql(text);
+ assertEquals(2, sqls.size());
+ assertEquals("\n\n\n\nshow databases", sqls.get(0));
+ assertEquals("\n\n\n\n\n\n\n\nshow \ntable\n", sqls.get(1));
}
@Test
@@ -228,26 +252,26 @@ public class SqlSplitterTest {
sqls = sqlSplitter.splitSql("//comment_1;\nshow tables");
assertEquals(1, sqls.size());
- assertEquals("show tables", sqls.get(0));
+ assertEquals("\nshow tables", sqls.get(0));
sqls = sqlSplitter.splitSql("//comment_1;\nshow tables;\n//comment_2");
assertEquals(1, sqls.size());
- assertEquals("show tables", sqls.get(0));
+ assertEquals("\nshow tables", sqls.get(0));
sqls = sqlSplitter.splitSql("show tables;\ndescribe table_1");
assertEquals(2, sqls.size());
assertEquals("show tables", sqls.get(0));
- assertEquals("describe table_1", sqls.get(1));
+ assertEquals("\ndescribe table_1", sqls.get(1));
sqls = sqlSplitter.splitSql("show tables;\n//comment_1;\ndescribe table_1");
assertEquals(2, sqls.size());
assertEquals("show tables", sqls.get(0));
- assertEquals("describe table_1", sqls.get(1));
+ assertEquals("\n\ndescribe table_1", sqls.get(1));
sqls = sqlSplitter.splitSql("select a\nfrom table_1;\ndescribe table_1;//comment_1");
assertEquals(2, sqls.size());
assertEquals("select a\nfrom table_1", sqls.get(0));
- assertEquals("describe table_1", sqls.get(1));
+ assertEquals("\n\ndescribe table_1", sqls.get(1));
sqls = sqlSplitter.splitSql("//comment_1;\n//comment_2\n");
assertEquals(0, sqls.size());
@@ -278,26 +302,26 @@ public class SqlSplitterTest {
sqls = sqlSplitter.splitSql("#comment_1;\nshow tables");
assertEquals(1, sqls.size());
- assertEquals("show tables", sqls.get(0));
+ assertEquals("\nshow tables", sqls.get(0));
sqls = sqlSplitter.splitSql("#comment_1;\nshow tables;\n#comment_2");
assertEquals(1, sqls.size());
- assertEquals("show tables", sqls.get(0));
+ assertEquals("\nshow tables", sqls.get(0));
sqls = sqlSplitter.splitSql("show tables;\ndescribe table_1");
assertEquals(2, sqls.size());
assertEquals("show tables", sqls.get(0));
- assertEquals("describe table_1", sqls.get(1));
+ assertEquals("\ndescribe table_1", sqls.get(1));
sqls = sqlSplitter.splitSql("show tables;\n#comment_1;\ndescribe table_1");
assertEquals(2, sqls.size());
assertEquals("show tables", sqls.get(0));
- assertEquals("describe table_1", sqls.get(1));
+ assertEquals("\n\ndescribe table_1", sqls.get(1));
sqls = sqlSplitter.splitSql("select a\nfrom table_1;\ndescribe table_1;#comment_1");
assertEquals(2, sqls.size());
assertEquals("select a\nfrom table_1", sqls.get(0));
- assertEquals("describe table_1", sqls.get(1));
+ assertEquals("\n\ndescribe table_1", sqls.get(1));
sqls = sqlSplitter.splitSql("#comment_1;\n#comment_2\n");
assertEquals(0, sqls.size());
diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/socket/NotebookServerTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/socket/NotebookServerTest.java
index 0936cff..636cb47 100644
--- a/zeppelin-server/src/test/java/org/apache/zeppelin/socket/NotebookServerTest.java
+++ b/zeppelin-server/src/test/java/org/apache/zeppelin/socket/NotebookServerTest.java
@@ -472,7 +472,7 @@ public class NotebookServerTest extends AbstractTestRestApi {
assertTrue(notebook.getNote(note.getId()).getName(),
notebook.getNote(note.getId()).getName().startsWith("Note converted from Jupyter_"));
assertEquals("md", notebook.getNote(note.getId()).getParagraphs().get(0).getIntpText());
- assertEquals("# matplotlib - 2D and 3D plotting in Python",
+ assertEquals("\n# matplotlib - 2D and 3D plotting in Python",
notebook.getNote(note.getId()).getParagraphs().get(0).getScriptText());
} finally {
if (note != null) {
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/ParagraphTextParser.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/ParagraphTextParser.java
index e534c32..1f9fe07 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/ParagraphTextParser.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/ParagraphTextParser.java
@@ -176,13 +176,31 @@ public class ParagraphTextParser {
if (startPos < text.length() && text.charAt(startPos) == '(') {
startPos = parseLocalProperties(text, startPos, localProperties);
}
- scriptText = text.substring(startPos).trim();
+ scriptText = text.substring(startPos);
} else {
intpText = "";
- scriptText = text.trim();
+ scriptText = text;
}
- return new ParseResult(intpText, scriptText, localProperties);
+ return new ParseResult(intpText, removeLeadingWhiteSpaces(scriptText), localProperties);
}
+ /**
+ * Strip the leading white spaces but not line separator.
+ *
+ * @param text
+ * @return
+ */
+ private static String removeLeadingWhiteSpaces(String text) {
+ int startPos = 0;
+ for (int i = 0; i < text.length(); ++i) {
+ if (text.charAt(i) == ' ') {
+ continue;
+ } else {
+ startPos = i;
+ break;
+ }
+ }
+ return text.substring(startPos);
+ }
}
diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/mock/MockInterpreter1.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/mock/MockInterpreter1.java
index 500c4f7..0c7e5df 100644
--- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/mock/MockInterpreter1.java
+++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/mock/MockInterpreter1.java
@@ -66,7 +66,7 @@ public class MockInterpreter1 extends Interpreter {
@Override
public InterpreterResult interpret(String st, InterpreterContext context) {
InterpreterResult result;
-
+ st = st.trim();
if ("getId".equals(st)) {
// get unique id of this interpreter instance
result = new InterpreterResult(InterpreterResult.Code.SUCCESS, "" + this.object_id + "-" + this.pid);
diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/mock/MockInterpreter2.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/mock/MockInterpreter2.java
index f36df56..43f9f51 100644
--- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/mock/MockInterpreter2.java
+++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/mock/MockInterpreter2.java
@@ -56,7 +56,7 @@ public class MockInterpreter2 extends Interpreter{
@Override
public InterpreterResult interpret(String st, InterpreterContext context) {
InterpreterResult result;
-
+ st = st.trim();
if ("getId".equals(st)) {
// get unique id of this interpreter instance
result = new InterpreterResult(InterpreterResult.Code.SUCCESS, "" + this.hashCode());
diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/ParagraphTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/ParagraphTest.java
index 4bd41f9..a4414ac 100644
--- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/ParagraphTest.java
+++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/ParagraphTest.java
@@ -171,23 +171,23 @@ public class ParagraphTest extends AbstractInterpreterTest {
Paragraph paragraph = new Paragraph(note, null);
paragraph.setText("%test\r\n###Hello");
assertEquals("test", paragraph.getIntpText());
- assertEquals("###Hello", paragraph.getScriptText());
+ assertEquals("\r\n###Hello", paragraph.getScriptText());
paragraph.setText("%test\t###Hello");
assertEquals("test", paragraph.getIntpText());
- assertEquals("###Hello", paragraph.getScriptText());
+ assertEquals("\t###Hello", paragraph.getScriptText());
paragraph.setText("%test\u000b###Hello");
assertEquals("test", paragraph.getIntpText());
- assertEquals("###Hello", paragraph.getScriptText());
+ assertEquals("\u000b###Hello", paragraph.getScriptText());
paragraph.setText("%test\f###Hello");
assertEquals("test", paragraph.getIntpText());
- assertEquals("###Hello", paragraph.getScriptText());
+ assertEquals("\f###Hello", paragraph.getScriptText());
paragraph.setText("%test\n###Hello");
assertEquals("test", paragraph.getIntpText());
- assertEquals("###Hello", paragraph.getScriptText());
+ assertEquals("\n###Hello", paragraph.getScriptText());
paragraph.setText("%test ###Hello");
assertEquals("test", paragraph.getIntpText());
@@ -334,15 +334,15 @@ public class ParagraphTest extends AbstractInterpreterTest {
Triple.of(" %jdbc schema.tab\n\n", 18, 10),
Triple.of(" \n%jdbc schema.\n \n", 16, 7),
Triple.of(" \n%jdbc schema.\n \n", 16, 7),
- Triple.of(" \n%jdbc\n\n schema\n \n", 17, 6),
- Triple.of("%another\n\n schema.", 18, 7),
- Triple.of("\n\n schema.", 10, 7),
+ Triple.of(" \n%jdbc\n\n schema\n \n", 17, 9),
+ Triple.of("%another\n\n schema.", 18, 10),
+ Triple.of("\n\n schema.", 10, 10),
Triple.of("schema.", 7, 7),
Triple.of("schema. \n", 7, 7),
Triple.of(" \n %jdbc", 11, 0),
Triple.of("\n %jdbc", 9, 0),
- Triple.of("%jdbc \n schema", 16, 6),
- Triple.of("%jdbc \n \n schema", 20, 6)
+ Triple.of("%jdbc \n schema", 16, 9),
+ Triple.of("%jdbc \n \n schema", 20, 13)
);
for (Triple<String, Integer, Integer> data : dataSet) {
diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/ParagraphTextParserTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/ParagraphTextParserTest.java
index f3ef227..31f750f 100644
--- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/ParagraphTextParserTest.java
+++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/ParagraphTextParserTest.java
@@ -43,7 +43,7 @@ public class ParagraphTextParserTest {
assertEquals("cassandra", parseResult.getIntpText());
assertEquals(4, parseResult.getLocalProperties().size());
assertEquals("E, d MMM yy", parseResult.getLocalProperties().get("timeFormat"));
- assertEquals("select * from system_auth.roles;", parseResult.getScriptText());
+ assertEquals("\nselect * from system_auth.roles;", parseResult.getScriptText());
}
@Test
@@ -75,10 +75,10 @@ public class ParagraphTextParserTest {
@Test
public void testParagraphTextNoLocalProperties() {
- ParagraphTextParser.ParseResult parseResult = ParagraphTextParser.parse("%spark.pyspark sc.version");
+ ParagraphTextParser.ParseResult parseResult = ParagraphTextParser.parse("%spark.pyspark\nsc.version");
assertEquals("spark.pyspark", parseResult.getIntpText());
assertEquals(0, parseResult.getLocalProperties().size());
- assertEquals("sc.version", parseResult.getScriptText());
+ assertEquals("\nsc.version", parseResult.getScriptText());
}
@Test