Author: dwoods
Date: Tue Mar 9 22:29:18 2010
New Revision: 921174
URL: http://svn.apache.org/viewvc?rev=921174&view=rev
Log:
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.
Modified:
openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/BatchingPreparedStatementManagerImpl.java
openjpa/trunk/openjpa-lib/src/main/java/org/apache/openjpa/lib/jdbc/LoggingConnectionDecorator.java
openjpa/trunk/openjpa-lib/src/main/java/org/apache/openjpa/lib/jdbc/ReportingSQLException.java
Modified:
openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/BatchingPreparedStatementManagerImpl.java
URL:
http://svn.apache.org/viewvc/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/BatchingPreparedStatementManagerImpl.java?rev=921174&r1=921173&r2=921174&view=diff
==============================================================================
---
openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/BatchingPreparedStatementManagerImpl.java
(original)
+++
openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/BatchingPreparedStatementManagerImpl.java
Tue Mar 9 22:29:18 2010
@@ -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 BatchingPreparedStatementMa
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();
Modified:
openjpa/trunk/openjpa-lib/src/main/java/org/apache/openjpa/lib/jdbc/LoggingConnectionDecorator.java
URL:
http://svn.apache.org/viewvc/openjpa/trunk/openjpa-lib/src/main/java/org/apache/openjpa/lib/jdbc/LoggingConnectionDecorator.java?rev=921174&r1=921173&r2=921174&view=diff
==============================================================================
---
openjpa/trunk/openjpa-lib/src/main/java/org/apache/openjpa/lib/jdbc/LoggingConnectionDecorator.java
(original)
+++
openjpa/trunk/openjpa-lib/src/main/java/org/apache/openjpa/lib/jdbc/LoggingConnectionDecorator.java
Tue Mar 9 22:29:18 2010
@@ -85,7 +85,7 @@ public class LoggingConnectionDecorator
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
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
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
}
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
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
// 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
}
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() {
Modified:
openjpa/trunk/openjpa-lib/src/main/java/org/apache/openjpa/lib/jdbc/ReportingSQLException.java
URL:
http://svn.apache.org/viewvc/openjpa/trunk/openjpa-lib/src/main/java/org/apache/openjpa/lib/jdbc/ReportingSQLException.java?rev=921174&r1=921173&r2=921174&view=diff
==============================================================================
---
openjpa/trunk/openjpa-lib/src/main/java/org/apache/openjpa/lib/jdbc/ReportingSQLException.java
(original)
+++
openjpa/trunk/openjpa-lib/src/main/java/org/apache/openjpa/lib/jdbc/ReportingSQLException.java
Tue Mar 9 22:29:18 2010
@@ -34,7 +34,10 @@ public class ReportingSQLException exten
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 exten
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 {
