pgjdbc / pgjdbc

Postgresql JDBC Driver
http://jdbc.postgresql.org
BSD 2-Clause "Simplified" License
1.5k stars 851 forks source link

commit does not close resultsets. #843

Open swissgrammie opened 7 years ago

swissgrammie commented 7 years ago

I set both the connection holdability

        conn.setHoldability(CLOSE_CURSORS_AT_COMMIT);

and the statement

        Statement stmt = conn.createStatement(ResultSet.TYPE_SCROLL_INSENSITIVE,
                ResultSet.CONCUR_READ_ONLY,
                ResultSet.CLOSE_CURSORS_AT_COMMIT);

        ResultSet rs = stmt.executeQuery("select * from dummy");

But when I issue a commit the resultset remains open conn.commit();

The spec says that a commit() close resultsets so there is a little wiggle room here BUT when i explicitly set the holdability level I would expect a resultset to close after a commit.

davecramer commented 7 years ago

Can you show me where the spec says this ? Are you conflating closing the cursor at commit ?

swissgrammie commented 7 years ago

In 15.2.5 the spec says: A ResultSet object is implicitly closed when [...] the ResultSet is created with a Holdability of CLOSE_CURSORS_AT_COMMIT and an implicit or explicit commit occurs

On Tue, Jun 13, 2017 at 12:01 PM Dave Cramer notifications@github.com wrote:

Can you show me where the spec says this ? Are you conflating closing the cursor at commit ?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/pgjdbc/pgjdbc/issues/843#issuecomment-308068338, or mute the thread https://github.com/notifications/unsubscribe-auth/AB9NHVlzl0ZEtavem0osHyxbNeZ7Lt4Gks5sDl3rgaJpZM4N4FJS .

swissgrammie commented 7 years ago

from the same section, it also states that the resultset is closed if the connection is closed, something I also found not to be true with postgres

On Tue, Jun 13, 2017 at 12:01 PM Dave Cramer notifications@github.com wrote:

Can you show me where the spec says this ? Are you conflating closing the cursor at commit ?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/pgjdbc/pgjdbc/issues/843#issuecomment-308068338, or mute the thread https://github.com/notifications/unsubscribe-auth/AB9NHVlzl0ZEtavem0osHyxbNeZ7Lt4Gks5sDl3rgaJpZM4N4FJS .

davecramer commented 7 years ago

The only challenge with this is that in autocommit mode the result set would be closed before it was returned. Closing the connection should close the result set for sure. Although I personally would never rely on implicit behaviour in my code

swissgrammie commented 7 years ago

whilst i agree with you about the poor coding practice of relying on implicit behavior, life has handed me some lemons... I am, unfortunately, dealing with a codebase that has connections and commits handled by a transaction environment. Cursors (held and non held) are managed in a different class (in a different packaged product) and because of the way the 2 products are structured I have no connection between the 2 other than being able to check if the resultset has closed.

This works fine for db2, the original target, but I have to ensure that it also works for other databases. I attempted a simple workaround, by explicitly closing the connection and simply accepting the potential performance loss, but postgres still reports the resultset to be open even after a connection.close().

davecramer commented 7 years ago

Fair enough. I had a quick perusal of the code and we are relying on GC to close things if they are not explicitly closed. We'd be grateful for pull requests!

swissgrammie commented 7 years ago

ok I am an open source noob so bear with me on the pull request. I will get to it this evening/tomorrow morning.

brettwooldridge commented 7 years ago

@swissgrammie I am working on this in conjunction with the above referenced issue #855.