[ 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