From 7554cf67d9d31a534cc6a3161880d8e86159774e Mon Sep 17 00:00:00 2001 From: Jesse Minchey <131408591+jdm43@users.noreply.github.com> Date: Mon, 1 May 2023 19:46:31 -0600 Subject: [PATCH] PR feedback #3 PR feedback #3 Adding debug logging --- .../morf/jdbc/ResultSetIterator.java | 5 +++ .../morf/jdbc/SqlScriptExecutor.java | 11 +++-- .../morf/jdbc/TestResultSetIterator.java | 41 ++----------------- 3 files changed, 15 insertions(+), 42 deletions(-) diff --git a/morf-core/src/main/java/org/alfasoftware/morf/jdbc/ResultSetIterator.java b/morf-core/src/main/java/org/alfasoftware/morf/jdbc/ResultSetIterator.java index 7d8e663e8..7d0a50b7f 100644 --- a/morf-core/src/main/java/org/alfasoftware/morf/jdbc/ResultSetIterator.java +++ b/morf-core/src/main/java/org/alfasoftware/morf/jdbc/ResultSetIterator.java @@ -21,6 +21,8 @@ import org.alfasoftware.morf.sql.element.Direction; import org.alfasoftware.morf.sql.element.FieldReference; import org.alfasoftware.morf.sql.element.TableReference; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; /** * Provides data set iterator functionality based on a jdbc result set. @@ -28,6 +30,8 @@ * @author Copyright (c) Alfa Financial Software 2017 */ class ResultSetIterator implements Iterator, AutoCloseable { + /** Standard logger */ + private static final Log log = LogFactory.getLog(ResultSetIterator.class); /** * The underlying result set to iterate over. @@ -103,6 +107,7 @@ public ResultSetIterator(Table table, String query, Connection connection, Optio this.statement.setFetchDirection(ResultSet.FETCH_FORWARD); this.statement.setFetchSize((connectionResources.isPresent() && connectionResources.get().getFetchSizeForBulkSelects() != null) ? connectionResources.get().getFetchSizeForBulkSelects() : sqlDialect.fetchSizeForBulkSelects()); + log.debug("Executing query for table [" + table.getName() + "] with fetch size [" + statement.getFetchSize() + "]. Stack trace: [" + Thread.currentThread().getStackTrace().toString()); this.resultSet = statement.executeQuery(query); this.sortedMetadata = ResultSetMetadataSorter.sortedCopy(table.columns(), resultSet); advanceResultSet(); diff --git a/morf-core/src/main/java/org/alfasoftware/morf/jdbc/SqlScriptExecutor.java b/morf-core/src/main/java/org/alfasoftware/morf/jdbc/SqlScriptExecutor.java index 0c677b295..5a3c7ccc1 100755 --- a/morf-core/src/main/java/org/alfasoftware/morf/jdbc/SqlScriptExecutor.java +++ b/morf-core/src/main/java/org/alfasoftware/morf/jdbc/SqlScriptExecutor.java @@ -56,7 +56,7 @@ public class SqlScriptExecutor { private final SqlDialect sqlDialect; private int fetchSizeForBulkSelects; - private int fetchSizeForBulkSelectsAllowingConnectionUseDuringStreamingAsString; + private int fetchSizeForBulkSelectsAllowingConnectionUseDuringStreaming; /** * Create an SQL executor with the given visitor, who will @@ -70,7 +70,7 @@ public class SqlScriptExecutor { this.dataSource = dataSource; this.sqlDialect = sqlDialect; this.fetchSizeForBulkSelects = sqlDialect.fetchSizeForBulkSelects(); - this.fetchSizeForBulkSelectsAllowingConnectionUseDuringStreamingAsString = sqlDialect.fetchSizeForBulkSelectsAllowingConnectionUseDuringStreaming(); + this.fetchSizeForBulkSelectsAllowingConnectionUseDuringStreaming = sqlDialect.fetchSizeForBulkSelectsAllowingConnectionUseDuringStreaming(); if (visitor == null) { this.visitor = new NullVisitor(); } else { @@ -92,7 +92,7 @@ public class SqlScriptExecutor { this.sqlDialect = sqlDialect; this.fetchSizeForBulkSelects = connectionResources.getFetchSizeForBulkSelects() != null ? connectionResources.getFetchSizeForBulkSelects() : sqlDialect.fetchSizeForBulkSelects(); - this.fetchSizeForBulkSelectsAllowingConnectionUseDuringStreamingAsString = connectionResources.getFetchSizeForBulkSelectsAllowingConnectionUseDuringStreaming() != null + this.fetchSizeForBulkSelectsAllowingConnectionUseDuringStreaming = connectionResources.getFetchSizeForBulkSelectsAllowingConnectionUseDuringStreaming() != null ? connectionResources.getFetchSizeForBulkSelectsAllowingConnectionUseDuringStreaming() : sqlDialect.fetchSizeForBulkSelectsAllowingConnectionUseDuringStreaming(); if (visitor == null) { this.visitor = new NullVisitor(); @@ -548,10 +548,13 @@ private T executeQuery(String sql, Iterable parameterMetadata, try { ParseResult parseResult = NamedParameterPreparedStatement.parseSql(sql, sqlDialect); try (NamedParameterPreparedStatement preparedStatement = standalone ? parseResult.createForQueryOn(connection) : parseResult.createFor(connection)) { + if (standalone) { preparedStatement.setFetchSize(fetchSizeForBulkSelects); + log.debug("Executing query [" + sql + "] with standalone = [" + standalone + "] and fetch size: [" + fetchSizeForBulkSelects +"]. Stack trace: [" + Thread.currentThread().getStackTrace().toString()); } else { - preparedStatement.setFetchSize(fetchSizeForBulkSelectsAllowingConnectionUseDuringStreamingAsString); + preparedStatement.setFetchSize(fetchSizeForBulkSelectsAllowingConnectionUseDuringStreaming); + log.debug("Executing query [" + sql + "] with standalone = [" + standalone + "] and fetch size: [" + fetchSizeForBulkSelectsAllowingConnectionUseDuringStreaming +"]. Stack trace: [" + Thread.currentThread().getStackTrace().toString()); } return executeQuery(preparedStatement, parameterMetadata, parameterData, resultSetProcessor, maxRows, queryTimeout); } diff --git a/morf-core/src/test/java/org/alfasoftware/morf/jdbc/TestResultSetIterator.java b/morf-core/src/test/java/org/alfasoftware/morf/jdbc/TestResultSetIterator.java index f99d6ea48..4f371a705 100644 --- a/morf-core/src/test/java/org/alfasoftware/morf/jdbc/TestResultSetIterator.java +++ b/morf-core/src/test/java/org/alfasoftware/morf/jdbc/TestResultSetIterator.java @@ -226,34 +226,17 @@ public void testQueryWithResultSetAndFetchSize() throws Exception { ResultSet resultSet = mock(ResultSet.class); given(statement.executeQuery(query)).willReturn(resultSet); given(resultSet.findColumn("Column")).willReturn(1); - given(resultSet.next()).willReturn(true).willReturn(true).willReturn(false); given(connectionResources.getFetchSizeForBulkSelects()).willReturn(1000); // When @SuppressWarnings("resource") /* Resources are closed in ResultSetIterator.close() automatically when caller attempts to advance past the last row in the result set. */ - ResultSetIterator resultSetIterator = new ResultSetIterator(table, query, connection, Optional.of(connectionResources), sqlDialect); + ResultSetIterator resultSetIterator = new ResultSetIterator(table, query, connection, Optional.of(connectionResources), sqlDialect); // Then - assertTrue(resultSetIterator.hasNext()); - - resultSetIterator.next(); - resultSetIterator.next(); - assertFalse(resultSetIterator.hasNext()); - - verify(resultSet).close(); - verify(statement).close(); verify(statement).setFetchSize(1000); - - boolean gotException = false; - try { - resultSetIterator.next(); - } catch (NoSuchElementException e) { - gotException = true; - } - assertTrue(gotException); } @@ -270,7 +253,6 @@ public void testBuildWithEmptyColumnOrderingAndFetchSize() throws Exception { given(resultSet.findColumn("Column")).willReturn(1); given(sqlDialect.convertStatementToSQL(any(SelectStatement.class))).willReturn(query); given(statement.executeQuery(query)).willReturn(resultSet); - given(resultSet.next()).willReturn(true).willReturn(true).willReturn(false); given(connectionResources.getFetchSizeForBulkSelects()).willReturn(1000); @@ -278,30 +260,13 @@ public void testBuildWithEmptyColumnOrderingAndFetchSize() throws Exception { @SuppressWarnings("resource") /* Resources are closed in ResultSetIterator.close() automatically when caller attempts to advance past the last row in the result set. */ - ResultSetIterator resultSetIterator = new ResultSetIterator(table, Lists.newArrayList(), connection, Optional.of(connectionResources), sqlDialect); + ResultSetIterator resultSetIterator = new ResultSetIterator(table, Lists.newArrayList(), connection, Optional.of(connectionResources), sqlDialect); // Then - assertTrue(resultSetIterator.hasNext()); - resultSetIterator.next(); - resultSetIterator.next(); - - assertFalse(resultSetIterator.hasNext()); - - verify(resultSet).close(); - verify(statement).close(); verify(statement).setFetchSize(1000); - - - boolean gotException = false; - try { - resultSetIterator.next(); - } catch (NoSuchElementException e) { - gotException = true; - } - assertTrue(gotException); } - + private static Table buildTable() { return new Table() {