Java JDBC ResultSet closed

Question

When running this code, I get a ResultSet closed error. The called methods and data are as follows:

Log output:

[2014-05-05 22:34:09.169 Debug] select count(*) as total from companies
[2014-05-05 22:34:09.170 Debug] ResultSet closed

Methods:

public static Boolean recordsExist(String string, Connection c) {
    try {
        String[] query = string.split("\\*");
        String sql = "select count(*) as total" + query[1];
        ResultSet resultset = queryDB(sql, c);
        resultset.next();
        int count = resultset.getInt(1);
        Log.debug(Integer.toString(count));
        resultset.close();
        if (count > 0) {
            Log.debug("recordsExist returning true");
            return true;
        } else {
            Log.debug("recordsExist returning false");
            return false;
        }

    } catch (Exception e) {
        Log.debug(e.getMessage());
        return false;
    }
}

public static ResultSet queryDB(String sql, Connection c) throws SQLException {
    Log.debug(sql);
    Statement s = c.createStatement();
    ResultSet resultset = s.executeQuery(sql);
    s.close();
    return resultset;
}

Sql string specified:

select * from companies

also why do you close resultset in try block ? you should close in finally block — user3470953, May 06 '14 at 05:57
Unfortunately, your exception handling swallows the stack trace, which is kind of an anti-pattern. — Dawood ibn Kareem, May 06 '14 at 06:20

Emil L · Answer 1 · 2014-05-06T06:06:40.190

From the Java API:

Note:When a Statement object is closed, its current ResultSet object, if one exists, is also closed.

You will need to process the data in the ResultSet before closing the Statement. Something along these lines should work:

public static Boolean recordsExist(String string, Connection c) {
    try {
        String[] query = string.split("\\*");
        String sql = "select count(*) as total" + query[1];
        return queryDB(sql, c);


    } catch (Exception e) {
        Log.debug(e.getMessage());
        return false;
    }
}

public static boolean queryDB(String sql, Connection c) throws SQLException {
    Log.debug(sql);
    Statement s = c.createStatement();
    ResultSet resultset = s.executeQuery(sql);
    boolean result = processResult(resultset);
    s.close();
    return result;
}

public static boolean processResult(ResultSet resultset) {
    resultset.next();
    int count = resultset.getInt(1);
    Log.debug(Integer.toString(count));
    resultset.close(); 
    if (count > 0) {
        Log.debug("recordsExist returning true");
        return true;
    } else {
        Log.debug("recordsExist returning false");
        return false;
    }

}

I would also consider adding more error handling to ensure that you do not leak resources. For instance if an Exception occurs when processing the result then your statement will not be properly closed.

Deepu--Java · Accepted Answer · 2014-05-06T06:01:30.343

2

Don't close the Statement. Scenario is like this first close resultset then statement. Write you code like this:

    public static Boolean recordsExist(String string, Connection c) {
    try {
        String[] query = string.split("\\*");
        String sql = "select count(*) as total" + query[1];
        Statement s = c.createStatement();
        ResultSet resultset = queryDB(sql, c, s);
        resultset.next();
        int count = resultset.getInt(1);
        Log.debug(Integer.toString(count));
        resultset.close();
        s.close();
        if (count > 0) {
            Log.debug("recordsExist returning true");
            return true;
        } else {
            Log.debug("recordsExist returning false");
            return false;
        }

    } catch (Exception e) {
        Log.debug(e.getMessage());
        return false;
    }
}

public static ResultSet queryDB(String sql, Connection c, Statement s ) throws SQLException {
    Log.debug(sql);
    ResultSet resultset = s.executeQuery(sql);
    return resultset;
}

edited May 06 '14 at 06:01

answered May 06 '14 at 05:53

Deepu--Java

3,742
3
19
30

you never have a chance to close the statement. – Xing Fei May 06 '14 at 05:55
I think you should clear you first. I am telling him the reason not telling how to write the code. – Deepu--Java May 06 '14 at 05:57
@XingFei it's for you. See the edit and clear yourself. This is the solution. – Deepu--Java May 06 '14 at 06:03
yes it is better than the previous code, but what if queryDB causes a SQLException, the statement is not closed. I know you are telling him where he is wrong, but your code is not correct, only better than his. – Xing Fei May 06 '14 at 06:09
1

This is the correct answer. It explains what has gone wrong, and suggests a fix. Sure, it's not the best code in the world, but it's as good as it gets without giving an entire lesson on exception handling. – Dawood ibn Kareem May 06 '14 at 06:22

score 1 · Answer 3 · edited May 23 '17 at 12:11

Always check for next result set first as shown below

 if(resultset.next()){
     int count = resultset.getInt(1);
 }

I have already posted a nice ConnectionUtil class to manage all the connections in a single class for whole application.

Close the Statement, ResultSet and Connection in finally clause in the end to make sure that all are closed in any case as shown in below code.

Connection conn = null;
PreparedStatement stmt = null;
ResultSet rs = null;
try {
    ...
} finally {
    if (rs != null) {
        rs.close();
    }
    if (stmt != null) {
        stmt.close();
    }
    if (conn != null) {
        conn.close();
    }
}

Note: Don't use Log.debug(e.getMessage()); for ERROR message otherwise you will be lost in logs to find out the errors only.

Follow what I have suggested to make your code more clear to understand. close all the things at one place in `finally` clause. — Braj, May 06 '14 at 06:38

Java JDBC ResultSet closed

3 Answers3