1

This question should be simple and it may annoy , but still I got this doubt about closing of Result Set using Java. It should be done for each statement or result set should be closed only in final?

       try {
        Class.forName(<driver class>);                
        con = DriverManager.getConnection(
            "IP", 
            "username",
            "password");
        for(String dealId : items) {
            String sql= "SQL Query";
            preparedStatement = con.prepareStatement(sql);
            rs = preparedStatement.executeQuery();
            while(rs.next()) {
                count += rs.getInt("total");                    
            }
            // Result should be closed here as the statement got executed?
        }
        System.out.println(count);
        if(items.size() == count) {
            dealsBelongToTheParty = true;
        }

    } catch (Exception e) {
        e.printStackTrace();
    } finally {
        rs.close(); // Or this is right?
        preparedStatement.close();
        if(!con.isClosed())
            con.close();
    }
Aliy
  • 197
  • 14
  • 1
    best done in finally block – Ramanlfc Sep 28 '17 at 11:22
  • 3
    even better would be to use try-with-resource – Sharon Ben Asher Sep 28 '17 at 11:22
  • You seem to be creating a new result set in each iteration of your `for` loop. In that case, the logical thing would be to close each one in the for loop, instead of just closing whichever one is left at the end. – khelwood Sep 28 '17 at 11:23
  • Cramming multiple close statements in one finally block is a really bad idea. You'll cause a connection leak. – Nathan Hughes Sep 28 '17 at 11:35
  • You are leaking both ResultSets and PreparedStatements. You should close both every time around the loop. Fortunately provided you are using a fully compliant JDBC driver they will all get closed when you close the statement. But that may not be soon enough for all purposes. – user207421 Sep 28 '17 at 11:35
  • 1) First of all put preparedStatement = con.prepareStatement(sql); outside the loop. 2) close ResultSet each time: rs = preparedStatement.executeQuery(); creates a new ResultSet each time, the previous resutl set remains unclosed – Evgeniy Dorofeev Sep 28 '17 at 11:36
  • @EvgeniyDorofeev it is assumed that the loop var `dealId` is used in the query... – Sharon Ben Asher Sep 28 '17 at 11:37
  • @ Sharon Ben Asher yes, but this is supposed to be parameter of parameterized SQL – Evgeniy Dorofeev Sep 28 '17 at 11:39
  • @EvgeniyDorofeev this is another issue of optimizing the sql query. it will work just fine if the variable is embedded in the sql string – Sharon Ben Asher Sep 28 '17 at 11:41
  • @EJP, Can you provide the exact solution where there will be no leakages? – Aliy Sep 28 '17 at 11:44

2 Answers2

2

I suggest to use Java 7 try-with-resources
(the code below assumes that the loop var dealId is used in the query)

Class.forName(<driver class>);                
try (Connection con = DriverManager.getConnection(
        "IP", "username", "password")) {
    for(String dealId : items) {
        String sql= "SQL Query with " + dealId;
        // resources are opened by order 
        try (PreparedStatement preparedStatement = con.prepareStatement(sql);
             ResultSet rs = preparedStatement.executeQuery()) {
            while(rs.next()) {
                count += rs.getInt("total");                    
            }
        } // resources are implicitly closed in reverse order of open
    }
} catch (Exception e) {
    e.printStackTrace();
}

System.out.println(count);
if(items.size() == count) {
    dealsBelongToTheParty = true;
}
Sharon Ben Asher
  • 13,849
  • 5
  • 33
  • 47
0

https://docs.oracle.com/javase/7/docs/api/java/sql/ResultSet.html

"A ResultSet object is automatically closed when the Statement object that generated it is closed, re-executed, or used to retrieve the next result from a sequence of multiple results."

So, you shouldn't worry about closing ResultSet.

DDovzhenko
  • 1,295
  • 1
  • 15
  • 34