You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zeppelin.apache.org by fe...@apache.org on 2016/02/17 23:11:33 UTC

incubator-zeppelin git commit: [Zeppelin-628 ] Fix parse propertyKey in interpreter name for JDBC

Repository: incubator-zeppelin
Updated Branches:
  refs/heads/master 4ac7ff5ca -> a283dfa87


[Zeppelin-628 ] Fix parse propertyKey in interpreter name for JDBC

### What is this PR for?
Fix bug
https://issues.apache.org/jira/browse/ZEPPELIN-628

### Todos

### How should this be tested?
run a query that contains (something)...eg
```
%jdbc
select max(ss_promo_sk), ss_customer_sk from qhive.tpcds_orc_500.store_sales where ss_sold_date_sk >= 2452640 and ss_customer_sk > 3 and ss_customer_sk < 20 group by ss_customer_sk
```
It is ok if the **propertyKey** is default:
```
PropertyKey: default, SQL command: 'select max(ss_promo_sk), ss_customer_sk from qhive.tpcds_orc_500.store_sales where ss_sold_date_sk >= 2452640 and ss_customer_sk > 3 and ss_customer_sk < 20 group by ss_customer_sk'
```
### Questions:

Does the licenses files need update? no
Is there breaking changes for older versions? no
Does this needs documentation? no

Author: vgmartinez <vi...@gmail.com>

Closes #667 from vgmartinez/bug_628 and squashes the following commits:

4859cac [vgmartinez] fix test for parse propertyKey
810c14e [vgmartinez] add more tests for parse prefix
9d59c60 [vgmartinez] fixed parse properties


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

Branch: refs/heads/master
Commit: a283dfa87f1b69d85e26c758c9998678f127d47a
Parents: 4ac7ff5
Author: vgmartinez <vi...@gmail.com>
Authored: Tue Feb 16 09:48:14 2016 +0100
Committer: Felix Cheung <fe...@apache.org>
Committed: Wed Feb 17 14:11:23 2016 -0800

----------------------------------------------------------------------
 .../apache/zeppelin/jdbc/JDBCInterpreter.java   | 40 +++++++++++------
 .../zeppelin/jdbc/JDBCInterpreterTest.java      | 47 ++++++++++++++++++++
 2 files changed, 73 insertions(+), 14 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-zeppelin/blob/a283dfa8/jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java
----------------------------------------------------------------------
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 4b9775e..0f74e6e 100644
--- a/jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java
+++ b/jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java
@@ -33,6 +33,7 @@ import java.util.Set;
 
 import org.apache.zeppelin.interpreter.Interpreter;
 import org.apache.zeppelin.interpreter.InterpreterContext;
+import org.apache.zeppelin.interpreter.InterpreterException;
 import org.apache.zeppelin.interpreter.InterpreterPropertyBuilder;
 import org.apache.zeppelin.interpreter.InterpreterResult;
 import org.apache.zeppelin.interpreter.InterpreterResult.Code;
