[ http://issues.apache.org/jira/browse/DERBY-1313?page=all ]
Fernanda Pizzorno updated DERBY-1313:
-------------------------------------
Attachment: derby-1313v2.diff
derby-1313v2.stat
The patch derby-1313v2.diff addreses the review comments.
[...]
> * NetCursor.java
> - Several readFdoca* methods are very similar, and you have
> introduced yet another. Use of some minion code would be in order
> to remove redundancy here, I think, but maybe as part of another
> patch.
I don't think this should be done as a part of this patch
> - I also note that the parsing methods in general in NetCursor to a
> large degree duplicate similar methods in NetConnectionReply;
> candidate for refactoring, too. Another time...!
I agree that some refactoring could be done here, but I don't think
it should be done as a part of this issue.
> - It is OK that not all protocol elements have parsing code added,
> since the server currently doesn't send them and you *do* throw an
> exception if they are encountered (e.g. NetCursor#parseSQLDIAGCN).
> Maybe a list somewhere (like an overview) of which elements are only
> partially implemented would be nice. But this is probably more a
> general peeve for this code. Your call (itch?) ;)
The partially implemented elements introduced by this patch are:
- SQLDIAGSTT
- SQLDIAGCN
- SQLDCTOKS
- SQLDCXGRP
> * DRDAConnThread.java
> - spurious blank line introduced: 63
The blank line has been removed.
> - wrong comment:
>
> // DRDA diagnostic level -> DIAGLVL - SQL Error diagnistic level
> // DIAGLVL1 (0xF0) - Null SQLDIAGRP (DEFAULT)
> // DIAGLVL1 (0xF1) - Non-null SQLDIAGRP
> // DIAGLVL1 (0xF2) - Non-null SQLDIAGRP, and null message text fields
> private byte diagnosticLevel = (byte)0xF0;
>
> They are all called DIAGLVL1 ;) Maybe point to CodePoint.java
> instead to avoid redudancy?
This comment has been changed to:
// DRDA diagnostic level, DIAGLVL0 by default
> * SURTest.java
> - Is the detectability methods tested in connection with problems seen
> in DERBY-1249, for the client case? Or should
> testRowUpdateAndRowDeleted have some tests for the conflict case as
> well?
Detectability methods are tested in connection with cursor operation
conflict. These tests were added as a part of the work done for
DERBY-1251.
> Finally, you did not mention which tests have been run, if any?
I have successfully run derbyall.
> SUR: Use DRDA's extended diagnostic to send ROW_UPDATED and ROW_DELETED
> warnings.
> ---------------------------------------------------------------------------------
>
> Key: DERBY-1313
> URL: http://issues.apache.org/jira/browse/DERBY-1313
> Project: Derby
> Type: Bug
> Components: JDBC
> Reporter: Fernanda Pizzorno
> Assignee: Fernanda Pizzorno
> Attachments: derby-1313.diff, derby-1313.stat, derby-1313v2.diff,
> derby-1313v2.stat
>
> Detectability of own changes is implemented in the client using warnings cf
> the write-up for DERBY-775. When a row has been deleted and/or updated, a
> warning will be sent to the client to indicate that fact. Presently, only one
> warning can be sent each time a data row is sent from to the client, that
> means that some warnings may be lost. Using extended diagnostic allows us to
> send several warnings for each data row.
> I propose to use extended diagnostics to send ROW_UPDATED and ROW_DELETED
> warnings when necessary. This may later be extended for other warnings, but I
> do not plan to do it as a part of the work in this issue.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
