I don't know why the boolean closeResources
parameter is needed
Except for the queryForStream
method, virtually all of them are true
and there is no reason for queryForStream
to be false either
Comment From: snicoll
there is no reason for queryForStream to be false either
What makes you think that?
Comment From: sbrannen
Hi @thdwoqor,
Congratulations on submitting your first PR for the Spring Framework! 👍
Unfortunately we cannot accept this, since it breaks the contract of queryForStream()
.
I don't know why the
boolean closeResources
parameter is needed Except for thequeryForStream
method, virtually all of them aretrue
and there is no reason forqueryForStream
to be false either
queryForStream()
returns a Stream
that the user is required to close, as per the documentation in the Javadoc for that method.
The reason that queryForStream()
invokes execute(new StreamStatementCallback(), false)
with false
is that the StreamStatementCallback
has an onClose()
method which closes the resources when the user closes the stream.
In light of that, I am closing this issue as invalid.
Comment From: thdwoqor
@snicoll @sbrannen
@Override
public <T> Stream<T> queryForStream(String sql, RowMapper<T> rowMapper) throws DataAccessException {
class StreamStatementCallback implements StatementCallback<Stream<T>>, SqlProvider {
@Override
public Stream<T> doInStatement(Statement stmt) throws SQLException {
ResultSet rs = stmt.executeQuery(sql);
Connection con = stmt.getConnection();
return new ResultSetSpliterator<>(rs, rowMapper).stream().onClose(() -> {
JdbcUtils.closeResultSet(rs);
JdbcUtils.closeStatement(stmt); // this
DataSourceUtils.releaseConnection(con, getDataSource()); // this
});
}
@Override
public String getSql() {
return sql;
}
}
return result(execute(new StreamStatementCallback(), false));
}
The queryForStream method uses the closeStatement and releaseConnection in doInStatement, and isn't that effectively closeResources true?
Comment From: sbrannen
The queryForStream method uses the closeStatement and releaseConnection in doInStatement, and isn't that effectively closeResources true?
No.
It actually does not invoke that in doInStatement
.
Rather, as I mentioned in https://github.com/spring-projects/spring-framework/pull/31384#issuecomment-1752919505, that code is a lambda expression supplied to onClose()
which is only invoked if the user closes the Stream
.
Comment From: sbrannen
@thdwoqor, if you have further questions, please keep our policy in mind:
Thanks for getting in touch, but it feels like this is a question that would be better suited to Stack Overflow. As mentioned in the guidelines for contributing, we prefer to use the issue tracker only for bugs and enhancements. Feel free to update this issue with a link to the re-posted question (so that other people can find it) or add some more details if you feel this is a genuine bug.