@@ -172,6 +173,9 @@ public class JDBCInterpreter extends Interpreter {
   
   public Connection getConnection(String propertyKey)  throws ClassNotFoundException, SQLException {
     Connection connection = null;
+    if (propertyKey == null || propertiesMap.get(propertyKey) == null) {
+      return null;
+    }
     if (propertyKeyUnusedConnectionListMap.containsKey(propertyKey)) {
       ArrayList<Connection> connectionList = propertyKeyUnusedConnectionListMap.get(propertyKey);
       if (0 != connectionList.size()) {
@@ -206,6 +210,10 @@ public class JDBCInterpreter extends Interpreter {
     } else {
       connection = getConnection(propertyKey);
     }
+    
+    if (connection == null) {
+      return null;
+    }
 
     Statement statement = connection.createStatement();
     if (isStatementClosed(statement)) {
@@ -260,6 +268,10 @@ public class JDBCInterpreter extends Interpreter {
     try {
 
       Statement statement = getStatement(propertyKey, paragraphId);
+      
+      if (statement == null) {
+        return new InterpreterResult(Code.ERROR, "Prefix not found.");
+      }
       statement.setMaxRows(getMaxResult());
 
       StringBuilder msg = null;
@@ -344,12 +356,10 @@ public class JDBCInterpreter extends Interpreter {
     logger.info("Run SQL command '{}'", cmd);
     String propertyKey = getPropertyKey(cmd);
 
-    if (null != propertyKey) {
+    if (null != propertyKey && !propertyKey.equals(DEFAULT_KEY)) {
       cmd = cmd.substring(propertyKey.length() + 2);
-    } else {
-      propertyKey = DEFAULT_KEY;
     }
-
+    
     cmd = cmd.trim();
 
     logger.info("PropertyKey: {}, SQL command: '{}'", propertyKey, cmd);
@@ -371,17 +381,19 @@ public class JDBCInterpreter extends Interpreter {
   }
 
   public String getPropertyKey(String cmd) {
-    int firstLineIndex = cmd.indexOf("\n");
-    if (-1 == firstLineIndex) {
-      firstLineIndex = cmd.length();
-    }
-    int configStartIndex = cmd.indexOf("(");
-    int configLastIndex = cmd.indexOf(")");
-    if (configStartIndex != -1 && configLastIndex != -1
-        && configLastIndex < firstLineIndex && configLastIndex < firstLineIndex) {
-      return cmd.substring(configStartIndex + 1, configLastIndex);
+    boolean firstLineIndex = cmd.startsWith("(");
+
+    if (firstLineIndex) {
+      int configStartIndex = cmd.indexOf("(");
+      int configLastIndex = cmd.indexOf(")");
+      if (configStartIndex != -1 && configLastIndex != -1) {
+        return cmd.substring(configStartIndex + 1, configLastIndex);
+      } else {
+        return null;
+      }
+    } else {
+      return DEFAULT_KEY;
     }
-    return null;
   }
   
   @Override

http://git-wip-us.apache.org/repos/asf/incubator-zeppelin/blob/a283dfa8/jdbc/src/test/java/org/apache/zeppelin/jdbc/JDBCInterpreterTest.java
----------------------------------------------------------------------
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 049b137..302f490 100644
--- a/jdbc/src/test/java/org/apache/zeppelin/jdbc/JDBCInterpreterTest.java
+++ b/jdbc/src/test/java/org/apache/zeppelin/jdbc/JDBCInterpreterTest.java
@@ -69,6 +69,53 @@ public class JDBCInterpreterTest extends BasicJDBCTestCaseAdapter {
     );
   }
 
+
+  @Test
+  public void testForParsePropertyKey() throws IOException {
+    JDBCInterpreter t = new JDBCInterpreter(new Properties());
+    
+    assertEquals(t.getPropertyKey("(fake) select max(cant) from test_table where id >= 2452640"),
+        "fake");
+    
+    assertEquals(t.getPropertyKey("() select max(cant) from test_table where id >= 2452640"),
+        "");
+    
+    assertEquals(t.getPropertyKey(")fake( select max(cant) from test_table where id >= 2452640"),
+        "default");
+        
+    // when you use a %jdbc(prefix1), prefix1 is the propertyKey as form part of the cmd string
+    assertEquals(t.getPropertyKey("(prefix1)\n select max(cant) from test_table where id >= 2452640"),
+        "prefix1");
+    
+    assertEquals(t.getPropertyKey("(prefix2) select max(cant) from test_table where id >= 2452640"),
+            "prefix2");
+    
+    // when you use a %jdbc, prefix is the default
+    assertEquals(t.getPropertyKey("select max(cant) from test_table where id >= 2452640"),
+            "default");
+  }
+  
+  @Test
+  public void testForMapPrefix() throws SQLException, IOException {
+    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", "");
+    JDBCInterpreter t = new JDBCInterpreter(properties);
+    t.open();
+
+    String sqlQuery = "(fake) select * from test_table";
+
+    InterpreterResult interpreterResult = t.interpret(sqlQuery, new InterpreterContext("", "1", "","", null,null,null,null,null,null));
+
+    // if prefix not found return ERROR and Prefix not found.
+    assertEquals(InterpreterResult.Code.ERROR, interpreterResult.code());
+    assertEquals("Prefix not found.", interpreterResult.message());
+  }
+  
   @Test
   public void testDefaultProperties() throws SQLException {
     JDBCInterpreter jdbcInterpreter = new JDBCInterpreter(new Properties());