0

I am reading an excel file that contains 30000 rows and trying to update an Oracle dB table field based on some logic. My Java application error out "java.sql.SQLException: - ORA-01000: maximum open cursors exceeded" when it writes approximately 700th record in the table. Need help in optimising the code so as to avoid this error.

import java.io.FileInputStream;
import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.PreparedStatement;
import java.sql.SQLException;
import java.util.Iterator;
import java.util.ResourceBundle;

import org.apache.poi.xssf.usermodel.XSSFRow;
import org.apache.poi.xssf.usermodel.XSSFSheet;
import org.apache.poi.xssf.usermodel.XSSFWorkbook;

import oracle.jdbc.driver.OracleDriver;

public class UpdateTest {
    private static Connection conn = null;
    static ResourceBundle bundle = ResourceBundle.getBundle("PropertiesFile");
    public static void main(String[] args) {

    String filename = bundle.getString("FILEPATH") + bundle.getString("FILENAME");
    FileInputStream fileInputStream = null;
    String input = null;
    PreparedStatement preparedStatement = null;
    Integer result = null;
    int counter = 0;

    try {
        DriverManager.registerDriver(new OracleDriver());
        conn = DriverManager.getConnection(
                bundle.getString("DATABASE_URL"), 
                bundle.getString("DATABASE_USERNAME"),
                bundle.getString("DATABASE_PASSWORD"));
        conn.setAutoCommit(false);

        fileInputStream = new FileInputStream(filename);
        XSSFWorkbook workbook = new XSSFWorkbook(fileInputStream);
        XSSFSheet sheet = workbook.getSheetAt(0);
        System.out.println("Number of records to be updated: " + (sheet.getPhysicalNumberOfRows() - 1));

        Iterator i = sheet.iterator();
        while (i.hasNext()) {
            XSSFRow row = (XSSFRow) i.next();
            input = row.getCell(0).toString();
            preparedStatement = conn.prepareStatement("update table1 set column1='value' where input=?");
            preparedStatement.setString(1, input);
            result = preparedStatement.executeUpdate();
        }

        if (preparedStatement != null) {
            preparedStatement.close();
        }
        conn.commit();
        conn.close();

    } catch (Exception e) {
        if (conn != null) {
            try {
                conn.rollback();
            } catch (SQLException e1) {
                e1.printStackTrace();
            }
        }
        e.printStackTrace();
    } finally {
        try {
            if (conn != null && !conn.isClosed()) {
                if (!conn.getAutoCommit()) {
                    conn.commit();
                    conn.setAutoCommit(true);
                }
                conn.close();
                conn = null;
            }
        } catch (SQLException e) {
            e.printStackTrace();
        }
    }

}

}
Mark Rotteveel
  • 100,966
  • 191
  • 140
  • 197
IMJS
  • 863
  • 2
  • 13
  • 25

2 Answers2

6

Each call to prepareStatement() creates a new cursor in the Oracle server which is not closed in your loop.

The correct solution to avoid the "too many open cursors" is to only create one cursor by preparing the statement only once before the loop.

preparedStatement = conn.prepareStatement("update table1 set column1='value' where input=?");

Iterator i = sheet.iterator();
while (i.hasNext()) {
    XSSFRow row = (XSSFRow) i.next();
    input = row.getCell(0).toString();
    preparedStatement.setString(1, input);
    result = preparedStatement.executeUpdate();
 }

Then close it after the loop in a finally block.

Calling prepareStatement() in a loop defeats the purpose and intention of a PreparedStatement.

  • 1
    The problem is not directly the creation of multiple prepared statements, but forgetting to close those multiple prepared statements (except the last one). – Mark Rotteveel Apr 20 '18 at 11:34
  • @MarkRotteveel: preparing the same SQL string over and over and over again in a loop **is** a problem. –  Apr 20 '18 at 11:35
  • 1
    I don't disagree with that, but the primary cause for the _"ORA-01000: maximum open cursors exceeded"_ error is preparing multiple statements in the loop and only closing the last one. Sure, not preparing the same statements multiple times avoids the issue, but knowing **why** also makes it easier to avoid the problem in other cases. – Mark Rotteveel Apr 20 '18 at 11:39
  • 1
    I thought I addressed that ;) I have edited my answer to make that more obvious. –  Apr 20 '18 at 11:47
  • @a_horse_with_no_name - Thanks! This solves the issue. – IMJS Apr 23 '18 at 05:49
1

Move your preparedStatement.close() inside of while:

preparedStatement = conn.prepareStatement("update table1 set column1='value' where input=?");
while (i.hasNext()) {
    XSSFRow row = (XSSFRow) i.next();
    input = row.getCell(0).toString();

    preparedStatement.clearParameters();
    preparedStatement.setString(1, input);
    result = preparedStatement.executeUpdate();
}

if (preparedStatement != null) {
    preparedStatement.close();
}

When assings a new preparedStatement you are losing the reference and only it is closing the last preparedStatement.

If you use ResultSet on other part of your code, remember close it too if you are doing a loop.

EDIT: Reusing the prepared statement, you can close it outside of loop. More details here

Alberto
  • 745
  • 1
  • 6
  • 25
  • _PreparedStatement_ should be closed in _finnaly_ block, because sometimes you may encounter an exception – mkuligowski Apr 20 '18 at 10:13
  • 1
    Yes, it is right, but it is not closed in while, you will are consuming resources are not being used. – Alberto Apr 20 '18 at 10:16
  • 1
    Yes if reuse the PreparedStatement (now edited), but if not, must be closed. You can see it in https://stackoverflow.com/questions/2467125/reusing-a-preparedstatement-multiple-times – Alberto Apr 20 '18 at 10:55