[ 
http://issues.apache.org/jira/browse/DERBY-1313?page=comments#action_12413130 ] 

Dag H. Wanvik commented on DERBY-1313:
--------------------------------------

Patch looks clean and good to me. Quite some work digging through
thoses specs and getting the details right! 

It needs to be updated, though, I had to svn up to an older point in
time (I used May 17) to be able to apply the patch cleanly.

* 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 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...!

- 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?) ;)

* DRDAConnThread.java
- spurious blank line introduced: 63

- 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?

* 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?

Finally, you did not mention which tests have been run, if any?


> 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
>
> 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

Reply via email to