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

Lance Andersen - Oracle Lance.Andersen at oracle.com
Thu May 31 22:16:16 UTC 2012

Here is the revision using the try with resources as David suggested

http://cr.openjdk.java.net/~lancea/7145913/webrev.01/

Best Lance On May 31, 2012, at 3:10 PM, David Schlosnagle wrote:

On Thu, May 31, 2012 at 1:50 PM, Lance Andersen - Oracle <Lance.Andersen at oracle.com> wrote:

Hi,

Looking for a reviewer for 7145913. This addresses an issue with where a SQLException could be thrown when writing a row back with an auto-incremented column on some databases. Webrev is http://cr.openjdk.java.net/~lancea/7145913/webrev.00/. RowSet TCK, sqe tests and unit tests all pass with this fix. Hi Lance, I had a few quick comments: * Do you need the multiple calls to CachedResultSet.getObject for the primary key, or would it be worthwhile to store the result in a local variable? * This seems to be pre-existing before your changes, but the PreparedStatement pstmtSel and ResultSet rs, rs2 used within the insertNewRow method are never closed, which could lead to resource leaks. * Minor nit, the indentation in this file seems off, especially for the method JavaDoc. I've included a minor cleanup below of the insertNewRow method using try-with-resources. Thanks, Dave /** * Inserts a row that has been inserted into the given * CachedRowSet object into the data source from which * the rowset is derived, returning false if the insertion * was successful. * * @param crs the CachedRowSet object that has had a row inserted * and to whose underlying data source the row will be inserted * @param pstmt the PreparedStatement object that will be used * to execute the insertion * @return false to indicate that the insertion was successful; * true otherwise * @throws SQLException if a database access error occurs */ private boolean insertNewRow(CachedRowSet crs, PreparedStatement pstmt, CachedRowSetImpl crsRes) throws SQLException { boolean returnVal = false; try (PreparedStatement pstmtSel = con.prepareStatement(selectCmd, ResultSet.TYPESCROLLSENSITIVE, ResultSet.CONCURREADONLY); ResultSet rs = pstmtSel.executeQuery(); ResultSet rs2 = con.getMetaData().getPrimaryKeys(null, null, crs.getTableName()) ) { ResultSetMetaData rsmd = crs.getMetaData(); int icolCount = rsmd.getColumnCount(); String[] primaryKeys = new String[icolCount]; int k = 0; while (rs2.next()) { primaryKeys[k] = rs2.getString("COLUMNNAME"); k++; } if (rs.next()) { for (String pkName : primaryKeys) { if (!isPKNameValid(pkName, rsmd)) { /* We came here as one of the the primary keys * of the table is not present in the cached * rowset object, it should be an autoincrement column * and not included while creating CachedRowSet * Object, proceed to check for other primary keys */ continue; } Object crsPK = crs.getObject(pkName); if (crsPK == null) { /* * It is possible that the PK is null on some databases * and will be filled in at insert time (MySQL for example) */ break; } String rsPK = rs.getObject(pkName).toString(); if (crsPK.toString().equals(rsPK)) { returnVal = true; this.crsResolve.moveToInsertRow(); for (int i = 1; i <= icolCount; i++) { String colname = (rs.getMetaData()).getColumnName(i); if (colname.equals(pkName)) this.crsResolve.updateObject(i,rsPK); else this.crsResolve.updateNull(i); } this.crsResolve.insertRow(); this.crsResolve.moveToCurrentRow(); } } } if (returnVal) { return returnVal; } try { int i; for (i = 1; i <= icolCount; i++) { Object obj = crs.getObject(i); if (obj != null) { pstmt.setObject(i, obj); } else { pstmt.setNull(i,crs.getMetaData().getColumnType(i)); } } i = pstmt.executeUpdate(); return false; } catch (SQLException ex) { /* * Cursor will come here if executeUpdate fails. * There can be many reasons why the insertion failed, * one can be violation of primary key. * Hence we cannot exactly identify why the insertion failed * Present the current row as a null row to the user. **/ this.crsResolve.moveToInsertRow(); for (int i = 1; i <= icolCount; i++) { this.crsResolve.updateNull(i); } this.crsResolve.insertRow(); this.crsResolve.moveToCurrentRow(); return true; } } }

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