[ http://issues.apache.org/jira/browse/DERBY-775?page=comments#action_12373360 ]
Øystein Grøvlen commented on DERBY-775: --------------------------------------- So far I have reviewed all code except test changes. The patch looks good. I have a few comments/questions: 1. metadata_net.properties: - Is the format and constant values used here documented anywhere? 2. DRDAConnThread - parseSQLATTR(): Variable insensitive can be removed entirely. It serves no purpose anymore. 3. NetResultSet/NetResultSet40 - Parameters of constructor has comments with possible values. This needs to be updated to include QRYSNSSTC. 4. NetCursor - The writeup says the purpose is to pop warnings, but the code seems to only handle a single warning. - Why do you have to delay the call to setIsUpdataDeleteHole until later and not do it when testing for warnings? 5. NetStatementReply - Call to ClientJDBCObjectFactory.newNetResultSet() lists possible values. This needs to be updated to include QRYSNSSTC. 6. Statement - Why have you made warnings_ protected? It seems like it is package private that is needed. Why not keep it private and supply a get-method? 7. ResultSet - Why have you made suggestedFetchSize_ public? It seems like it could be protected. - Comments for fetchSize_ and suggestedFetchSize_ are a bit brief. (And if they are to be public, you should supply javadoc). - relativex(): Why do not the isBeforeFirstX()/isAfterLastX() test for all result sets? - relativex(): It seems like the code to call getAbsoluteResultSet is duplicated. I do not think the second call will ever be executed. - rowUpdated(): Why not always call getIsRowUpdated? I would think it was valid for all result sets. - rowUpdated()/rowDeleted(): Thss methods are a bit asymmetric. rowUpdated() calls a get-method while rowDeleted accesses isUpdateDeleteHole_ directly. - updateRow(): I tried to browse the spec and the javadoc and I did not find anything that said that there is a difference between forward only cursors and scrollable cursors with respect to validity of the current position after and update. Where is this stated? - checkForUpdatableResultSet(): What is the difference between this new method and the existing checkUpdatableCursor()? It seems better to change the existing than to make a new method. > Network client: Add support for scrollable, updatable, insensitive result sets > ------------------------------------------------------------------------------ > > Key: DERBY-775 > URL: http://issues.apache.org/jira/browse/DERBY-775 > Project: Derby > Type: New Feature > Components: Network Client, JDBC > Versions: 10.2.0.0 > Reporter: Dag H. Wanvik > Assignee: Dag H. Wanvik > Priority: Minor > Attachments: 775-writeup.txt, derby-775-1.diff, derby-775-1.stat, > derby-775-2.diff, derby-775-2.stat > > This is a part of the DERBY-690 effort. -- 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
