[ZEPPELIN-2216] general solution to precode. refactoring jdbc precode

docs/interpreter/jdbc.md

@@ -121,7 +121,7 @@ The JDBC interpreter properties are defined by default like below.
   <tr>
     <td>default.precode</td>
     <td></td>
-    <td>Some SQL which executes while opening connection</td>
+    <td>Some SQL which executes every time after initialization of the interpreter (see [Binding mode](../manual/interpreters.md#interpreter-binding-mode))</td>
   </tr>
   <tr>
     <td>default.completer.schemaFilters</td>

docs/manual/interpreters.md

@@ -113,3 +113,11 @@ interpreter.start()
 The above code will start interpreter thread inside your process. Once the interpreter is started you can configure zeppelin to connect to RemoteInterpreter by checking **Connect to existing process** checkbox and then provide **Host** and **Port** on which interpreter process is listening as shown in the image below:
 
 <img src="../assets/themes/zeppelin/img/screenshots/existing_interpreter.png" width="450px">
+
+## Precode
+
+Snippet of code (language of interpreter) that executes after initialization of the interpreter depends on [Binding mode](#interpreter-binding-mode). To configure add parameter with class of interpreter (`zeppelin.<ClassName>.precode`) except JDBCInterpreter ([JDBC precode](../interpreter/jdbc.md#usage-precode)). 
+
+<img src="../assets/themes/zeppelin/img/screenshots/interpreter_precode.png" width="800px">
+
+

jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java

@@ -332,9 +332,6 @@ private Connection getConnectionFromPool(String url, String user, String propert
 
     if (!getJDBCConfiguration(user).isConnectionInDBDriverPool(propertyKey)) {
       createConnectionPool(url, user, propertyKey, properties);
-      try (Connection connection = DriverManager.getConnection(jdbcDriver)) {
-        executePrecode(connection, propertyKey);
-      }
     }
     return DriverManager.getConnection(jdbcDriver);
   }
@@ -572,18 +569,19 @@ protected ArrayList<String> splitSqlQueries(String sql) {
     return queries;
   }
 
-  private void executePrecode(Connection connection, String propertyKey) throws SQLException {
-    String precode = getProperty(String.format(PRECODE_KEY_TEMPLATE, propertyKey));
-    if (StringUtils.isNotBlank(precode)) {
-      precode = StringUtils.trim(precode);
-      logger.debug("Run SQL precode '{}'", precode);
-      try (Statement statement = connection.createStatement()) {
-        statement.execute(precode);
-        if (!connection.getAutoCommit()) {
-          connection.commit();
+  public InterpreterResult executePrecode(InterpreterContext interpreterContext) {
+    InterpreterResult interpreterResult = null;
+    for (String propertyKey : basePropretiesMap.keySet()) {
+      String precode = getProperty(String.format("%s.precode", propertyKey));
+      if (StringUtils.isNotBlank(precode)) {
+        interpreterResult = executeSql(propertyKey, precode, interpreterContext);
+        if (interpreterResult.code() != Code.SUCCESS) {
+          break;
         }
       }
     }
+
+    return interpreterResult;
   }
 
   private InterpreterResult executeSql(String propertyKey, String sql,

jdbc/src/test/java/org/apache/zeppelin/jdbc/JDBCInterpreterTest.java

@@ -400,17 +400,18 @@ public void testPrecode() throws SQLException, IOException {
     properties.setProperty("default.url", getJdbcConnection());
     properties.setProperty("default.user", "");
     properties.setProperty("default.password", "");
-    properties.setProperty(DEFAULT_PRECODE, "SET @testVariable=1");
+    properties.setProperty(DEFAULT_PRECODE, "create table test_precode (id int); insert into test_precode values (1);");
     JDBCInterpreter jdbcInterpreter = new JDBCInterpreter(properties);
     jdbcInterpreter.open();
+    jdbcInterpreter.executePrecode(interpreterContext);
 
-    String sqlQuery = "select @testVariable";
+    String sqlQuery = "select *from test_precode";
 
     InterpreterResult interpreterResult = jdbcInterpreter.interpret(sqlQuery, interpreterContext);
 
     assertEquals(InterpreterResult.Code.SUCCESS, interpreterResult.code());
     assertEquals(InterpreterResult.Type.TABLE, interpreterResult.message().get(0).getType());
-    assertEquals("@TESTVARIABLE\n1\n", interpreterResult.message().get(0).getData());
+    assertEquals("ID\n1\n", interpreterResult.message().get(0).getData());
   }
 
   @Test
@@ -420,13 +421,15 @@ public void testIncorrectPrecode() throws SQLException, IOException {
     properties.setProperty("default.url", getJdbcConnection());
     properties.setProperty("default.user", "");
     properties.setProperty("default.password", "");
-    properties.setProperty(DEFAULT_PRECODE, "incorrect command");
+    properties.setProperty(DEFAULT_PRECODE, "select 1");
+    properties.setProperty("incorrect.driver", "org.h2.Driver");
+    properties.setProperty("incorrect.url", getJdbcConnection());
+    properties.setProperty("incorrect.user", "");
+    properties.setProperty("incorrect.password", "");
+    properties.setProperty(String.format(PRECODE_KEY_TEMPLATE, "incorrect"), "incorrect command");
     JDBCInterpreter jdbcInterpreter = new JDBCInterpreter(properties);
     jdbcInterpreter.open();
-
-    String sqlQuery = "select 1";
-
-    InterpreterResult interpreterResult = jdbcInterpreter.interpret(sqlQuery, interpreterContext);
+    InterpreterResult interpreterResult = jdbcInterpreter.executePrecode(interpreterContext);
 
     assertEquals(InterpreterResult.Code.ERROR, interpreterResult.code());
     assertEquals(InterpreterResult.Type.TEXT, interpreterResult.message().get(0).getType());
@@ -439,17 +442,18 @@ public void testPrecodeWithAnotherPrefix() throws SQLException, IOException {
     properties.setProperty("anotherPrefix.url", getJdbcConnection());
     properties.setProperty("anotherPrefix.user", "");
     properties.setProperty("anotherPrefix.password", "");
-    properties.setProperty(String.format(PRECODE_KEY_TEMPLATE, "anotherPrefix"), "SET @testVariable=2");
+    properties.setProperty(String.format(PRECODE_KEY_TEMPLATE, "anotherPrefix"), "create table test_precode_2 (id int); insert into test_precode_2 values (2);");
     JDBCInterpreter jdbcInterpreter = new JDBCInterpreter(properties);
     jdbcInterpreter.open();
+    jdbcInterpreter.executePrecode(interpreterContext);
 
-    String sqlQuery = "(anotherPrefix) select @testVariable";
+    String sqlQuery = "(anotherPrefix) select *from test_precode_2";
 
     InterpreterResult interpreterResult = jdbcInterpreter.interpret(sqlQuery, interpreterContext);
 
     assertEquals(InterpreterResult.Code.SUCCESS, interpreterResult.code());
     assertEquals(InterpreterResult.Type.TABLE, interpreterResult.message().get(0).getType());
-    assertEquals("@TESTVARIABLE\n2\n", interpreterResult.message().get(0).getData());
+    assertEquals("ID\n2\n", interpreterResult.message().get(0).getData());
   }
 
   @Test

zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/Interpreter.java

@@ -64,6 +64,19 @@ public abstract class Interpreter {
   @ZeppelinApi
   public abstract void close();
 
+  /**
+   * Run precode if exists.
+   */
+  @ZeppelinApi
+  public InterpreterResult executePrecode(InterpreterContext interpreterContext) {
+    String simpleName = this.getClass().getSimpleName();
+    String precode = getProperty(String.format("zeppelin.%s.precode", simpleName));
+    if (StringUtils.isNotBlank(precode)) {
+      return interpret(precode, interpreterContext);
+    }
+    return null;
+  }
+
   /**
    * Run code and return result, in synchronous way.
    *

zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/LazyOpenInterpreter.java

@@ -21,7 +21,6 @@
 import java.util.List;
 import java.util.Properties;
 
-import org.apache.zeppelin.interpreter.remote.RemoteInterpreter;
 import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion;
 import org.apache.zeppelin.scheduler.Scheduler;
 
@@ -73,6 +72,11 @@ public synchronized void open() {
     }
   }
 
+  @Override
+  public InterpreterResult executePrecode(InterpreterContext interpreterContext) {
+    return intp.executePrecode(interpreterContext);
+  }
+
   @Override
   public void close() {
     synchronized (intp) {
@@ -157,7 +161,7 @@ public void setInterpreterGroup(InterpreterGroup interpreterGroup) {
   public void setClassloaderUrls(URL [] urls) {
     intp.setClassloaderUrls(urls);
   }
-  
+
   @Override
   public void registerHook(String noteId, String event, String cmd) {
     intp.registerHook(noteId, event, cmd);

zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterServer.java

@@ -481,19 +481,24 @@ protected Object jobRun() throws Throwable {
       try {
         InterpreterContext.set(context);
 
+        InterpreterResult result = null;
+
         // Open the interpreter instance prior to calling interpret().
         // This is necessary because the earliest we can register a hook
         // is from within the open() method.
         LazyOpenInterpreter lazy = (LazyOpenInterpreter) interpreter;
         if (!lazy.isOpen()) {
           lazy.open();
+          result = lazy.executePrecode(context);
         }
 
-        // Add hooks to script from registry.
-        // Global scope first, followed by notebook scope
-        processInterpreterHooks(null);
-        processInterpreterHooks(context.getNoteId());
-        InterpreterResult result = interpreter.interpret(script, context);
+        if (result == null || result.code() == Code.SUCCESS) {
+          // Add hooks to script from registry.
+          // Global scope first, followed by notebook scope
+          processInterpreterHooks(null);
+          processInterpreterHooks(context.getNoteId());
+          result = interpreter.interpret(script, context);
+        }
 
         // data from context.out is prepended to InterpreterResult if both defined
         context.out.flush();

zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/remote/RemoteInterpreterTest.java

@@ -170,6 +170,79 @@ public void testRemoteInterperterCall() throws TTransportException, IOException
 
   }
 
+  @Test
+  public void testExecuteIncorrectPrecode() throws TTransportException, IOException {
+    Properties p = new Properties();
+    p.put("zeppelin.MockInterpreterA.precode", "fail test");
+    intpGroup.put("note", new LinkedList<Interpreter>());
+
+    RemoteInterpreter intpA = createMockInterpreterA(p);
+
+    intpGroup.get("note").add(intpA);
+
+    intpA.setInterpreterGroup(intpGroup);
+
+    RemoteInterpreterProcess process = intpA.getInterpreterProcess();
+
+    intpA.open();
+
+    InterpreterResult result = intpA.interpret("1",
+        new InterpreterContext(
+            "note",
+            "id",
+            null,
+            "title",
+            "text",
+            new AuthenticationInfo(),
+            new HashMap<String, Object>(),
+            new GUI(),
+            new AngularObjectRegistry(intpGroup.getId(), null),
+            new LocalResourcePool("pool1"),
+            new LinkedList<InterpreterContextRunner>(), null));
+
+
+
+    intpA.close();
+    assertEquals(Code.ERROR, result.code());
+  }
+
+  @Test
+  public void testExecuteCorrectPrecode() throws TTransportException, IOException {
+    Properties p = new Properties();
+    p.put("zeppelin.MockInterpreterA.precode", "2");
+    intpGroup.put("note", new LinkedList<Interpreter>());
+
+    RemoteInterpreter intpA = createMockInterpreterA(p);
+
+    intpGroup.get("note").add(intpA);
+
+    intpA.setInterpreterGroup(intpGroup);
+
+    RemoteInterpreterProcess process = intpA.getInterpreterProcess();
+
+    intpA.open();
+
+    InterpreterResult result = intpA.interpret("1",
+        new InterpreterContext(
+            "note",
+            "id",
+            null,
+            "title",
+            "text",
+            new AuthenticationInfo(),
+            new HashMap<String, Object>(),
+            new GUI(),
+            new AngularObjectRegistry(intpGroup.getId(), null),
+            new LocalResourcePool("pool1"),
+            new LinkedList<InterpreterContextRunner>(), null));
+
+
+
+    intpA.close();
+    assertEquals(Code.SUCCESS, result.code());
+    assertEquals("1", result.message().get(0).getData());
+  }
+
   @Test
   public void testRemoteInterperterErrorStatus() throws TTransportException, IOException {
     Properties p = new Properties();