You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sqoop.apache.org by ja...@apache.org on 2015/08/20 01:41:43 UTC
sqoop git commit: SQOOP-2509: Sqoop2: Findbugs: Fix resource leak
problem in GenericJdbcExecutor and related classes
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 <ja...@apache.org>
Authored: Wed Aug 19 16:41:10 2015 -0700
Committer: Jarek Jarcec Cecho <ja...@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 {