OPENJPA-1550 When batchLimit=-1 or >1 and an exception is caused, the params and failedObject are missing from the resultant exception. Original patch contributed by Heath Thomann.

git-svn-id: https://svn.apache.org/repos/asf/openjpa/trunk@921174 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Donald Woods 2010-03-09 22:29:18 +00:00
parent 8c8cd98a65
commit 695bd9c4bf
3 changed files with 127 additions and 34 deletions

View File

@ -33,6 +33,7 @@ import org.apache.openjpa.jdbc.sql.Row;
import org.apache.openjpa.jdbc.sql.RowImpl;
import org.apache.openjpa.jdbc.sql.SQLExceptions;
import org.apache.openjpa.kernel.OpenJPAStateManager;
import org.apache.openjpa.lib.jdbc.ReportingSQLException;
import org.apache.openjpa.lib.log.Log;
import org.apache.openjpa.lib.util.Localizer;
import org.apache.openjpa.util.OptimisticException;
@ -187,13 +188,44 @@ public class BatchingPreparedStatementManagerImpl extends
checkUpdateCount(rtn, batchedRowsBaseIndex, ps);
}
} catch (SQLException se) {
SQLException sqex = se.getNextException();
if (sqex == null)
sqex = se;
throw SQLExceptions.getStore(sqex, ps, _dict);
//If we look at PreparedStatementManagerImpl.flushAndUpdate (which is the 'non-batch' code path
//similar to this path, or I should say, the path which is taken instead of this path when
//we aren't using batching), we see that the catch block doesn't do a 'se.getNextException'.
//When we do a 'getNextException', the 'next exception' doesn't contain the same message as se.
//That is, 'next exception' contains a subset msg which is contained in se. For legacy, should
//we continute to use 'sqex' in the 'old path' and use 'se' in the next path/code?????
// SQLException sqex = se.getNextException();
// if (sqex == null)
// sqex = se;
SQLException sqex = se;
if (se instanceof ReportingSQLException){
int index = ((ReportingSQLException) se).getIndexOfFirstFailedObject();
//if we have only batched one statement, the index should be 0. As can be seen above,
//if 'batchSize == 1' a different path is taken (the 'single row' path), and if that row
//fails, we know that the index is 0 since there is only one row.
if (batchSize == 1){
index = 0;
}
//index should not be less than 0 this path, but if for some reason it is, lets
//resort to the 'old way' and simply pass the 'ps' as the failed object.
if (index < 0){
throw SQLExceptions.getStore(sqex, ps, _dict);
}
else{
throw SQLExceptions.getStore(sqex, ((RowImpl)(_batchedRows.get(index))).getFailedObject(), _dict);
}
}
else{
throw SQLExceptions.getStore(sqex, ps, _dict);
}
} finally {
_batchedSql = null;
batchedRows.clear();
//Clear the Params now....should this be done above?
ps.clearParameters();
if (ps != null) {
try {
ps.close();

View File

@ -85,7 +85,7 @@ public class LoggingConnectionDecorator implements ConnectionDecorator {
private static final int WARN_THROW = 5;
private static final int WARN_HANDLE = 6;
private static final String[] WARNING_ACTIONS = new String[7];
static {
WARNING_ACTIONS[WARN_IGNORE] = "ignore";
WARNING_ACTIONS[WARN_LOG_TRACE] = "trace";
@ -229,29 +229,36 @@ public class LoggingConnectionDecorator implements ConnectionDecorator {
return ConcreteClassGenerator.newInstance(loggingConnectionImpl, LoggingConnectionDecorator.this, conn);
}
/**
* Include SQL in exception.
*/
private SQLException wrap(SQLException sqle, Statement stmnt) {
if (sqle instanceof ReportingSQLException)
return (ReportingSQLException) sqle;
return new ReportingSQLException(sqle, stmnt, null);
return wrap(sqle, stmnt, null, -1);
}
/**
* Include SQL in exception.
*/
private SQLException wrap(SQLException sqle, String sql) {
if (sqle instanceof ReportingSQLException)
return (ReportingSQLException) sqle;
return new ReportingSQLException(sqle, null, sql);
return wrap(sqle, null, sql, -1);
}
private SQLException wrap(SQLException sqle, Statement stmnt, String sql) {
if (sqle instanceof ReportingSQLException)
return (ReportingSQLException) sqle;
return new ReportingSQLException(sqle, stmnt, sql);
return wrap(sqle, stmnt, sql, -1);
}
private SQLException wrap(SQLException sqle, Statement stmnt, int indexOfFailedBatchObject) {
return wrap(sqle, stmnt, null, -1);
}
/**
* Include SQL in exception.
*/
private SQLException wrap(SQLException sqle, Statement stmnt, String sql, int indexOfFailedBatchObject) {
ReportingSQLException toReturn = null;
if (sqle instanceof ReportingSQLException) {
toReturn = (ReportingSQLException) sqle;
} else {
toReturn = new ReportingSQLException(sqle, stmnt, sql);
}
toReturn.setIndexOfFirstFailedObject(indexOfFailedBatchObject);
return toReturn;
}
/**
@ -972,6 +979,9 @@ public class LoggingConnectionDecorator implements ConnectionDecorator {
private final String _sql;
private List<String> _params = null;
private List<List<String>> _paramBatch = null;
// When batching is used, this variable contains the index into the
// last successfully executed batched statement.
int batchedRowsBaseIndex = 0;
public LoggingPreparedStatement(PreparedStatement stmnt, String sql)
throws SQLException {
@ -1076,11 +1086,29 @@ public class LoggingConnectionDecorator implements ConnectionDecorator {
}
public int[] executeBatch() throws SQLException {
int indexOfFirstFailedObject = -1;
logBatchSQL(this);
long start = System.currentTimeMillis();
SQLException err = null;
try {
return super.executeBatch();
int[] toReturn = super.executeBatch();
//executeBatch is called any time the number of batched statements
//is equal to, or less than, batchLimit. In the 'catch' block below,
//the logic seeks to find an index based on the current executeBatch
//results. This is fine when executeBatch is only called once, but
//if executeBatch is called many times, the _paramsBatch will continue
//to grow, as such, to index into _paramsBatch, we need to take into
//account the number of times executeBatch is called in or der to
//correctly index into _paramsBatch. To that end, each time executeBatch
//is called, lets get the size of _paramBatch. This will effectively
//tell us the index of the last successfully executed batch statement.
//If an exception is caused, then we know that _paramBatch.size was
//the index of the LAST row to successfully execute.
if (_paramBatch != null){
batchedRowsBaseIndex = _paramBatch.size();
}
return toReturn;
} catch (SQLException se) {
// if the exception is a BatchUpdateException, and
// we are tracking parameters, then set the current
@ -1093,12 +1121,11 @@ public class LoggingConnectionDecorator implements ConnectionDecorator {
getUpdateCounts();
if (count != null && count.length <= _paramBatch.size())
{
int index = -1;
for (int i = 0; i < count.length; i++) {
// -3 is Statement.STATEMENT_FAILED, but is
// only available in JDK 1.4+
if (count[i] == Statement.EXECUTE_FAILED) {
index = i;
indexOfFirstFailedObject = i;
break;
}
}
@ -1106,15 +1133,31 @@ public class LoggingConnectionDecorator implements ConnectionDecorator {
// no -3 element: it may be that the server stopped
// processing, so the size of the count will be
// the index
if (index == -1)
index = count.length + 1;
//See the Javadoc for 'getUpdateCounts'; a provider
//may stop processing when the first failure occurs,
//as such, it may only return 'UpdateCounts' for the
//first few which pass. As such, the failed
//index is 'count.length', NOT count.length+1. That
//is, if the provider ONLY returns the first few that
//passes (i.e. say an array of [1,1] is returned) then
//length is 2, and since _paramBatch starts at 0, we
//don't want to use length+1 as that will give us the
//wrong index.
if (indexOfFirstFailedObject == -1){
indexOfFirstFailedObject = count.length;
}
//Finally, whatever the index is at this point, add batchedRowsBaseIndex
//to it to get the final index. Recall, we need to start our index from the
//last batch which successfully executed.
indexOfFirstFailedObject += batchedRowsBaseIndex;
// set the current params to the saved values
if (index < _paramBatch.size())
_params = (List<String>) _paramBatch.get(index);
if (indexOfFirstFailedObject < _paramBatch.size())
_params = (List) _paramBatch.get(indexOfFirstFailedObject);
}
}
err = wrap(se, LoggingPreparedStatement.this);
err = wrap(se, LoggingPreparedStatement.this, indexOfFirstFailedObject);
throw err;
} finally {
logTime(start);
@ -1366,10 +1409,17 @@ public class LoggingConnectionDecorator implements ConnectionDecorator {
}
private void clearLogParameters(boolean batch) {
if (_params != null)
_params.clear();
if (batch && _paramBatch != null)
_paramBatch.clear();
//Made !batch...we only want to clear if
//we are NOT using batching. If we clear now,
//the _params will not be displayed in the resultant
//exception message. But when should we 'clear' them???
if (!batch){
if (_params != null)
_params.clear();
if (_paramBatch != null)
_paramBatch.clear();
}
}
private boolean shouldTrackParameters() {

View File

@ -34,7 +34,10 @@ public class ReportingSQLException extends SQLException {
private final transient Statement _stmnt;
private final SQLException _sqle;
private final String _sql;
// When batching is used, and an object/row in the batch causes an
// exception, this variable will hold the index of the first failing object.
private int indexOfFirstFailedObject=-1;
/**
* Supply original exception and non-null Statement and/or SQL string.
*/
@ -74,7 +77,15 @@ public class ReportingSQLException extends SQLException {
public Statement getStatement() {
return _stmnt;
}
public int getIndexOfFirstFailedObject(){
return indexOfFirstFailedObject;
}
public void setIndexOfFirstFailedObject(int index){
indexOfFirstFailedObject=index;
}
private static String getExceptionMessage(SQLException sqle,
Statement stmnt, String sql) {
try {