sqoop-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jar...@apache.org
Subject sqoop git commit: SQOOP-2509: Sqoop2: Findbugs: Fix resource leak problem in GenericJdbcExecutor and related classes
Date Wed, 19 Aug 2015 23:41:43 GMT
Repository: sqoop
Updated Branches:
  refs/heads/sqoop2 93d9a7714 -> 1c24ecbde


SQOOP-2509: Sqoop2: Findbugs: Fix resource leak problem in GenericJdbcExecutor and related
classes

(Colin Ma via Jarek Jarcec Cecho)


Project: http://git-wip-us.apache.org/repos/asf/sqoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/sqoop/commit/1c24ecbd
Tree: http://git-wip-us.apache.org/repos/asf/sqoop/tree/1c24ecbd
Diff: http://git-wip-us.apache.org/repos/asf/sqoop/diff/1c24ecbd

Branch: refs/heads/sqoop2
Commit: 1c24ecbde7a81454dad97b09aec43c79fc96049e
Parents: 93d9a77
Author: Jarek Jarcec Cecho <jarcec@apache.org>
Authored: Wed Aug 19 16:41:10 2015 -0700
Committer: Jarek Jarcec Cecho <jarcec@apache.org>
Committed: Wed Aug 19 16:41:10 2015 -0700

----------------------------------------------------------------------
 .../connector/jdbc/GenericJdbcExecutor.java     | 40 ++++++--------------
 .../connector/jdbc/GenericJdbcExtractor.java    |  7 ++--
 .../jdbc/GenericJdbcFromInitializer.java        | 31 +++++----------
 .../jdbc/GenericJdbcToInitializer.java          | 18 +++------
 .../connector/jdbc/GenericJdbcExecutorTest.java |  6 +--
 .../apache/sqoop/connector/jdbc/TestLoader.java | 28 ++++++++------
 6 files changed, 48 insertions(+), 82 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/sqoop/blob/1c24ecbd/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExecutor.java
----------------------------------------------------------------------
diff --git a/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExecutor.java
b/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExecutor.java
index 3770e07..a5315da 100644
--- a/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExecutor.java
+++ b/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExecutor.java
@@ -124,16 +124,8 @@ public class GenericJdbcExecutor {
     }
   }
 
