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 2018/01/19 03:44:11 UTC
zeppelin git commit: [ZEPPELIN-3098] Livy Interpreter fails if row
contains newline or tab
Repository: zeppelin
Updated Branches:
refs/heads/master c48aba3e9 -> 2d36f848f
[ZEPPELIN-3098] Livy Interpreter fails if row contains newline or tab
### What is this PR for?
This PR fixes the issue of newlines and tabs breaking results in the SQL interpreter in Livy.
The Livy interpreter will return incorrect results if a row contains \n or \t characters.
In the case of the newline, the result will be:
Line is missing from results if the \n appears anywhere but the end of a cell
`String index out of range: 17` if it appears at the end of a cell
In the case of the tab, the result will be misaligned columns if the tab appears in the middle of a cell
The output showing these error is attached to the Jira.
I have changed the parsing and any newline or tab characters will be escaped
### What type of PR is it?
Bug Fix
### What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-3098
### How should this be tested?
Unittests have been added
### 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: Alex Bush <al...@coxauto.co.uk>
Author: Alex Bush <bu...@users.noreply.github.com>
Closes #2701 from bushnoh/zeppelin-3098 and squashes the following commits:
9f2f6e3 [Alex Bush] Merge pull request #3 from apache/master
14e120d [Alex Bush] Merge pull request #2 from apache/master
46981fd [Alex Bush] Merge pull request #1 from apache/master
ee5a41b [Alex Bush] Merge remote-tracking branch 'upstream/master'
67a93b5 [Alex Bush] Merge branch 'master' of https://github.com/bushnoh/zeppelin
31cdbdc [Alex Bush] Added another comment explaining the regexp change
d054af0 [Alex Bush] Force a dummy change for Travis
64a42be [Alex Bush] Fix for newline and tab in data
Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo
Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/2d36f848
Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/2d36f848
Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/2d36f848
Branch: refs/heads/master
Commit: 2d36f848fc1936a1c8090cf1220b9cd8b089665b
Parents: c48aba3
Author: Alex Bush <al...@coxauto.co.uk>
Authored: Thu Jan 11 16:10:44 2018 +0000
Committer: Jeff Zhang <zj...@apache.org>
Committed: Fri Jan 19 11:44:07 2018 +0800
----------------------------------------------------------------------
.../zeppelin/livy/LivySparkSQLInterpreter.java | 28 +++++++++----
.../zeppelin/livy/LivySQLInterpreterTest.java | 44 ++++++++++++++++++++
2 files changed, 64 insertions(+), 8 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/2d36f848/livy/src/main/java/org/apache/zeppelin/livy/LivySparkSQLInterpreter.java
----------------------------------------------------------------------
diff --git a/livy/src/main/java/org/apache/zeppelin/livy/LivySparkSQLInterpreter.java b/livy/src/main/java/org/apache/zeppelin/livy/LivySparkSQLInterpreter.java
index b93626d..7b2d7d6 100644
--- a/livy/src/main/java/org/apache/zeppelin/livy/LivySparkSQLInterpreter.java
+++ b/livy/src/main/java/org/apache/zeppelin/livy/LivySparkSQLInterpreter.java
@@ -18,6 +18,7 @@
package org.apache.zeppelin.livy;
import org.apache.commons.lang.StringUtils;
+import static org.apache.commons.lang.StringEscapeUtils.escapeJavaScript;
import org.apache.zeppelin.interpreter.*;
import org.apache.zeppelin.scheduler.Scheduler;
import org.apache.zeppelin.scheduler.SchedulerFactory;
@@ -26,7 +27,6 @@ import java.util.ArrayList;
import java.util.List;
import java.util.Properties;
-
/**
* Livy SparkSQL Interpreter for Zeppelin.
*/
@@ -166,15 +166,18 @@ public class LivySparkSQLInterpreter extends BaseLivyInterpreter {
protected List<String> parseSQLOutput(String output) {
List<String> rows = new ArrayList<>();
- String[] lines = output.split("\n");
+ // Get first line by breaking on \n. We can guarantee
+ // that \n marks the end of the first line, but not for
+ // subsequent lines (as it could be in the cells)
+ String firstLine = output.split("\n", 2)[0];
// at least 4 lines, even for empty sql output
// +---+---+
// | a| b|
// +---+---+
// +---+---+
- // use the first line to determinte the position of feach cell
- String[] tokens = StringUtils.split(lines[0], "\\+");
+ // use the first line to determine the position of each cell
+ String[] tokens = StringUtils.split(firstLine, "\\+");
// pairs keeps the start/end position of each cell. We parse it from the first row
// which use '+' as separator
List<Pair> pairs = new ArrayList<>();
@@ -186,17 +189,26 @@ public class LivySparkSQLInterpreter extends BaseLivyInterpreter {
pairs.add(new Pair(start, end));
}
- for (String line : lines) {
+ // Use the header line to determine the position
+ // of subsequent lines
+ int lineStart = 0;
+ int lineEnd = firstLine.length();
+ while (lineEnd < output.length()) {
// Only match format "|....|"
// skip line like "+---+---+" and "only showing top 1 row"
- if (line.matches("^\\|.*\\|$")) {
+ String line = output.substring(lineStart, lineEnd);
+ // Use the DOTALL regex mode to match newlines
+ if (line.matches("(?s)^\\|.*\\|$")) {
List<String> cells = new ArrayList<>();
for (Pair pair : pairs) {
- // strip the blank space around the cell
- cells.add(line.substring(pair.start, pair.end).trim());
+ // strip the blank space around the cell and escape the string
+ cells.add(escapeJavaScript(line.substring(pair.start, pair.end)).trim());
}
rows.add(StringUtils.join(cells, "\t"));
}
+ // Determine position of next line skipping newline
+ lineStart += firstLine.length() + 1;
+ lineEnd = lineStart + firstLine.length();
}
return rows;
}
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/2d36f848/livy/src/test/java/org/apache/zeppelin/livy/LivySQLInterpreterTest.java
----------------------------------------------------------------------
diff --git a/livy/src/test/java/org/apache/zeppelin/livy/LivySQLInterpreterTest.java b/livy/src/test/java/org/apache/zeppelin/livy/LivySQLInterpreterTest.java
index 24d70ec..0541b87 100644
--- a/livy/src/test/java/org/apache/zeppelin/livy/LivySQLInterpreterTest.java
+++ b/livy/src/test/java/org/apache/zeppelin/livy/LivySQLInterpreterTest.java
@@ -124,5 +124,49 @@ public class LivySQLInterpreterTest {
assertEquals(2, rows.size());
assertEquals("a", rows.get(0));
assertEquals("1", rows.get(1));
+
+
+ // sql output with 3 rows, 3 columns, showing "only showing top 3 rows" with a line break in the data
+ // +---+---+---+
+ // | a| b| c|
+ // +---+---+---+
+ // | 1a| 1b| 1c|
+ // | 2a| 2
+ // b| 2c|
+ // | 3a| 3b| 3c|
+ // +---+---+---+
+ // only showing top 3 rows
+ rows = sqlInterpreter.parseSQLOutput("+---+----+---+\n" +
+ "| a| b| c|\n" +
+ "+---+----+---+\n" +
+ "| 1a| 1b| 1c|\n" +
+ "| 2a| 2\nb| 2c|\n" +
+ "| 3a| 3b| 3c|\n" +
+ "+---+---+---+\n" +
+ "only showing top 3 rows");
+ assertEquals(4, rows.size());
+ assertEquals("a\tb\tc", rows.get(0));
+ assertEquals("1a\t1b\t1c", rows.get(1));
+ assertEquals("2a\t2\\nb\t2c", rows.get(2));
+ assertEquals("3a\t3b\t3c", rows.get(3));
+
+
+ // sql output with 2 rows and one containing a tab
+ // +---+---+
+ // | a| b|
+ // +---+---+
+ // | 1| \ta|
+ // | 2| 2b|
+ // +---+---+
+ rows = sqlInterpreter.parseSQLOutput("+---+---+\n" +
+ "| a| b|\n" +
+ "+---+---+\n" +
+ "| 1| \ta|\n" +
+ "| 2| 2b|\n" +
+ "+---+---+");
+ assertEquals(3, rows.size());
+ assertEquals("a\tb", rows.get(0));
+ assertEquals("1\t\\ta", rows.get(1));
+ assertEquals("2\t2b", rows.get(2));
}
}