8

Below is the code which I am using to insert multiple records( around 5000-7000) in the Oracle Database using Prepared Statement.

The way I am doing currently is good? Or it can be improve more using some batch thing?

pstatement = db_connection.prepareStatement(PDSLnPConstants.UPSERT_SQL);

for (Entry<Integer, LinkedHashMap<Integer, String>> entry : MAPPING.entrySet()) {

    pstatement.setInt(1, entry.getKey());
    pstatement.setString(2, entry.getValue().get(LnPConstants.CGUID_ID));
    pstatement.setString(3, entry.getValue().get(LnPConstants.PGUID_ID));
    pstatement.setString(4, entry.getValue().get(LnPConstants.SGUID_ID));
    pstatement.setString(5, entry.getValue().get(LnPConstants.UID_ID));
    pstatement.setString(6, entry.getValue().get(LnPConstants.ULOC_ID));
    pstatement.setString(7, entry.getValue().get(LnPConstants.SLOC_ID));
    pstatement.setString(8, entry.getValue().get(LnPConstants.PLOC_ID));
    pstatement.setString(9, entry.getValue().get(LnPConstants.ALOC_ID));
    pstatement.setString(10, entry.getValue().get(LnPConstants.SITE_ID));
    pstatement.executeUpdate();

    pstatement.clearParameters();
}

Udpated Code That I am Using:-

public void runNextCommand() {

    Connection db_connection = null;
    PreparedStatement pstatement = null;
    int batchLimit = 1000;
    boolean autoCommit = false;

    try {
        db_connection = getDBConnection();

        autoCommit = db_connection.getAutoCommit();
        db_connection.setAutoCommit(false); //Turn off autoCommit
        pstatement = db_connection.prepareStatement(LnPConstants.UPSERT_SQL); // create a statement

        for (Entry<Integer, LinkedHashMap<Integer, String>> entry : GUID_ID_MAPPING.entrySet()) {
            pstatement.setInt(1, entry.getKey());
            pstatement.setString(2, entry.getValue().get(LnPConstants.CGUID_ID));
            pstatement.setString(3, entry.getValue().get(LnPConstants.PGUID_ID));
            pstatement.setString(4, entry.getValue().get(LnPConstants.SGUID_ID));
            pstatement.setString(5, entry.getValue().get(LnPConstants.UID_ID));
            pstatement.setString(6, entry.getValue().get(LnPConstants.ULOC_ID));
            pstatement.setString(7, entry.getValue().get(LnPConstants.SLOC_ID));
            pstatement.setString(8, entry.getValue().get(LnPConstants.PLOC_ID));
            pstatement.setString(9, entry.getValue().get(LnPConstants.ALOC_ID));
            pstatement.setString(10, entry.getValue().get(LnPConstants.SITE_ID));
            pstatement.addBatch();

            batchLimit--;

            if(batchLimit == 0){
                pstatement.executeBatch();
                pstatement.clearBatch();
                batchLimit = 1000;
            }
            pstatement.clearParameters();
        }

    } catch (SQLException e) {
        getLogger().log(LogLevel.ERROR, e);
    } finally {
        try {
            pstatement.executeBatch();
            db_connection.commit();
            db_connection.setAutoCommit(autoCommit);
        } catch (SQLException e1) {
            getLogger().log(LogLevel.ERROR, e1.getMessage(), e1.fillInStackTrace());
        }

        if (pstatement  != null) {
            try {
                pstatement.close();
                pstatement = null;
            } catch (SQLException e) {
                getLogger().log(LogLevel.ERROR, e.getMessage(), e.fillInStackTrace());
            }
        }
        if (db_connection!= null) {
            try {
                db_connection.close();
                db_connection = null;
            } catch (SQLException e) {
                getLogger().log(LogLevel.ERROR, e.getMessage(), e.fillInStackTrace());
            }
        }
    }
}
AKIWEB
  • 19,008
  • 67
  • 180
  • 294
  • (Make sure transactions are being used correctly. Otherwise the posted code "will not be good", even [re-]using prepared statements. That is, INSERT/UPDATE is "fast". COMMIT is "slow".) –  Aug 23 '12 at 21:59

3 Answers3

9

You can think of using addBatch() and executing a back of statements in one shot. Also, as @pst commented in your question, consider using trasaction.

The way you would do is as follows:

