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

Closing Connection Does Not Close Statement And ResultSet #1903

Closed
danuvian opened this issue Sep 30, 2020 · 6 comments · Fixed by hmcts/rse-case-framework#187 or PatilShreyas/NotyKT#144

Comments

@danuvian
Copy link

Describe the issue

(1) Closing the connection object does not close the statement and resultset objects, which it should according to what I read online about JDBC.

(2) Closing the statement object does not close the resultset object.

I tried a different database and driver (MySQL) and verified that the MySQL JDBC driver DOES close these two objects once the connection is closed.

Driver Version?

42.2.16

Java Version?

openjdk version "11.0.7" 2020-04-14

OS Version?

MacOS Catalina - 10.15.6

PostgreSQL Version?

Docker image - "postgres:13"

To Reproduce
Steps to reproduce the behaviour:

public static void main(String[] args) throws Exception {
        String[] dbInfo = {"jdbc:postgresql:my_database", "postgres", "fakepassword"};
        if(args != null && ArrayUtils.contains(args, "mysql"))  {
            dbInfo = new String[]{"jdbc:mysql://localhost:3306/my_database", "mysql", "fakepassword"};
        }
        System.out.println("Trying to connect to: " + dbInfo[0]);

        Connection con = DriverManager.getConnection(dbInfo[0], dbInfo[1], dbInfo[2]);
        PreparedStatement ps = con.prepareStatement("select * from courses"); 
        ResultSet rs = ps.executeQuery();
        while (rs.next()) {
            System.out.println("id: " + rs.getInt("id") + ", code: " + rs.getString("code") + ", description: "
                    + rs.getString("description"));
        }

        con.close();
        // postresql is not closing resultset or preparedstatement upon con closing!!!
        // postresql:   con closed? true, ps closed? false, rs closed? false
        // mysql:       con closed? true, ps closed? true, rs closed? true
        System.out.println("con closed? " + con.isClosed() + ", ps closed? " + ps.isClosed() 
            + ", rs closed? " + rs.isClosed());
    }

Expected behaviour
A clear and concise description of what you expected to happen.
And what actually happens

Closing connection object closes the statement and resultset objects but what actually happens is that the statement and resultset object are still open (isClosed() returns false).

Closing the statement object closes the resultset object but what actually happens is that the resultset object is still open (isClosed() returns false).

Logs
If possible PostgreSQL logs surrounding the occurrence of the issue
Additionally logs from the driver can be obtained adding

loggerLevel=TRACE&loggerFile=pgjdbc-trace.log 

to the connection string
Using the following template code make sure the bug can be replicated in the driver alone.

import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.ResultSet;
import java.sql.Statement;
import java.util.Properties;

public class TestNullsFirst {
    public static void main(String []args) throws Exception {


        String url = "jdbc:postgresql://localhost:5432/test";

        Properties props = new Properties();
        props.setProperty("user", "test");
        props.setProperty("password", "test");
        try ( Connection conn = DriverManager.getConnection(url, props) ){
            try ( Statement statement = conn.createStatement() ) {
                try (ResultSet rs = statement.executeQuery( "select lastname from users order by lastname asc nulls first") ){
                    if (rs.next())
                        System.out.println( "Get String: " + rs.getString(1));
                }
            }
        }
    }
}
@davecramer
Copy link
Member

While they may close them this is not a requirement of the spec as far as I can tell.

https://docs.oracle.com/javase/7/docs/api/java/sql/Connection.html#close()

@danuvian
Copy link
Author

danuvian commented Sep 30, 2020

@davecramer

I googled and found some links that say it should be closed but then there are people that say the statement and resultset should be assumed to be closed by closing the connection.

The first answer says it should be closed:
https://stackoverflow.com/questions/14024311/does-closing-connection-automatically-close-statement-and-resultset

Not the most popular answer but... from Enerccio:
https://stackoverflow.com/questions/4507440/must-jdbc-resultsets-and-statements-be-closed-separately-although-the-connection

In any case, whether it is in the JDBC spec of not, it would be NICE to have the connection close() also close the statement and resultset. It seems like some other drivers already do that (MySQL) and having PG implement this feature would be really neat, as that would facilitate just using try-with-resources on the Connection object itself and have it clean up everything (con, stmt, rs) once that block of code is done.

try(Connection con = DriverManager.getConnection(dbInfo[0], dbInfo[1], dbInfo[2])) {
PreparedStatement ps = con...
ResultSet rs = ps.executeQuery(...)
while(rs.next()) {
...
}
}
// At this point, everything is closed and is very convenient and no closing code to write for stmt and rs

Thank you.

@davecramer
Copy link
Member

Note that https://docs.oracle.com/javase/7/docs/api/java/sql/Statement.html#close()
Explicitly says the ResultSets are closed when the statement is closed. No such language exists for the connection. JDBC resources can mean anything. I'm curious why is this even an issue?

@danuvian
Copy link
Author

danuvian commented Sep 30, 2020

Thanks Dave for the link.

I tested closing the statement but the resultset object was still open (isClosed() = false). So I think this particular issue is a genuine bug.

To answer your question, as to why, first I thought it was required for the connection to close the stmt and rs so I thought it was a bug. From a certain point of view, one could think that whatever resources are closed in the pg implementation of resultset are "JDBC resources" and therefore should be closed. But let's say it's not required, then I would like to change the connection issue to a suggestion. Mainly I do not want to write close() on resultset and stmt objects and add a try/catches around then, when instead I could write a try-with-resources on the connection object and that would close all three objects after the block finishes. It would make the code less verbose and more elegant, in my opinion. And it would also duplicate the behavior in other drivers which I think provides a convenience.

davecramer added a commit to davecramer/pgjdbc that referenced this issue Sep 30, 2020
@davecramer
Copy link
Member

It's sort of actually closed internally. We set it to null in the statement. The object is invalid but the state is "open" I have a PR to fix it see above

@danuvian
Copy link
Author

danuvian commented Oct 1, 2020

Thanks Dave. It looks good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment