Review request for 7145913 CachedRowSetSwriter.insertNewRow() throws SQLException (original) (raw)

Lance Andersen - Oracle Lance.Andersen at oracle.com
Thu Jun 14 19:10:30 UTC 2012

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

More information about the core-libs-dev mailing list