10

I have a problem with closing a connection to MySQL.

I'm getting the error:

java.sql.SQLException: Operation not allowed after ResultSet closed

My code:

public static ResultSet sqlquery (String query)
{
 ResultSet rs=null;
 Connection connection=null;
 Statement st=null;
 try{   
     Class.forName("com.mysql.jdbc.Driver");
     connection = DriverManager.getConnection("databaseadress","username","password");
     st = connection.createStatement();  
     rs = st.executeQuery(query);

    }catch(SQLException e){System.out.println("SQL error: " + e);}
      catch(Exception e){System.out.println("Error: " + e);}
       finally {
       try{
          if(rs != null) rs.close();
          if(st!= null) st.close();
          if(connection != null)  connection.close();
  }catch(SQLException e){System.out.println("SQL error : " + e);}

    }
     return rs;
}
eebbesen
  • 5,070
  • 8
  • 48
  • 70
mlodikkal
  • 113
  • 1
  • 1
  • 4
  • 1
    I'm no database expert, but it seems to me that you don't want to close your connection until you're done with the ResultSet. So what happens when you change your code to reflect this? Why not extract the data from your ResultSet within the method, and return an ArrayList of whatever objects you wish to create with your data, and then return this list from the method? – Hovercraft Full Of Eels Aug 25 '14 at 20:30
  • 1
    You closed the ResultSet, so everything is deleted. Why you want to use it? Extract the data before you close it and return the data – Marco Acierno Aug 25 '14 at 20:32
  • Which line of code is the exception occurring on? Are you sure it's one that you've shown here? – Dawood ibn Kareem Aug 25 '14 at 20:36
  • It appears he's returning the `ResultSet` object and getting the exception later in code not shown here. – Jim Garrison Aug 25 '14 at 20:37
  • @JimGarrison Yes, I know that. I was hoping that by examining the stack trace, he'd figure that out. – Dawood ibn Kareem Aug 25 '14 at 20:37
  • Yes, I use function sqlquerry in other part of the code. It worked without connection closed, but I've realized that it's not a good idea ;) – mlodikkal Aug 25 '14 at 21:13

4 Answers4

14

JDBC doesn't bring back all the results of a query in a ResultSet, because there may be too many of them to fetch them all eagerly. Instead it gives you something you can use to retrieve the results, but which goes away when the connection closes. So when you pass it back from your method after closing the database connection, nothing else can use it.

What you can do instead is to have this method use the resultSet to populate an object or a collection of objects, and pass that populated object back.

If you change your code to pass in a rowMapper (that takes a resultSet and passes back an object populated with the current row in the resultset), and use that to populate a container object that you pass back, then you'll have something as reusable as what what you've written, but which actually works because it doesn't depend on having the connection held open after the call is finished.

Here's your example code rewritten to use the rowmapper, get rid of some unnecessary exception-catching, and fix a bug that will prevent the connection from getting closed in some cases:

public static List<T> sqlquery (String query, RowMapper<T> rowMapper) throws SQLException
{
    Connection connection=null;
    Statement st=null;
    ResultSet rs=null;
    // don't need Class.forName anymore with type4 driver     
    connection = DriverManager.getConnection("databaseadress","username","password");
    st = connection.createStatement();  
    rs = st.executeQuery(query);
    List<T> list = new ArrayList<T>();
    while (rs.next()) {
        list.add(rowMapper.mapRow(rs));
    }
    // don't let exception thrown on close of
    // statement or resultset prevent the
    // connection from getting closed
    if(rs != null) 
        try {rs.close()} catch (SQLException e){log.info(e);}
    if(st!= null) 
        try {st.close()} catch (SQLException e){log.info(e);}
    if(connection != null)  
        try {connection.close()} catch (SQLException e){log.info(e);}
    return list;
}

If you don't catch each exception thrown on close individually, as shown above, you risk failing to close the connection if either the statement or the resultSet throw an exception on close.

This is similar to what spring-jdbc does, it defines a RowMapper as:

public interface RowMapper<T> {
    T mapRow(ResultSet, int rowNum) throws SQLException;
}

The next step is going to be parameterizing your queries so you don't have to surround parameter values in quotes or worry about sql injection. See this answer for an example of how spring-jdbc handles this. The long term answer here is, it would be better to adopt spring-jdbc or something similar to it than to reinvent it piecemeal.

Community
  • 1
  • 1
Nathan Hughes
  • 94,330
  • 19
  • 181
  • 276
10

This is the way JDBC works. In your code you closed the ResultSet and the Connection, after which the ResultSet is no longer usable. If you want it to be usable you must leave it (and the Connection) open.

However, if you return the ResultSet, you should refactor your code so the calling method provides the Connection.

Jim Garrison
  • 85,615
  • 20
  • 155
  • 190
3
RowSetFactory factory = RowSetProvider.newFactory();
CachedRowSet rowset = factory.createCachedRowSet();
rowset.populate(ResultSet data)

/* now you can close you connection and prepare statement*/

Boris Fain
  • 11
  • 2
1

Once the connection is closed you can no longer use any of the resources (statements, prepared statements, result sets), all of them are automatically closed. So you need to do all of your processing while the resources are open.

Try filling and returning a DTO, this way you can have the data you need without keeping a connection alive.

Paco Lagunas
  • 320
  • 1
  • 13