boolean autoCommit = connection.getAutoCommit();
try{
    connection.setAutoCommit(false //Turn off autoCommit
    pstatement = db_connection.prepareStatement(PDSLnPConstants.UPSERT_SQL);

    int batchLimit = 1000;

    try{
        for (Entry<Integer, LinkedHashMap<Integer, String>> entry : MAPPING.entrySet()) {
            pstatement.setInt(1, entry.getKey());
            pstatement.setString(2, entry.getValue().get(LnPConstants.CGUID_ID));
            pstatement.setString(3, entry.getValue().get(LnPConstants.PGUID_ID));
            pstatement.setString(4, entry.getValue().get(LnPConstants.SGUID_ID));
            pstatement.setString(5, entry.getValue().get(LnPConstants.UID_ID));
            pstatement.setString(6, entry.getValue().get(LnPConstants.ULOC_ID));
            pstatement.setString(7, entry.getValue().get(LnPConstants.SLOC_ID));
            pstatement.setString(8, entry.getValue().get(LnPConstants.PLOC_ID));
            pstatement.setString(9, entry.getValue().get(LnPConstants.ALOC_ID));
            pstatement.setString(10, entry.getValue().get(LnPConstants.SITE_ID));
            
            pstatement.addBatch();
            batchLimit--;
            
            if(batchLimit == 0){
                pstatement.executeBatch();
                pstatement.clearBatch();
                batchLimit = 1000;
            }
             pstatement.clearParameters();
        }
    }finally{
        //for the remaining ones
        pstatement.executeBatch();
        
        //commit your updates
        connection.commit();
    }
}finally{
    connection.setAutoCommit(autoCommit);
}

The idea is to set a limit for batch updates and execute a database update only when you reach a particular limit. This way you're limiting a database call to once every batchLimit that you've defined. This way it would be faster.

Also note for the transaction, I've just shown how and when to commit. This might not always be the correct point to commit because this decision would be based on your requirement. You might also want to perform a rollback in case of an exception. So it's upto you to decide.

Have a look at "Using Transaction" tutorial to get a better picture of how to use transaction.

Lluís Suñol
  • 3,466
  • 3
  • 22
  • 33
Sujay
  • 6,753
  • 2
  • 30
  • 49
  • Can you provide me an example basis on my code so that I get clear picture how to use `addBatch()` here? It will be of great help. – AKIWEB Aug 23 '12 at 22:01
  • @Nevzz03: Updated my answer with a sample code. Hope this helps :) – Sujay Aug 23 '12 at 22:03
  • Thanks Sujay for the detailed answer. Appreciated your help. I just updated my question with the full code that I am using currently as suggested by you after making changes. Can yout take a look and let me know if everything looks good to you and I am not doing anything wrong? – AKIWEB Aug 23 '12 at 22:29
  • @Nevzz03: Well in case you encounter a `SQLException` when you're trying to perform a database update, you might consider a `rollback`. Also, I had set the `batchLimit` as 1000 which you can obviously tune based on your requirement. Other than that I think your code looks fine enough to me :) – Sujay Aug 23 '12 at 22:34
  • Oh and you forgot to reset your `autoCommit` to its initial value. Take care of that as well – Sujay Aug 23 '12 at 22:35
  • I did that right? In my finally block? `finally { try { pstatement.executeBatch(); db_connection.commit(); db_connection.setAutoCommit(autoCommit); }` – AKIWEB Aug 23 '12 at 23:01
  • @Nevzz03: sorry, i missed that! if you found this helpful, do consider accepting this as an answer :) – Sujay Aug 23 '12 at 23:07
  • @Nevzz03: you are also missing a `batchLimit--;` statement – Sujay Aug 23 '12 at 23:11
  • what can i do if further executions depend on "ps.getUpdateCount()" ? – wutzebaer Mar 17 '16 at 09:09
1

Your piece of code seems good to me.

Just for code cleanness, I'd put entry.getValue() into a variable (call it value).
And there's no need to call clearParameters().

Last, remember to correctly dispose the prepared statement when you don't need it anymore (close()).

gd1
  • 11,300
  • 7
  • 49
  • 88
  • Thanks gd1 for the suggestions. Why do you think there's no need to call `clearParameters()`? I just wanted to know from the knowledge point of view. – AKIWEB Aug 23 '12 at 22:03
1

Yes, doing batch updates would significantly improve your performance. Just google for it, my preferred answer is this one from Mkyong.com. Else, your code looks ok. "clearParameters()" is not really necessary, it might even consume some processor cycles. Important: if AutoCommit was enabled, don't forget to disable it before, and enable it after doing the updates, this brings again a tremendous improvement.

PS

Above recommendation is based also on my experience. I've just noted that this question was already asked here at Stackoverflow and the answer is very detailed. More on PreparedStatements and batches can be found in the Oracle docs here and about Transactions (AutoCommit) here.

Community
  • 1
  • 1
t0r0X
  • 4,212
  • 1
  • 38
  • 34