0

I am using the following code

con.setAutoCommit(false);

for(int i=0;i<requestBody.size();i++)
{
    bulk_Update_Qry = new StringBuffer();

    if (requestBody.getUserDetails().get(i).getFirstName() != null) 
        dbutil.setField(bulk_Update_Qry, "FIRST_NAME",requestBody.getUserDetails().get(i).getFirstName());
    if (requestBody.getUserDetails().get(i).getLastName() != null)
        dbutil.setField(bulk_Update_Qry, "LAST_NAME",requestBody.getUserDetails().get(i).getLastName());
    if (requestBody.getUserDetails().get(i).getPhone() != null)
        dbutil.setField(bulk_Update_Qry, "PHONE",requestBody.getUserDetails().get(i).getPhone() );
    if (requestBody.getUserDetails().get(i).getEmail() != null)
        dbutil.setField(bulk_Update_Qry, "EMAIL",requestBody.getUserDetails().get(i).getEmail());
    if (requestBody.getUserDetails().get(i).getAddress()!= null)
        dbutil.setField(bulk_Update_Qry, "ADDRESS",requestBody.getUserDetails().get(i).getAddress());
    if (requestBody.getUserDetails().get(i).getZip() != null)
        dbutil.setField(bulk_Update_Qry, "ZIP",requestBody.getUserDetails().get(i).getZip() );
    if (requestBody.getUserDetails().get(i).getCity() != null)
        dbutil.setField(bulk_Update_Qry, "CITY",requestBody.getUserDetails().get(i).getCity() );
    if (requestBody.getUserDetails().get(i).getState() != null)
        dbutil.setField(bulk_Update_Qry, "STATE",requestBody.getUserDetails().get(i).getState());
    if (requestBody.getUserDetails().get(i).getCountry() != null)
        dbutil.setField(bulk_Update_Qry, "COUNTRY",requestBody.getUserDetails().get(i).getCountry());

    System.out.println("UPDATE CINR_USER SET " + bulk_Update_Qry + " WHERE ID = \'" + requestBody.getUserDetails().get(i).getId() + "\'");
    ps = con.prepareStatement("UPDATE CINR_USER SET " + bulk_Update_Qry + " WHERE ID = \'" + requestBody.getUserDetails().get(i).getId() + "\'");

    ps.addBatch();
}
ps.executeBatch();
con.commit();

This is a dynamic update query.

setField is a function that I defined to verify if it's present in the request or not.

Problem that I'm facing

If there are 5 update queries, only the 5th query is being executed. I am not sure what's happening to the first four queries.

Also I cannot afford to have

ps = con.prepareStatement(.....)

outside the for loop as I am using a dynamic update query.

Could anyone clarify what I am doing wrong?

Mark Rotteveel
  • 100,966
  • 191
  • 140
  • 197
  • 1
    Because ps is changing within for loop..... – SMA Sep 30 '17 at 13:51
  • The answer lies within you. Or here: https://ericlippert.com/2014/03/05/how-to-debug-small-programs/ –  Sep 30 '17 at 13:52
  • Also, you're completely missing the point of prepared statements, which is to be able to pass parameters, in a safe way, using placeholders in the query, instead of doing string concatenation. Read the tutorial about prepared statements. – JB Nizet Sep 30 '17 at 13:56
  • is there any workaround to this issue ?? – Titus Roby K Sep 30 '17 at 14:05
  • The workaround is to write simpler and correct code that doesn't introduce giant security holes. – Mark Rotteveel Sep 30 '17 at 14:51

2 Answers2

2

In each loop you are creating a new PreparedStatement object, and the batch is per statement object. In other words, every loop you throw away the batch of the previous loop, at the end you have a prepared statement that has a batch containing only one statement, so you execute only the statement defined in the final run of the loop.

There are several ways to solve this:

  1. Use a plain Statement object created outside of the loop.
    This would work because you aren't actually using the primary feature of prepared statements, however this is also unsafe. The way you are currently constructing statements leaves you wide open to SQL injection.
  2. Don't try to use batches and just execute the statement inside the loop.
    But before doing this, you will need to get your act together and correctly parameterize your queries.
  3. Define multiple prepared statements with correctly parameterized queries before the loop, and add a batch to the right statement, execute all those queries after the loop.
Mark Rotteveel
  • 100,966
  • 191
  • 140
  • 197
  • As per the client requirement, requestBody which the client sends – Titus Roby K Sep 30 '17 at 18:37
  • As per the client requirement , request Body with which the client send is JSON Request with 500 occurences that too every elements within each JSON Obj is having optional fields so i have to form a validation that is i have to verify if that particular field is present in the request or not thus forming the Dynamic Update query Since 500 update queries are formed ..i have to do a batch update to increase the performance – Titus Roby K Sep 30 '17 at 18:44
  • @user8180066 I think your client would prefer an application that is secure over performance, and it is probably possible to create a single statement that handles it with parameters in a secure manner. Consider the liability and damage that your company and your client could suffer if someone manages to exploit this. – Mark Rotteveel Oct 01 '17 at 08:49
  • Yeah, you are right Mark, Thanks alot Mark for the help – Titus Roby K Oct 01 '17 at 09:55
1

Each time you call prepareStatement you create a new PreparedStatement object, which apparently has the effect of clearing the batch for the previous instance of the PreparedStatement object. So, when you finally call executeBatch it only contains the single entry for the last iteration of your loop.

Therefore, if you need to execute a different PreparedStatement for each iteration you'll need to call executeUpdate instead of addBatch (and omit the executeBatch call). Since you are (ab)using the PreparedStatement to execute dynamic SQL you could use a simple Statement and call addBatch(sql), but using dynamic SQL is strongly discouraged because it opens your code to SQL Injection vulnerabilities.

Gord Thompson
  • 116,920
  • 32
  • 215
  • 418