Skip to content

Commit

Permalink
PR feedback #3
Browse files Browse the repository at this point in the history
PR feedback #3
Adding debug logging
  • Loading branch information
jdm43 committed May 2, 2023
1 parent 8d4f946 commit 7554cf6
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,17 @@
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.
*
* @author Copyright (c) Alfa Financial Software 2017
*/
class ResultSetIterator implements Iterator<Record>, AutoCloseable {
/** Standard logger */
private static final Log log = LogFactory.getLog(ResultSetIterator.class);

/**
* The underlying result set to iterate over.
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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();
Expand Down Expand Up @@ -548,10 +548,13 @@ private <T> T executeQuery(String sql, Iterable<SqlParameter> 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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}


Expand All @@ -270,38 +253,20 @@ 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);


// 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, 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() {

Expand Down

0 comments on commit 7554cf6

Please sign in to comment.