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 the queryForStream method, virtually all of them are true and there is no reason for queryForStream 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.