Review request for 7145913 CachedRowSetSwriter.insertNewRow() throws SQLException

[David Schlosnagle]

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
     * <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;
            }
        }
    }