Repository: zeppelin
Updated Branches:
  refs/heads/master bd3b05316 -> 09b75c86c


[ZEPPELIN-2080] Fix null column parse issue in KylinInterpreter

### What is this PR for?
KylinInterpreter use regex pattern "\"results\":\\[\\[\"(.*?)\"]]" to extract 
the result, but if the last column is null, then no quotes in that field. The 
regex pattern will fail to match.
The KylinInterpreter should consider to support the null field case.

### What type of PR is it?
[Bug Fix]

### Todos

### What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-2080

### How should this be tested?
Has prepared the testcase in UnitTest

### 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: Billy Liu <billy...@apache.org>

Closes #1988 from yiming187/ZEPPELIN-2080 and squashes the following commits:

3e34314 [Billy Liu] [ZEPPELIN-2080] Fix wrong column parse in KylinInterpreter


Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo
Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/09b75c86
Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/09b75c86
Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/09b75c86

Branch: refs/heads/master
Commit: 09b75c86c8665730885f93282a690ffd22cd7e1a
Parents: bd3b053
Author: Billy Liu <billy...@apache.org>
Authored: Fri Feb 10 14:15:44 2017 +0800
Committer: Lee moon soo <m...@apache.org>
Committed: Sun Feb 19 15:24:10 2017 +0900

----------------------------------------------------------------------
 .../apache/zeppelin/kylin/KylinInterpreter.java |  15 +-
 kylin/src/test/java/KylinInterpreterTest.java   | 234 ----------------
 .../zeppelin/kylin/KylinInterpreterTest.java    | 269 +++++++++++++++++++
 3 files changed, 278 insertions(+), 240 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/zeppelin/blob/09b75c86/kylin/src/main/java/org/apache/zeppelin/kylin/KylinInterpreter.java
----------------------------------------------------------------------
diff --git 
a/kylin/src/main/java/org/apache/zeppelin/kylin/KylinInterpreter.java 
b/kylin/src/main/java/org/apache/zeppelin/kylin/KylinInterpreter.java
index c831c24..5969717 100755
--- a/kylin/src/main/java/org/apache/zeppelin/kylin/KylinInterpreter.java
+++ b/kylin/src/main/java/org/apache/zeppelin/kylin/KylinInterpreter.java
@@ -25,7 +25,6 @@ import org.apache.http.entity.StringEntity;
 import org.apache.http.impl.client.HttpClientBuilder;
 import org.apache.zeppelin.interpreter.Interpreter;
 import org.apache.zeppelin.interpreter.InterpreterContext;
-import org.apache.zeppelin.interpreter.InterpreterPropertyBuilder;
 import org.apache.zeppelin.interpreter.InterpreterResult;
 import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion;
 import org.slf4j.Logger;
