Hi Julo,
Thanks for this updates and insight. Please read the in-line comments.
Julius Stroffek wrote:
Hi Saurabh,
I went through the code and here are my observations:
1.) The error related to the main issue is detected correctly in the
function org.apache.derby.client.net.Request.writePlainScalarStream.
It is reported throught the call to netAgent_.accumulateReadException
and more errors are reported this way. Probably none of these errors
are handled in a way that this issue suppose they should be. The
exception reported by the 'repro' code attached to the issue is thrown
by the client code it self. However, the client sends some codepoints
(probably the whole request) which are sent correctly to the server
and executed.
This is correct .
2.) The accumulated exceptions are checked by the following code in
org.apache.derby.client.am.PreparedStatement.flowExecute(int executeType)
if (chainAutoCommit) {
// we have encountered an error in writing the
execute, so do not
// flow an autocommit
if (agent_.accumulatedReadExceptions_ != null) {
// currently, the only write exception we
encounter is for
// data truncation: SQLSTATE 01004, so we
don't bother checking for this
connection_.writeCommitSubstitute_();
commitSubstituted = true;
} else {
// there is no write error, so flow the commit
connection_.writeCommit();
}
}
3.) The writeCommitSubstitute function sends only a EXCSAT codepoint
(exchange server attributes), thus should not affect committing or
roll backing a transaction. I think that the transaction stays
uncommitted after these exceptions. The writeCommit function sends
RDBCMM codepoint (commit).
Correct, at this point the transaction is not committed, this only gets
committed by the next select statement.
4.) I would guess, that the error bit specified in DSS header is
applied only for errors detected by the server not those detected by
the client. Thus in my opinion it makes no sense to change it.
5.) If you look at the page
http://publib.boulder.ibm.com/html/as400/v4r5/ic2924/info/db2/rbafymstposcodes.htm,
this error is described as warning (SQLSTATE 01004). Considering this
and the code in derby, I think current behavior of derby was an
intent. However, how should I make difference between exception thrown
as a warning and an exception thrown as error? Even the code
differentiating exception to errors and warning have to be really
ugly. Even the different behavior of embedded driver a derby network
client is not a good idea.
6.) I can see only one solution to this error - calling a
writeRollback method instead of writeCommitSubstitute in the code
above and handle the case when the auto commit is turned off in a
similar way. This changes a behavior of derby a bit - every error will
rollback the current transaction. Is it possible to do this? Any other
suggestions?
I tried this approach and found the repro works absolutely fine.
Following are the changes I had made :
org.apache.derby.client.am.PreparedStatement.flowExecute(int executeType)
------------ code snippet-----------------
if (chainAutoCommit) {
// we have encountered an error in writing the
execute, so do not
// flow an autocommit
if (agent_.accumulatedReadExceptions_ != null) {
// currently, the only write exception we
encounter is for
// data truncation: SQLSTATE 01004, so we don't
bother checking for this
*//connection_.writeCommitSubstitute_();
connection_.writeLocalRollback_(); // applied
RollBack Approach
//commitSubstituted = true;*
} else {
// there is no write error, so flow the commit
connection_.writeCommit();
}
}
Well the writeLocalRollback_() method calls buildRDBRLLBCK() and sends
RDBRLLBCK code-point. I am not sure that this approach can be used for
all the cases.
Or do we need to implement writeRollback() as a new method to handle
this particular case (accumulatedReadException). Comments/Suggestions
please.
Thanks,
Saurabh
Cheers
Julo
Saurabh Vyas wrote:
Hi All,
I was looking into Derby-2017
(https://issues.apache.org/jira/browse/DERBY-2017) the following were
my findings :
- when error occurs, then a dummyDSS is built and the partial data
(which is *'this' *in the repro ) is sent across to NetworkServer.
Now when the next select statement is executed, it commits the
transaction and the partial data gets committed and select returns
the incorrect committed data.
- I modified the repro to rollback the transaction when error occurs,
then it behaves correctly.
code snippet....................
StringReader reader = new StringReader("this string is way
too long");
ps.setCharacterStream(1, reader, 5);
try {
ps.executeUpdate();
} catch (SQLException e) {
*c.rollback(); //see here*
System.out.println(e);
}
---------------------------------------------------
- DRDA Header specifications provides a flag which can notify that
error had occurred & do not continue.
/**
* Read DSS header
* DSS Header format is
* 2 bytes - length
* 1 byte - 'D0' - indicates DDM data
* 1 byte - DSS format
* |---|---------|----------|
* | 0 | flags | type |
* |---|---------|----------|
* | 0 | 1 2 3 | 4 5 6 7 |
* |---|---------|----------|
* bit 0 - '0'
* bit 1 - '0' - unchained, '1' - chained
* *bit 2 - '0' - do not continue on error, '1' -
continue on error //See Here*
* bit 3 - '0' - next DSS has different correlator, '1' -
next DSS has
* same correlator
..........
*/
- What I observed is, when error occurs (i.e. accumulateReadError)
and writeCommitSubstitute is called where a dummy DSS is built
(buildDummyEXCSAT()).
- buildDss() method write the header information and here we should
set the CONTINUE_ON_ERROR flag to '0' thus the data shold not be send
to NetworkServer.
Is my understanding correct? I am new to this part of derby and not
familiar with the DRDA implementation.
Comments/Suggestions please.
I tried to follow Army's work on DERBY-35, but that is much target
for chaining and I am not aware about Tomohito 's work in Layer B
streaming. Can anyone help me out in this.
Thanks,
Saurabh
