Review request for 7145913 CachedRowSetSwriter.insertNewRow() throws SQLException

[Lance Andersen - Oracle]

Thanks for the comments
On Jun 13, 2012, at 6:03 PM, Ulf Zibis wrote:

> 
> Am 13.06.2012 13:04, schrieb Lance Andersen - Oracle:
>> Hi Paul,
>> 
>> Thank you for taking  the time to review the code.
>> 
>> 
>> I made the change you suggested below
>> 
>> http://cr.openjdk.java.net/~lancea/7145913/webrev.02
> Typos:
> 912                 * Hence we cannot exactly identify why the insertion failed
> 913                 * Present the current row as a null row to the user.
> Please add a ',', use lowercase 'p' and maybe rename user to caller
> 912                 * Hence we cannot exactly identify why the insertion failed,
> 913                 * present the current row as a null row to the caller.
> 

I made this change
> In isPKNameValid(...) you could reuse icolCount from line 843.
> 
> Another additional thought:
> Isn't it likely, that the internal imlementation of ResultSetMetaData methods decrements the column index like following:
>    public String getColumnClassName(int column) {
>        return columns[column - 1].className;
> In this case, iterating over the range of 0..cols-1 could be a small performance gain after JIT optimization:
> 1469         for(int i = 0; i< cols; i++) {
> 1470             String colName = rsmd.getColumnClassName(i + 1);
> If not, the offset could be anyway treated by single opcode using complex addressing.
> 
> Same for iterating over ResultSet collumns.

I will review this for future updates, but for now, I did not make this change

Best
Lance
> 
> -Ulf
> 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
Lance.Andersen at oracle.com