@@ -53,7 +52,8 @@ public class KylinInterpreter extends Interpreter {
   static final String KYLIN_QUERY_LIMIT = "kylin.query.limit";
   static final String KYLIN_QUERY_ACCEPT_PARTIAL = "kylin.query.ispartial";
   static final Pattern KYLIN_TABLE_FORMAT_REGEX_LABEL = 
Pattern.compile("\"label\":\"(.*?)\"");
-  static final Pattern KYLIN_TABLE_FORMAT_REGEX = 
Pattern.compile("\"results\":\\[\\[\"(.*?)\"]]");
+  static final Pattern KYLIN_TABLE_FORMAT_REGEX_RESULTS =
+          Pattern.compile("\"results\":\\[\\[(.*?)]]");
 
   public KylinInterpreter(Properties property) {
     super(property);
@@ -189,7 +189,7 @@ public class KylinInterpreter extends Interpreter {
     return rett;
   }
 
-  private String formatResult(String msg) {
+  String formatResult(String msg) {
     StringBuilder res = new StringBuilder("%table ");
     
     Matcher ml = KYLIN_TABLE_FORMAT_REGEX_LABEL.matcher(msg);
@@ -198,16 +198,19 @@ public class KylinInterpreter extends Interpreter {
     } 
     res.append(" \n");
     
-    Matcher mr = KYLIN_TABLE_FORMAT_REGEX.matcher(msg);
+    Matcher mr = KYLIN_TABLE_FORMAT_REGEX_RESULTS.matcher(msg);
     String table = null;
     while (!mr.hitEnd() && mr.find()) {
       table = mr.group(1);
     }
 
-    String[] row = table.split("\"],\\[\"");
+    String[] row = table.split("],\\[");
     for (int i = 0; i < row.length; i++) {
-      String[] col = row[i].split("\",\"");
+      String[] col = row[i].split(",(?=(?:[^\"]*\"[^\"]*\")*[^\"]*$)", -1);
       for (int j = 0; j < col.length; j++) {
+        if (col[j] != null) {
+          col[j] = col[j].replaceAll("^\"|\"$", "");
+        }
         res.append(col[j] + " \t");
       }
       res.append(" \n");

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/09b75c86/kylin/src/test/java/KylinInterpreterTest.java
----------------------------------------------------------------------
diff --git a/kylin/src/test/java/KylinInterpreterTest.java 
b/kylin/src/test/java/KylinInterpreterTest.java
deleted file mode 100755
index 6cee13b..0000000
--- a/kylin/src/test/java/KylinInterpreterTest.java
+++ /dev/null
@@ -1,234 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements.  See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License.  You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-import org.apache.http.*;
-import org.apache.http.client.methods.HttpPost;
-import org.apache.http.message.AbstractHttpMessage;
-import org.apache.zeppelin.interpreter.InterpreterResult;
-import org.apache.zeppelin.kylin.KylinInterpreter;
-import org.junit.BeforeClass;
-import org.junit.Test;
-
-import java.io.ByteArrayInputStream;
-import java.io.IOException;
-import java.io.InputStream;
-import java.io.OutputStream;
-import java.util.Locale;
-import java.util.Properties;
-
-import static org.junit.Assert.assertEquals;
-
-public class KylinInterpreterTest {
-  static final Properties kylinProperties = new Properties();
-
-  @BeforeClass
-  public static void setUpClass() {
-    kylinProperties.put("kylin.api.url", 
"http://localhost:7070/kylin/api/query";);
-    kylinProperties.put("kylin.api.user", "ADMIN");
-    kylinProperties.put("kylin.api.password", "KYLIN");
-    kylinProperties.put("kylin.query.project", "default");
-    kylinProperties.put("kylin.query.offset", "0");
-    kylinProperties.put("kylin.query.limit", "5000");
-    kylinProperties.put("kylin.query.ispartial", "true");
-  }
-
-  @Test
-  public void testWithDefault(){
-    KylinInterpreter t = new MockKylinInterpreter(getDefaultProperties());
-    InterpreterResult result = t.interpret(
-        "select a.date,sum(b.measure) as measure from kylin_fact_table a " +
-            "inner join kylin_lookup_table b on a.date=b.date group by 
a.date", null);
-    assertEquals("default", t.getProject("select a.date,sum(b.measure) as 
measure " +
-                    "from kylin_fact_table a inner join kylin_lookup_table b 
on a.date=b.date group by a.date"));
-    
assertEquals(InterpreterResult.Type.TABLE,result.message().get(0).getType());
-  }
-
-  @Test
-  public void testWithProject(){
-    KylinInterpreter t = new MockKylinInterpreter(getDefaultProperties());
-    assertEquals("project2", t.getProject("(project2)\n select 
a.date,sum(b.measure) as measure " +
-            "from kylin_fact_table a inner join kylin_lookup_table b on 
a.date=b.date group by a.date"));
-    assertEquals("", t.getProject("()\n select a.date,sum(b.measure) as 
measure " +
-            "from kylin_fact_table a inner join kylin_lookup_table b on 
a.date=b.date group by a.date"));
-    assertEquals("\n select a.date,sum(b.measure) as measure from 
kylin_fact_table a inner join " +
-            "kylin_lookup_table b on a.date=b.date group by a.date", 
t.getSQL("(project2)\n select a.date," +
-            "sum(b.measure) as measure from kylin_fact_table a inner join 
kylin_lookup_table b on a.date=b.date " +
-            "group by a.date"));
-    assertEquals("\n select a.date,sum(b.measure) as measure from 
kylin_fact_table a inner join kylin_lookup_table b " +
-            "on a.date=b.date group by a.date", t.getSQL("()\n select 
a.date,sum(b.measure) as measure " +
-            "from kylin_fact_table a inner join kylin_lookup_table b on 
a.date=b.date group by a.date"));
-  }
-
-  private Properties getDefaultProperties(){
-    Properties prop = new Properties();
-    prop.put("kylin.api.username", "ADMIN");
-    prop.put("kylin.api.password", "KYLIN");
-    prop.put("kylin.api.url", "http://<host>:<port>/kylin/api/query");
-    prop.put("kylin.query.project", "default");
-    prop.put("kylin.query.offset", "0");
-    prop.put("kylin.query.limit", "5000");
-    prop.put("kylin.query.ispartial", "true");
-    return prop;
-  }
-}
-
-class MockKylinInterpreter extends KylinInterpreter {
-
-  public MockKylinInterpreter(Properties property) {
-    super(property);
-  }
-
-  @Override
-  public HttpResponse prepareRequest(String sql) throws IOException {
-    MockHttpClient client = new MockHttpClient();
-    return client.execute(new HttpPost());
-  }
-
-}
-
-class MockHttpClient{
-  public MockHttpResponse execute(HttpPost post){
-    return new MockHttpResponse();
-  }
-}
-
-class MockHttpResponse extends AbstractHttpMessage implements HttpResponse{
-
-  @Override
-  public StatusLine getStatusLine() {
-    return new MockStatusLine();
-  }
-
-  @Override
-  public void setStatusLine(StatusLine statusLine) {
-
-  }
-
-  @Override
-  public void setStatusLine(ProtocolVersion protocolVersion, int i) {
-
-  }
-
-  @Override
-  public void setStatusLine(ProtocolVersion protocolVersion, int i, String s) {
-
-  }
-
-  @Override
-  public void setStatusCode(int i) throws IllegalStateException {
-
-  }
-
-  @Override
-  public void setReasonPhrase(String s) throws IllegalStateException {
-
-  }
-
-  @Override
-  public HttpEntity getEntity() {
-    return new MockEntity();
-  }
-
-  @Override
-  public void setEntity(HttpEntity httpEntity) {
-
-  }
-
-  @Override
-  public Locale getLocale() {
-    return null;
-  }
-
-  @Override
-  public void setLocale(Locale locale) {
-
-  }
-
-  @Override
-  public ProtocolVersion getProtocolVersion() {
-    return null;
-  }
-}
-
-class MockStatusLine implements StatusLine{
-
-  @Override
-  public ProtocolVersion getProtocolVersion() {
-    return null;
-  }
-
-  @Override
-  public int getStatusCode() {
-    return 200;
-  }
-
-  @Override
-  public String getReasonPhrase() {
-    return null;
-  }
-}
-
-class MockEntity implements HttpEntity{
-
-  @Override
-  public boolean isRepeatable() {
-    return false;
-  }
-
-  @Override
-  public boolean isChunked() {
-    return false;
-  }
-
-  @Override
-  public long getContentLength() {
-    return 0;
-  }
-
-  @Override
-  public Header getContentType() {
-    return null;
-  }
-
-  @Override
-  public Header getContentEncoding() {
-    return null;
-  }
-
-  @Override
-  public InputStream getContent() throws IOException, IllegalStateException {
-    return new ByteArrayInputStream(("{\"columnMetas\":" +
-        "[{\"label\":\"PART_DT\"},{\"label\":\"measure\"}]," +
-        "\"results\":[[\"2012-01-03\",\"917.4138\"]," +
-        "[\"2012-05-06\",\"592.4823\"]]}").getBytes());
-  }
-
-  @Override
-  public void writeTo(OutputStream outputStream) throws IOException {
-
-  }
-
-  @Override
-  public boolean isStreaming() {
-    return false;
-  }
-
-  @Override
-  public void consumeContent() throws IOException {
-
-  }
-}

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/09b75c86/kylin/src/test/java/org/apache/zeppelin/kylin/KylinInterpreterTest.java
----------------------------------------------------------------------
diff --git 
a/kylin/src/test/java/org/apache/zeppelin/kylin/KylinInterpreterTest.java 
b/kylin/src/test/java/org/apache/zeppelin/kylin/KylinInterpreterTest.java
new file mode 100755
index 0000000..4471a07
--- /dev/null
+++ b/kylin/src/test/java/org/apache/zeppelin/kylin/KylinInterpreterTest.java
@@ -0,0 +1,269 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zeppelin.kylin;
+
+import static org.junit.Assert.assertEquals;
+
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.util.Locale;
+import java.util.Properties;
+
+import org.apache.http.Header;
+import org.apache.http.HttpEntity;
+import org.apache.http.HttpResponse;
+import org.apache.http.ProtocolVersion;
+import org.apache.http.StatusLine;
+import org.apache.http.client.methods.HttpPost;
+import org.apache.http.message.AbstractHttpMessage;
+import org.apache.zeppelin.interpreter.InterpreterResult;
+import org.junit.Assert;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+public class KylinInterpreterTest {
+  static final Properties kylinProperties = new Properties();
+
+  @BeforeClass
+  public static void setUpClass() {
+    kylinProperties.put("kylin.api.url", 
"http://localhost:7070/kylin/api/query";);
+    kylinProperties.put("kylin.api.user", "ADMIN");
+    kylinProperties.put("kylin.api.password", "KYLIN");
+    kylinProperties.put("kylin.query.project", "default");
+    kylinProperties.put("kylin.query.offset", "0");
+    kylinProperties.put("kylin.query.limit", "5000");
+    kylinProperties.put("kylin.query.ispartial", "true");
+  }
+
+  @Test
+  public void testWithDefault() {
+    KylinInterpreter t = new MockKylinInterpreter(getDefaultProperties());
+    InterpreterResult result = t.interpret(
+        "select a.date,sum(b.measure) as measure from kylin_fact_table a " +
+            "inner join kylin_lookup_table b on a.date=b.date group by 
a.date", null);
+    assertEquals("default", t.getProject("select a.date,sum(b.measure) as 
measure " +
+                    "from kylin_fact_table a inner join kylin_lookup_table b 
on a.date=b.date group by a.date"));
+    
assertEquals(InterpreterResult.Type.TABLE,result.message().get(0).getType());
+  }
+
+  @Test
+  public void testWithProject() {
+    KylinInterpreter t = new MockKylinInterpreter(getDefaultProperties());
+    assertEquals("project2", t.getProject("(project2)\n select 
a.date,sum(b.measure) as measure " +
+            "from kylin_fact_table a inner join kylin_lookup_table b on 
a.date=b.date group by a.date"));
+    assertEquals("", t.getProject("()\n select a.date,sum(b.measure) as 
measure " +
+            "from kylin_fact_table a inner join kylin_lookup_table b on 
a.date=b.date group by a.date"));
+    assertEquals("\n select a.date,sum(b.measure) as measure from 
kylin_fact_table a inner join " +
+            "kylin_lookup_table b on a.date=b.date group by a.date", 
t.getSQL("(project2)\n select a.date," +
+            "sum(b.measure) as measure from kylin_fact_table a inner join 
kylin_lookup_table b on a.date=b.date " +
+            "group by a.date"));
+    assertEquals("\n select a.date,sum(b.measure) as measure from 
kylin_fact_table a inner join kylin_lookup_table b " +
+            "on a.date=b.date group by a.date", t.getSQL("()\n select 
a.date,sum(b.measure) as measure " +
+            "from kylin_fact_table a inner join kylin_lookup_table b on 
a.date=b.date group by a.date"));
+  }
+
+  @Test
+  public void testParseResult() {
+    String msg = 
"{\"columnMetas\":[{\"isNullable\":1,\"displaySize\":256,\"label\":\"COUNTRY\",\"name\":\"COUNTRY\","
+            + 
"\"schemaName\":\"DEFAULT\",\"catelogName\":null,\"tableName\":\"SALES_TABLE\",\"precision\":256,"
+            + 
"\"scale\":0,\"columnType\":12,\"columnTypeName\":\"VARCHAR\",\"writable\":false,\"readOnly\":true,"
+            + 
"\"definitelyWritable\":false,\"autoIncrement\":false,\"caseSensitive\":true,\"searchable\":false,"
+            + 
"\"currency\":false,\"signed\":true},{\"isNullable\":1,\"displaySize\":256,\"label\":\"CURRENCY\","
+            + 
"\"name\":\"CURRENCY\",\"schemaName\":\"DEFAULT\",\"catelogName\":null,\"tableName\":\"SALES_TABLE\","
+            + 
"\"precision\":256,\"scale\":0,\"columnType\":12,\"columnTypeName\":\"VARCHAR\",\"writable\":false,"
+            + 
"\"readOnly\":true,\"definitelyWritable\":false,\"autoIncrement\":false,\"caseSensitive\":true,"
+            + 
"\"searchable\":false,\"currency\":false,\"signed\":true},{\"isNullable\":0,\"displaySize\":19,"
+            + 
"\"label\":\"COUNT__\",\"name\":\"COUNT__\",\"schemaName\":\"DEFAULT\",\"catelogName\":null,"
+            + 
"\"tableName\":\"SALES_TABLE\",\"precision\":19,\"scale\":0,\"columnType\":-5,\"columnTypeName\":"
+            + 
"\"BIGINT\",\"writable\":false,\"readOnly\":true,\"definitelyWritable\":false,\"autoIncrement\":false,"
+            + 
"\"caseSensitive\":true,\"searchable\":false,\"currency\":false,\"signed\":true}],\"results\":"
+            + 
"[[\"AMERICA\",\"USD\",null],[null,\"RMB\",0],[\"KOR\",null,100],[\"\\\"abc\\\"\",\"a,b,c\",-1]],"
+            + 
"\"cube\":\"Sample_Cube\",\"affectedRowCount\":0,\"isException\":false,\"exceptionMessage\":null,"
+            + 
"\"duration\":134,\"totalScanCount\":1,\"hitExceptionCache\":false,\"storageCacheUsed\":false,"
+            + "\"partial\":false}";
+    String expected="%table COUNTRY \tCURRENCY \tCOUNT__ \t \n" +
+            "AMERICA \tUSD \tnull \t \n" +
+            "null \tRMB \t0 \t \n" +
+            "KOR \tnull \t100 \t \n" +
+            "\\\"abc\\\" \ta,b,c \t-1 \t \n";
+    KylinInterpreter t = new MockKylinInterpreter(getDefaultProperties());
+    String actual = t.formatResult(msg);
+    Assert.assertEquals(expected, actual);
+  }
+
+  private Properties getDefaultProperties() {
+    Properties prop = new Properties();
+    prop.put("kylin.api.username", "ADMIN");
+    prop.put("kylin.api.password", "KYLIN");
+    prop.put("kylin.api.url", "http://<host>:<port>/kylin/api/query");
+    prop.put("kylin.query.project", "default");
+    prop.put("kylin.query.offset", "0");
+    prop.put("kylin.query.limit", "5000");
+    prop.put("kylin.query.ispartial", "true");
+    return prop;
+  }
+}
+
+class MockKylinInterpreter extends KylinInterpreter {
+
+  public MockKylinInterpreter(Properties property) {
+    super(property);
+  }
+
+  @Override
+  public HttpResponse prepareRequest(String sql) throws IOException {
+    MockHttpClient client = new MockHttpClient();
+    return client.execute(new HttpPost());
+  }
+
+}
+
+class MockHttpClient{
+  public MockHttpResponse execute(HttpPost post){
+    return new MockHttpResponse();
+  }
+}
+
+class MockHttpResponse extends AbstractHttpMessage implements HttpResponse{
+
+  @Override
+  public StatusLine getStatusLine() {
+    return new MockStatusLine();
+  }
+
+  @Override
+  public void setStatusLine(StatusLine statusLine) {
+
+  }
+
+  @Override
+  public void setStatusLine(ProtocolVersion protocolVersion, int i) {
+
+  }
+
+  @Override
+  public void setStatusLine(ProtocolVersion protocolVersion, int i, String s) {
+
+  }
+
+  @Override
+  public void setStatusCode(int i) throws IllegalStateException {
+
+  }
+
+  @Override
+  public void setReasonPhrase(String s) throws IllegalStateException {
+
+  }
+
+  @Override
+  public HttpEntity getEntity() {
+    return new MockEntity();
+  }
+
+  @Override
+  public void setEntity(HttpEntity httpEntity) {
+
+  }
+
+  @Override
+  public Locale getLocale() {
+    return null;
+  }
+
+  @Override
+  public void setLocale(Locale locale) {
+
+  }
+
+  @Override
+  public ProtocolVersion getProtocolVersion() {
+    return null;
+  }
+}
+
+class MockStatusLine implements StatusLine{
+
+  @Override
+  public ProtocolVersion getProtocolVersion() {
+    return null;
+  }
+
+  @Override
+  public int getStatusCode() {
+    return 200;
+  }
+
+  @Override
+  public String getReasonPhrase() {
+    return null;
+  }
+}
+
+class MockEntity implements HttpEntity{
+
+  @Override
+  public boolean isRepeatable() {
+    return false;
+  }
+
+  @Override
+  public boolean isChunked() {
+    return false;
+  }
+
+  @Override
+  public long getContentLength() {
+    return 0;
+  }
+
+  @Override
+  public Header getContentType() {
+    return null;
+  }
+
+  @Override
+  public Header getContentEncoding() {
+    return null;
+  }
+
+  @Override
+  public InputStream getContent() throws IOException, IllegalStateException {
+    return new ByteArrayInputStream(("{\"columnMetas\":" +
+        "[{\"label\":\"PART_DT\"},{\"label\":\"measure\"}]," +
+        "\"results\":[[\"2012-01-03\",\"917.4138\"]," +
+        "[\"2012-05-06\",\"592.4823\"]]}").getBytes());
+  }
+
+  @Override
+  public void writeTo(OutputStream outputStream) throws IOException {
+
+  }
+
+  @Override
+  public boolean isStreaming() {
+    return false;
+  }
+
+  @Override
+  public void consumeContent() throws IOException {
+
+  }
+}

Reply via email to