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 <[email protected]> Authored: Wed Aug 19 16:41:10 2015 -0700 Committer: Jarek Jarcec Cecho <[email protected]> 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 {
