Skip to content

Conversation

eddumelendez
Copy link
Contributor

See SPR-13826

@jmithmstr
Copy link

Please, do not use try-with-resources as exceptions from close() are not ignored. And I do not want those exceptions thrown up when everything but the close was OK.

@snicoll snicoll changed the title Move to JDBC 4.1 SPR-13826 - Move to JDBC 4.1 Jul 5, 2016
CallableStatement cs = null;
try {
try (Connection con = DataSourceUtils.getConnection(getDataSource())) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're not sure yet we want to do that. And we probably won't. Can you push force an update that doesn't use try/resource please?

See SPR-13826
@eddumelendez
Copy link
Contributor Author

@snicoll PR updated. Thanks

@jhoeller
Copy link
Contributor

@eddumelendez I share the concerns about try-with-resources. We do not just have transactional logic in DataSourceUtils.releaseConnection but also custom logging with no rethrowing in closeStatement and closeResultSet. Since there is no user-level benefit in changing those to try-with-resources, as far as I can see, let's rather leave this as-is.

Thanks for your efforts in any case!

@jhoeller jhoeller closed this Jul 12, 2016
@jhoeller
Copy link
Contributor

jhoeller commented Jul 12, 2016

@eddumelendez I appreciate your efforts but I share @jmithmstr's concerns about try-with-resources. We have some custom logging logic - suppressing any close exception - in all of our close delegates, not just the transactional logic in DataSourceUtils.releaseConnection... Since there is no user-level benefit in changing those to try-with-resources, let's rather leave this as-is.

@jhoeller
Copy link
Contributor

jhoeller commented Jul 12, 2016

As for SPR-13826, that issue is more about relying on JDBC 4.1 features such as getParameterType than about the resource handling code. Since unconditional use of such JDBC operations requires full support in all common JDBC drivers out there, it's constrained by the drivers and not by the JDBC API level in JDK 7/8.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants