You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lens.apache.org by am...@apache.org on 2015/02/24 09:06:04 UTC

incubator-lens git commit: LENS-332 : Do syntax and semantic validation through prepare statement (Sushil Mohanty via amareshwari)

Repository: incubator-lens
Updated Branches:
  refs/heads/master ce49afe63 -> b5ba6bd5a


LENS-332 : Do syntax and semantic validation through prepare statement (Sushil Mohanty via amareshwari)


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

Branch: refs/heads/master
Commit: b5ba6bd5a6eb428385ee7984ea22d94d2f9f06af
Parents: ce49afe
Author: Amareshwari Sriramadasu <am...@apache.org>
Authored: Tue Feb 24 13:35:44 2015 +0530
Committer: Amareshwari Sriramadasu <am...@apache.org>
Committed: Tue Feb 24 13:35:44 2015 +0530

----------------------------------------------------------------------
 .../org/apache/lens/driver/jdbc/JDBCDriver.java | 99 ++++++++++++++++----
 .../driver/jdbc/JDBCDriverConfConstants.java    |  6 ++
 .../src/main/resources/jdbcdriver-default.xml   |  6 ++
 .../apache/lens/driver/jdbc/TestJdbcDriver.java | 21 +++++
 src/site/apt/admin/jdbcdriver-config.apt        |  2 +
 5 files changed, 116 insertions(+), 18 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-lens/blob/b5ba6bd5/lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/JDBCDriver.java
----------------------------------------------------------------------
diff --git a/lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/JDBCDriver.java b/lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/JDBCDriver.java
index 15ca19a..83b956e 100644
--- a/lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/JDBCDriver.java
+++ b/lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/JDBCDriver.java
@@ -27,6 +27,8 @@ import java.io.ObjectInput;
 import java.io.ObjectOutput;
 import java.sql.*;
 import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.Map;
 import java.util.concurrent.*;
 import java.util.concurrent.atomic.AtomicInteger;
 
