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.ander...@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 > * <code>CachedRowSet</code> object into the data source from which > * the rowset is derived, returning <code>false</code> if the insertion > * was successful. > * > * @param crs the <code>CachedRowSet</code> object that has had a > row inserted > * and to whose underlying data source the row will be inserted > * @param pstmt the <code>PreparedStatement</code> object that will be used > * to execute the insertion > * @return <code>false</code> to indicate that the insertion was > successful; > * <code>true</code> 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.TYPE_SCROLL_SENSITIVE, > ResultSet.CONCUR_READ_ONLY); > 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("COLUMN_NAME"); > 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.ander...@oracle.com
