Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't get Statement from closed ResultSet #1130

Merged
merged 1 commit into from
Oct 4, 2024

Conversation

finlaycurran
Copy link
Contributor

Summary

Don't attempt to get a statement from a closed ResultSet in getConnectionFromSqlObject.

Description

It's possible that getConnectionFromSqlObject is called on a ResultSet that is already closed, e.g. in DefaultConnectionPlugin when calling .close on a ResultSet. With some implementations, such as PgResultSet, calling getStatement on a closed ResultSet throws an SQLException. There is a performance overhead associated with creating and catching these exceptions everytime a ResultSet is used, and closed, so it should be avoided.

This is similar to an earlier issue fixed here: #682

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@sergiyvamz
Copy link
Contributor

Hi @finlaycurran

Thank you for your PR! We appreciate your help! Your PR is approved but it seems it's not up-to-date and needs to be rebased. Could you please rebase it?

Thank you!

It's possible that getConnectionFromSqlObject is called on a ResultSet that is already closed,
e.g. in DefaultConnectionPlugin when calling .close on a ResultSet. With some implementations,
such as PgResultSet, calling getStatement on a closed ResultSet throws an SQLException. There is a
performance overhead associated with creating and catching these exceptions everytime a ResultSet is
used, and closed, so it should be avoided.
@finlaycurran
Copy link
Contributor Author

@sergiyvamz done!

@sergiyvamz sergiyvamz merged commit 3459f83 into aws:main Oct 4, 2024
5 checks passed
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.

2 participants