@@ -543,7 +545,8 @@ public class JDBCDriver implements LensDriver {
 
   @Override
   public QueryCost estimate(AbstractQueryContext qctx) throws LensException {
-    return explain(qctx).getCost();
+    validate(qctx);
+    return JDBC_DRIVER_COST;
   }
 
   /**
@@ -599,41 +602,84 @@ public class JDBCDriver implements LensDriver {
   }
 
   /**
-   * Prepare the given query.
+   * Validate query using prepare
    *
-   * @param pContext the context
-   * @throws LensException the lens exception
+   * @param pContext
+   * @throws LensException
    */
-  @Override
-  public void prepare(PreparedQueryContext pContext) throws LensException {
+  public void validate(AbstractQueryContext pContext) throws LensException {
+    if (pContext.getDriverQuery(this) == null) {
+      throw new NullPointerException("Null driver query for " + pContext.getUserQuery());
+    }
+    boolean validateThroughPrepare = pContext.getDriverConf(this).getBoolean(JDBC_VALIDATE_THROUGH_PREPARE,
+        DEFAULT_JDBC_VALIDATE_THROUGH_PREPARE);
+    if (validateThroughPrepare) {
+      PreparedStatement stmt = prepareInternal(pContext);
+      if (stmt != null) {
+        try {
+          stmt.close();
+        } catch (SQLException e) {
+          throw new LensException();
+        }
+      }
+    }
+  }
+
+  private final Map<QueryPrepareHandle, PreparedStatement> preparedQueries =
+      new HashMap<QueryPrepareHandle, PreparedStatement>();
+
+  /**
+   * Internally prepare the query
+   *
+   * @param pContext
+   * @return
+   * @throws LensException
+   */
+  private PreparedStatement prepareInternal(AbstractQueryContext pContext) throws LensException {
+    if (pContext.getDriverQuery(this) == null) {
+      throw new NullPointerException("Null driver query for " + pContext.getUserQuery());
+    }
     checkConfigured();
     // Only create a prepared statement and then close it
-    String rewrittenQuery = rewriteQuery(pContext.getSelectedDriverQuery(), pContext.getSelectedDriverConf());
+    String rewrittenQuery = rewriteQuery(pContext.getDriverQuery(this), pContext.getDriverConf(this));
     Connection conn = null;
     PreparedStatement stmt = null;
     try {
       conn = getConnection();
       stmt = conn.prepareStatement(rewrittenQuery);
+      if (stmt.getWarnings() != null) {
+        throw new LensException(stmt.getWarnings());
+      }
     } catch (SQLException sql) {
       throw new LensException(sql);
     } finally {
-      if (stmt != null) {
-        try {
-          stmt.close();
-        } catch (SQLException e) {
-          LOG.error("Error closing statement: " + pContext.getPrepareHandle(), e);
-        }
-      }
-
       if (conn != null) {
         try {
           conn.close();
         } catch (SQLException e) {
-          LOG.error("Error closing connection: " + pContext.getPrepareHandle(), e);
+          LOG.error("Error closing connection: " + rewrittenQuery, e);
         }
       }
     }
-    LOG.info("Prepared: " + pContext.getPrepareHandle());
+    LOG.info("Prepared: " + rewrittenQuery);
+    return stmt;
+  }
+
+
+  /**
+   * Prepare the given query.
+   *
+   * @param pContext
+   *          the context
+   * @throws LensException
+   *           the lens exception
+   */
+  @Override
+  public void prepare(PreparedQueryContext pContext) throws LensException {
+    PreparedStatement stmt = prepareInternal(pContext);
+    if (stmt != null) {
+      preparedQueries.put(pContext.getPrepareHandle(), stmt);
+    }
   }
 
   /**
@@ -659,7 +705,13 @@ public class JDBCDriver implements LensDriver {
   @Override
   public void closePreparedQuery(QueryPrepareHandle handle) throws LensException {
     checkConfigured();
-    // Do nothing
+    try {
+      if (preparedQueries.get(handle) != null) {
+        preparedQueries.get(handle).close();
+      }
+    } catch (SQLException e) {
+      throw new LensException(e);
+    }
   }
 
   /**
@@ -875,6 +927,17 @@ public class JDBCDriver implements LensDriver {
           LOG.warn("Error closing query : " + query.getHandleId(), e);
         }
       }
+      for (QueryPrepareHandle query : new ArrayList<QueryPrepareHandle>(preparedQueries.keySet())) {
+        try {
+          try {
+            preparedQueries.get(query).close();
+          } catch (SQLException e) {
+            throw new LensException();
+          }
+        } catch (LensException e) {
+          LOG.warn("Error closing prapared query : " + query , e);
+        }
+      }
     } finally {
       queryContextMap.clear();
     }

http://git-wip-us.apache.org/repos/asf/incubator-lens/blob/b5ba6bd5/lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/JDBCDriverConfConstants.java
----------------------------------------------------------------------
diff --git a/lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/JDBCDriverConfConstants.java b/lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/JDBCDriverConfConstants.java
index 1c04b65..5abd040 100644
--- a/lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/JDBCDriverConfConstants.java
+++ b/lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/JDBCDriverConfConstants.java
@@ -82,6 +82,12 @@ public final class JDBCDriverConfConstants {
   /** The Constant DEFAULT_JDBC_EXPLAIN_KEYWORD_BEFORE_SELECT. */
   public static final boolean DEFAULT_JDBC_EXPLAIN_KEYWORD_BEFORE_SELECT = true;
 
+  /** The Constant JDBC_VALIDATE_THROUGH_PREPARE. */
+  public static final String JDBC_VALIDATE_THROUGH_PREPARE = JDBC_DRIVER_PFX + "validate.through.prepare";
+
+  /** The Constant DEFAULT_JDBC_VALIDATE_THROUGH_PREPARE. */
+  public static final boolean DEFAULT_JDBC_VALIDATE_THROUGH_PREPARE = true;
+
   public static final String JDBC_ENABLE_RESULTSET_STREAMING_RETRIEVAL =
     JDBC_DRIVER_PFX + "enable.resultset.streaming.retrieval";
   public static final boolean DEFAULT_JDBC_ENABLE_RESULTSET_STREAMING_RETRIEVAL = false;

http://git-wip-us.apache.org/repos/asf/incubator-lens/blob/b5ba6bd5/lens-driver-jdbc/src/main/resources/jdbcdriver-default.xml
----------------------------------------------------------------------
diff --git a/lens-driver-jdbc/src/main/resources/jdbcdriver-default.xml b/lens-driver-jdbc/src/main/resources/jdbcdriver-default.xml
index c715a0e..03c5414 100644
--- a/lens-driver-jdbc/src/main/resources/jdbcdriver-default.xml
+++ b/lens-driver-jdbc/src/main/resources/jdbcdriver-default.xml
@@ -144,4 +144,10 @@
     <description>Fetch size for JDBC result set</description>
   </property>
 
+  <property>
+    <name>lens.driver.jdbc.validate.through.prepare</name>
+    <value>true</value>
+    <description>Flag to enable query syntactic and semantic validation using prepared statement.</description>
+  </property>
+
 </configuration>

http://git-wip-us.apache.org/repos/asf/incubator-lens/blob/b5ba6bd5/lens-driver-jdbc/src/test/java/org/apache/lens/driver/jdbc/TestJdbcDriver.java
----------------------------------------------------------------------
diff --git a/lens-driver-jdbc/src/test/java/org/apache/lens/driver/jdbc/TestJdbcDriver.java b/lens-driver-jdbc/src/test/java/org/apache/lens/driver/jdbc/TestJdbcDriver.java
index a5843ba..68bd19b 100644
--- a/lens-driver-jdbc/src/test/java/org/apache/lens/driver/jdbc/TestJdbcDriver.java
+++ b/lens-driver-jdbc/src/test/java/org/apache/lens/driver/jdbc/TestJdbcDriver.java
@@ -274,6 +274,7 @@ public class TestJdbcDriver {
     }
   }
 
+
   /**
    * Test execute.
    *
@@ -324,7 +325,27 @@ public class TestJdbcDriver {
 
     final String query = "SELECT * from prepare_test";
     PreparedQueryContext pContext = new PreparedQueryContext(query, "SA", baseConf, drivers);
+    //run prepare
     driver.prepare(pContext);
+    //run validate
+    driver.validate(pContext);
+  }
+
+  /**
+   * Test prepare failing
+   *
+   * @throws Exception the exception
+   */
+  @Test
+  public void testPrepareFailing() throws Exception {
+    String query = "SELECT * FROM prepare_test2"; // Select query against non existing table
+    try {
+      PreparedQueryContext pContext = new PreparedQueryContext(query, "SA", baseConf, drivers);
+      driver.prepare(pContext);
+      Assert.fail("Running prepare on a non existing table.");
+    } catch (LensException ex) {
+      Assert.assertEquals(LensUtil.getCauseMessage(ex), "user lacks privilege or object not found: PREPARE_TEST2");
+    }
   }
 
   /**

http://git-wip-us.apache.org/repos/asf/incubator-lens/blob/b5ba6bd5/src/site/apt/admin/jdbcdriver-config.apt
----------------------------------------------------------------------
diff --git a/src/site/apt/admin/jdbcdriver-config.apt b/src/site/apt/admin/jdbcdriver-config.apt
index 126567a..dc325f1 100644
--- a/src/site/apt/admin/jdbcdriver-config.apt
+++ b/src/site/apt/admin/jdbcdriver-config.apt
@@ -60,4 +60,6 @@ Jdbc driver configuration
 *--+--+---+--+
 |18|lens.driver.jdbc.query.rewriter|org.apache.lens.driver.jdbc.ColumnarSQLRewriter|Rewriting the HQL to optimized sql queries|
 *--+--+---+--+
+|19|lens.driver.jdbc.validate.through.prepare|true|Flag to enable query syntactic and semantic validation using prepared statement.|
+*--+--+---+--+
 The configuration parameters and their default values