Need serious help here

[bryant_16]

Hi guys, I need help about database programming here.

Code:

String result = conManager.sqlExecuteQuery("select group_id, (select school_id from school where school_name='" + columns[3] + "') as school_id from groupstatus where group_type='" + columns[4] + "'");
                        rs = conManager.getResultSet();
                        String result2 = conManager2.sqlExecuteQuery("select group_id, (select school_id from school where school_name='" + columns[3] + "') as school_id from groupstatus where group_type='" + columns[4] + "'");

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

Code:

while (rs.next()) {
                            
                            while (rs2.next()) {
                                System.out.println("EGE");
                                System.out.println("INSERT INTO MEMBER VALUES('" + columns[0] + "','" + columns[2] + "','" + columns[1] + "','" + columns[5] + "'," + rs.getInt("group_id") + "," + rs.getInt("school_id") + ")");
                                //String hello = conManager.sqlExecute("insert into member values('" + columns[0] + "','" + columns[2] + "','" + columns[1] + "','" + columns[5] + "'," + rs.getInt("group_id") + "," + rs.getInt("school_id") + ")");
                              
                            }
                        }

This is what I have encountered after running the application.

This is what I am trying to do:
1) Read from csv files
2) Populate the values from the csv files into my database tables

[masijade]

When you execute a query on a Statement object, all resultsets currently associated with that statement will automatically be closed, and, let me guess, these "conManager.sqlExecute...." methods reuse statement objects.

Besides, what are you using result sets for when you are reading csv and inserting into a db? I see no reason in that to read from a db. If it's that you want to insert if it doesn't exists, and update if it does, then check out the sql merge statement.

(I.E. Google "SQL Merge")

[bryant_16]

Ya, I need to read from these csv files and insert its value to my database. My tables have primary and foreign keys, that is why from my first code wrap there, I will have to select those primary keys in other tables to populate into the other table which is the second code wrap over there...

But the resultset is always closed, any idea how do I go about solving it?

[masijade]

Like I said, make sure you are not re-using Statement objects. Rather create a new statement object foreach select query and have a separate one for the updates.

[bryant_16]

Oki, if thats the case, I will have 2 ResultSets right? Is there okay?

[bryant_16]

[code]
while ((columns = reader.readNext()) != null) {
rs = conManager.sqlExecuteQuery("select group_id, (select school_id from school where school_name='" + columns[3] + "') as school_id from groupstatus where group_type='" + columns[4] + "'");
while (rs.next()) {
String hello = conManager.sqlExecute("insert into member values('" + columns[0] + "','" + columns[2] + "','" + columns[1] + "','" + columns[5] + "'," + rs.getInt("group_id") + "," + rs.getInt("school_id") + ")");
}
}
Got back java.sql.SQLException: Operation not allowed after ResultSet closed

It only add one insert 'values' to my database while there are alot more of values to be added...

[masijade]

As I said, I don't know what this "conManager" thing is. Probably an in-house developed library. Well, it looks as though it is reusing the Statement object. Don't do that! I don't know any other way to tell this to you than I already have.

Look at that "conManager" class and make sure that it is not reusing Statement objects!

[bryant_16]

What you mean by reusing Statement objects? Over at my conManager, I have

Code:

public ResultSet sqlExecuteQuery(String executeStr) {
        try {
            //Execute the SQL Query statement
            rs = statement.executeQuery(executeStr);
            
        } catch (SQLException sqlError) {
            //If there is an error on the SQL query syntac, an error message will be returned
            //return ("Unable To Execute SQL Query : " + executeStr);
        }
        //return ResultSet to the method which calls to this sqlExecuteQuery
        return rs;
    }
public String sqlExecute(String executeStr) {
        try {
            //Execute the SQL statement
            statement.executeUpdate(executeStr);
            
        } catch (SQLException sqlError) {
            //If there is an SQL syntac arror, an error message will be printed out
            return ("Unable To Execute SQL Statement: " + executeStr);
        }
        //if there is no error message, an empty String will be returned
        return ("");
    }

[masijade]

Well, looking at that code, I can only assume that the variable "statement" is an instance variable. This means that every time a query is executed that the same statement is used every time (this is reusing statement objects). And, as I said earlier, when you execute a query on a Statement object, all ResultSets currently associated with that Statement object will be automatically closed. Like I said, create a new Statement object for each query, don't reuse the same one, or that what you currently see will always happen. Of course, this also means that you can't return a ResultSet from a method, since the Statement should also be closed immediately after using it (in a finally block, to be sure). Either read in the entire ResultSet and store it's contents in a List and return that, or redesign your entire program to facilitate the openeing and closing of Statements per query (which also means not returning a ResultSet).

