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 <[email protected]> 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 <[email protected]> Authored: Tue Feb 16 09:48:14 2016 +0100 Committer: Felix Cheung <[email protected]> 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());