-  public ResultSet executeQuery(String sql) {
-    try {
-      Statement statement = connection.createStatement(
-          ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY);
-      return statement.executeQuery(sql);
-
-    } catch (SQLException e) {
-      logSQLException(e);
-      throw new SqoopException(GenericJdbcConnectorError.GENERIC_JDBC_CONNECTOR_0002, e);
-    }
+  public Connection getConnection() {
+    return connection;
   }
 
   public PreparedStatement createStatement(String sql) {
@@ -261,29 +253,21 @@ public class GenericJdbcExecutor {
   }
 
   public long getTableRowCount(String tableName) {
-    ResultSet resultSet = executeQuery("SELECT COUNT(1) FROM " + encloseIdentifier(tableName));
-    try {
+    try (Statement statement = connection.createStatement(
+            ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY);
+         ResultSet resultSet = statement.executeQuery("SELECT COUNT(1) FROM " + encloseIdentifier(tableName));)
{
       resultSet.next();
       return resultSet.getLong(1);
     } catch(SQLException e) {
       throw new SqoopException(
         GenericJdbcConnectorError.GENERIC_JDBC_CONNECTOR_0004, e);
-    } finally {
-      try {
-        if(resultSet != null)
-          resultSet.close();
-      } catch(SQLException e) {
-        logSQLException(e, "Got SQLException while closing resultset.");
-      }
     }
   }
 
   public void executeUpdate(String sql) {
-    try {
-      Statement statement = connection.createStatement(
-          ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY);
+    try (Statement statement = connection.createStatement(
+            ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY)) {
       statement.executeUpdate(sql);
-
     } catch (SQLException e) {
       logSQLException(e);
       throw new SqoopException(GenericJdbcConnectorError.GENERIC_JDBC_CONNECTOR_0002, e);
@@ -415,7 +399,7 @@ public class GenericJdbcExecutor {
 
       // Few shortcuts so that we don't have run full loop
       if(primaryKeyColumns.isEmpty()) {
-        return null;
+        return new String[] {};
       } else if(primaryKeyColumns.size() == 1){
         return new String[] {primaryKeyColumns.get(0).getKey()};
       }
@@ -434,11 +418,9 @@ public class GenericJdbcExecutor {
   }
 
   public String[] getQueryColumns(String query) {
-    try {
-      Statement statement = connection.createStatement(
-          ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY);
-      ResultSet rs = statement.executeQuery(query);
-
+    try (Statement statement = connection.createStatement(
+            ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY);
+         ResultSet rs = statement.executeQuery(query);) {
       ResultSetMetaData rsmd = rs.getMetaData();
       int count = rsmd.getColumnCount();
       String[] columns = new String[count];

http://git-wip-us.apache.org/repos/asf/sqoop/blob/1c24ecbd/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExtractor.java
----------------------------------------------------------------------
diff --git a/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExtractor.java
b/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExtractor.java
index 62730fd..8bf43e4 100644
--- a/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExtractor.java
+++ b/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExtractor.java
@@ -20,6 +20,7 @@ package org.apache.sqoop.connector.jdbc;
 import java.sql.ResultSet;
 import java.sql.ResultSetMetaData;
 import java.sql.SQLException;
+import java.sql.Statement;
 
 import org.apache.log4j.Logger;
 import org.apache.sqoop.common.SqoopException;
@@ -50,11 +51,11 @@ public class GenericJdbcExtractor extends Extractor<LinkConfiguration,
FromJobCo
     LOG.info("Using query: " + query);
 
     rowsRead = 0;
-    ResultSet resultSet = executor.executeQuery(query);
-
     Schema schema = context.getSchema();
     Column[] schemaColumns = schema.getColumnsArray();
-    try {
+    try (Statement statement = executor.getConnection().createStatement(
+            ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY);
+         ResultSet resultSet = statement.executeQuery(query);) {
       ResultSetMetaData metaData = resultSet.getMetaData();
       int columnCount = metaData.getColumnCount();
       if (schemaColumns.length != columnCount) {

http://git-wip-us.apache.org/repos/asf/sqoop/blob/1c24ecbd/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcFromInitializer.java
----------------------------------------------------------------------
diff --git a/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcFromInitializer.java
b/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcFromInitializer.java
index 8bf7b6e..909ed74 100644
--- a/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcFromInitializer.java
+++ b/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcFromInitializer.java
@@ -21,6 +21,7 @@ import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.ResultSetMetaData;
 import java.sql.SQLException;
+import java.sql.Statement;
 import java.util.Set;
 
 import org.apache.commons.lang.StringUtils;
@@ -78,13 +79,11 @@ public class GenericJdbcFromInitializer extends Initializer<LinkConfiguration,
F
     }
 
     Schema schema = new Schema(schemaName);
-    ResultSet rs = null;
     ResultSetMetaData rsmt = null;
-    try {
-      rs = executor.executeQuery(
-        context.getString(GenericJdbcConnectorConstants.CONNECTOR_JDBC_FROM_DATA_SQL)
-          .replace(GenericJdbcConnectorConstants.SQL_CONDITIONS_TOKEN, "1 = 0")
-      );
+    try (Statement statement = executor.getConnection().createStatement(
+            ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY);
+         ResultSet rs = statement.executeQuery(context.getString(GenericJdbcConnectorConstants.CONNECTOR_JDBC_FROM_DATA_SQL)
+                 .replace(GenericJdbcConnectorConstants.SQL_CONDITIONS_TOKEN, "1 = 0"));)
{
 
       rsmt = rs.getMetaData();
       for (int i = 1 ; i <= rsmt.getColumnCount(); i++) {
@@ -103,13 +102,6 @@ public class GenericJdbcFromInitializer extends Initializer<LinkConfiguration,
F
     } catch (SQLException e) {
       throw new SqoopException(GenericJdbcConnectorError.GENERIC_JDBC_CONNECTOR_0016, e);
     } finally {
-      if(rs != null) {
-        try {
-          rs.close();
-        } catch (SQLException e) {
-          LOG.info("Ignoring exception while closing ResultSet", e);
-        }
-      }
       if (executor != null) {
         executor.close();
       }
@@ -137,7 +129,7 @@ public class GenericJdbcFromInitializer extends Initializer<LinkConfiguration,
F
     if (StringUtils.isBlank(partitionColumnName) && tableImport) {
       String [] primaryKeyColumns = executor.getPrimaryKey(jobConf.fromJobConfig.schemaName,
jobConf.fromJobConfig.tableName);
       LOG.info("Found primary key columns [" + StringUtils.join(primaryKeyColumns, ", ")
+ "]");
-      if(primaryKeyColumns == null) {
+      if(primaryKeyColumns.length == 0) {
         throw new SqoopException(GenericJdbcConnectorError.GENERIC_JDBC_CONNECTOR_0025, "Please
specify partition column.");
       } else if (primaryKeyColumns.length > 1) {
         LOG.warn("Table have compound primary key, for partitioner we're using only first
column of the key: " + primaryKeyColumns[0]);
@@ -178,10 +170,9 @@ public class GenericJdbcFromInitializer extends Initializer<LinkConfiguration,
F
       String incrementalNewMaxValueQuery = sb.toString();
       LOG.info("Incremental new max value query:  " + incrementalNewMaxValueQuery);
 
-      ResultSet rs = null;
-      try {
-        rs = executor.executeQuery(incrementalNewMaxValueQuery);
-
+      try (Statement statement = executor.getConnection().createStatement(
+              ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY);
+           ResultSet rs = statement.executeQuery(incrementalNewMaxValueQuery);) {
         if (!rs.next()) {
           throw new SqoopException(GenericJdbcConnectorError.GENERIC_JDBC_CONNECTOR_0022);
         }
@@ -189,10 +180,6 @@ public class GenericJdbcFromInitializer extends Initializer<LinkConfiguration,
F
         incrementalMaxValue = rs.getString(1);
         context.setString(GenericJdbcConnectorConstants.CONNECTOR_JDBC_LAST_INCREMENTAL_VALUE,
incrementalMaxValue);
         LOG.info("New maximal value for incremental import is " + incrementalMaxValue);
-      } finally {
-        if(rs != null) {
-          rs.close();
-        }
       }
     }
 

http://git-wip-us.apache.org/repos/asf/sqoop/blob/1c24ecbd/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcToInitializer.java
----------------------------------------------------------------------
diff --git a/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcToInitializer.java
b/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcToInitializer.java
index f97e731..ad375fd 100644
--- a/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcToInitializer.java
+++ b/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcToInitializer.java
@@ -20,6 +20,7 @@ package org.apache.sqoop.connector.jdbc;
 import java.sql.ResultSet;
 import java.sql.ResultSetMetaData;
 import java.sql.SQLException;
+import java.sql.Statement;
 import java.util.Set;
 
 import org.apache.commons.lang.StringUtils;
@@ -70,12 +71,11 @@ public class GenericJdbcToInitializer extends Initializer<LinkConfiguration,
ToJ
     }
 
     Schema schema = new Schema(schemaName);
-    ResultSet rs = null;
-    ResultSetMetaData rsmt = null;
-    try {
-      rs = executor.executeQuery("SELECT * FROM " + schemaName + " WHERE 1 = 0");
+    try (Statement statement = executor.getConnection().createStatement(
+            ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY);
+         ResultSet rs = statement.executeQuery("SELECT * FROM " + schemaName + " WHERE 1
= 0");) {
 
-      rsmt = rs.getMetaData();
+      ResultSetMetaData rsmt = rs.getMetaData();
       for (int i = 1 ; i <= rsmt.getColumnCount(); i++) {
         String columnName = rsmt.getColumnName(i);
 
@@ -93,14 +93,6 @@ public class GenericJdbcToInitializer extends Initializer<LinkConfiguration,
ToJ
       return schema;
     } catch (SQLException e) {
       throw new SqoopException(GenericJdbcConnectorError.GENERIC_JDBC_CONNECTOR_0016, e);
-    } finally {
-      if(rs != null) {
-        try {
-          rs.close();
-        } catch (SQLException e) {
-          LOG.info("Ignoring exception while closing ResultSet", e);
-        }
-      }
     }
   }
 

http://git-wip-us.apache.org/repos/asf/sqoop/blob/1c24ecbd/connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/GenericJdbcExecutorTest.java
----------------------------------------------------------------------
diff --git a/connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/GenericJdbcExecutorTest.java
b/connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/GenericJdbcExecutorTest.java
index 59c12f3..5587840 100644
--- a/connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/GenericJdbcExecutorTest.java
+++ b/connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/GenericJdbcExecutorTest.java
@@ -89,9 +89,9 @@ public class GenericJdbcExecutorTest {
 
   @Test
   public void testGetPrimaryKey() {
-    assertNull(executor.getPrimaryKey("non-existing-table"));
-    assertNull(executor.getPrimaryKey("non-existing-schema", "non-existing-table"));
-    assertNull(executor.getPrimaryKey("non-existing-catalog", "non-existing-schema", "non-existing-table"));
+    assertEquals(executor.getPrimaryKey("non-existing-table"), new String[] {});
+    assertEquals(executor.getPrimaryKey("non-existing-schema", "non-existing-table"), new
String[] {});
+    assertEquals(executor.getPrimaryKey("non-existing-catalog", "non-existing-schema", "non-existing-table"),
new String[] {});
 
     assertEquals(executor.getPrimaryKey(schema, table), new String[] {"ICOL"});
     assertEquals(executor.getPrimaryKey(compoundPrimaryKeyTable), new String[] {"VCOL", "ICOL"});

http://git-wip-us.apache.org/repos/asf/sqoop/blob/1c24ecbd/connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestLoader.java
----------------------------------------------------------------------
diff --git a/connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestLoader.java
b/connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestLoader.java
index c69ec03..65e2a4b 100644
--- a/connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestLoader.java
+++ b/connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestLoader.java
@@ -18,6 +18,7 @@
 package org.apache.sqoop.connector.jdbc;
 
 import java.sql.ResultSet;
+import java.sql.Statement;
 
 import org.apache.sqoop.common.MutableContext;
 import org.apache.sqoop.common.MutableMapContext;
@@ -110,19 +111,22 @@ public class TestLoader {
     loader.load(loaderContext, linkConfig, jobConfig);
 
     int index = START;
-    ResultSet rs = executor.executeQuery("SELECT * FROM "
-        + executor.encloseIdentifier(tableName) + " ORDER BY ICOL");
-    while (rs.next()) {
-      assertEquals(index, rs.getObject(1));
-      assertEquals((double) index, rs.getObject(2));
-      assertEquals(String.valueOf(index), rs.getObject(3));
-      assertEquals("2004-10-19", rs.getObject(4).toString());
-      assertEquals("2004-10-19 10:23:34.0", rs.getObject(5).toString());
-      assertEquals("11:33:59", rs.getObject(6).toString());
-
-      index++;
+    try (Statement statement = executor.getConnection().createStatement(
+            ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY);
+         ResultSet rs = statement.executeQuery("SELECT * FROM "
+                 + executor.encloseIdentifier(tableName) + " ORDER BY ICOL");) {
+      while (rs.next()) {
+        assertEquals(index, rs.getObject(1));
+        assertEquals((double) index, rs.getObject(2));
+        assertEquals(String.valueOf(index), rs.getObject(3));
+        assertEquals("2004-10-19", rs.getObject(4).toString());
+        assertEquals("2004-10-19 10:23:34.0", rs.getObject(5).toString());
+        assertEquals("11:33:59", rs.getObject(6).toString());
+
+        index++;
+      }
+      assertEquals(numberOfRows, index - START);
     }
-    assertEquals(numberOfRows, index-START);
   }
 
   public class DummyReader extends DataReader {


Mime
View raw message