[bryant_16]

Okay, so I have to add Statement statement = new Statement(); each time? Sorry, I'm rather lost in this.

[masijade]

Yes, and you need to close it in every method as well, but closing it will usually close the ResultSet as well (at least it is suppossed to). which, of course, means that you can't return the ResultSet from the method.

You need to refactor this code. The easiest way would be to write a "getStatement" method and call that everywhere right before calling one of these methods, then pass it to that method (which means modifying those methods to take a Statement as a parameter),
and to do that in a try block and add a finally block in which you should close that statement, doing it this way will allow you to return a resultset from the method (not that I think this is the "right" way to do it, but it is the easiest "refactoring" path, at the moment).

Edit:

For example

in this "conManager" class

Code:

  public Statement getStatement() {
    return conn.createStatement();
  }
  public ResultSet sqlExecuteQuery(String executeStr, Statement stmt) {
    ...
  }
  public String sqlExecute(String executeStr, Statement stmt) {
    ...
  }

Then in the calling methods

Code:

    Statement s1 = null;
    Statement s2 = null;
    Statement s3 = null;
  try {
    s1 = conManager.getStatement();
    s2 = conManager.getStatement();
    s3 = conManager.getStatement();
    ResultSet rs1 = conManager.sqlExecuteQuery(sql, s1);
    while (rs1.next()) {
      ResultSet rs2 = conManager.sqlExecuteQuery(sql2, s2);
      while (rs2.next()) {
        String result = conManager.sqlExecute(insertSql, s3);
      }
    }
  } finally {
    if (s1 != null) try { s1.close() } catch (SQLException sqle) {};
    if (s2 != null) try { s2.close() } catch (SQLException sqle) {};
    if (s3 != null) try { s3.close() } catch (SQLException sqle) {};
  }

As you can see above, s3 can be freely "reused" since you have no ResultSets dependant on it.
s2 can be reused outside of the inner while loop, because by that point the ResultSet has been exhausted.
s1 cannot be reused, as the ResultSet that is dependant on it has not been exhausted until the method is complete, anyway.

The above code does not do any error checking, and it also does not close the ResultSets (which it definately should). Those are for you to do, as well as adapting it to your code. I will reiterate, however, that I do not approve of this approach, only that it is one refactoring that offers the "path of least resistance", at this point in time.

[t.Donas]

Yes you are right I agree with you, I've realised after facing the broblem that Braynt is facing. Like this is the code I used to search for students:

public void trySearch()
{
String frstName, lstName;
int studNum;
int FinalGrd = 0;
try
{
int enterStudentNumber = Integer.parseInt(JOptionPane.showInputDialog(null, "Please enter student number & click Ok","Enter Student Number" ,JOptionPane.QUESTION_MESSAGE));

Statement st = conn.createStatement();

ResultSet rs1 = st.executeQuery("SELECT StudentNumber,FirstName,LastName FROM Students WHERE StudentNumber = '" + enterStudentNumber + "'");
ResultSet rs2 = st.executeQuery("SELECT Test1,Test2,Assignment1,Assignment2,Exam FROM Marks WHERE StudentNumber = '" + enterStudentNumber + "'");
ResultSet rs3 = st.executeQuery("SELECT FinalGrade FROM StudentGrade WHERE StudentNumber = '" + enterStudentNumber + "'");
while(rs1.next())
{
studNum = rs1.getInt("StudentNumber");
frstName = rs1.getString("FirstName");
lstName = rs1.getString("LastName");

txtStudentNumber.setText(String.valueOf(studNum));
txtFirstName.setText(frstName);
txtLastName.setText(lstName);
}
while(rs2.next())
{
intTest1 = rs2.getInt("Test1");
intTest2 = rs2.getInt("Test2");
intAssignment1 = rs2.getInt("Assignment1");
intAssignment2 = rs2.getInt("Assignment2");
intExam = rs2.getInt("Exam");

txtClassTest1.setText(String.valueOf(intTest1));
txtClassTest2.setText(String.valueOf(intTest2));
txtClassAssignment1.setText(String.valueOf(intAssi gnment1));
txtClassAssignment2.setText(String.valueOf(intAssi gnment2));
txtExamination.setText(String.valueOf(intExam));
}
while(rs3.next())
{
FinalGrd = rs3.getInt("FinalGrade");

txtFinalGrade.setText(String.valueOf(FinalGrd));
}
}
catch(SQLException sql3)
{
JOptionPane.showMessageDialog(null,"SQL (select) error " + sql3.getMessage(),"Error Occured",JOptionPane.ERROR_MESSAGE);
}
}