Fix updateRow when there are primary keys and unique keys

[davecramer]

PR #2199 made some incorrect assumptions about unique keys that were incorrect.

When we have both primary keys and unique keys only use the Primary keys for updating the row.

[chalmagr]

I think this still has issues if the table has more than one unique key - it will require both unique keys instead of using the one that actually exists in the resultset. I've made the following change instead to the PgResultSet method isUpdateable

  boolean isUpdateable() throws SQLException {
    ...
    String lastConstraintName = null;
    boolean constraintFound = false;

    while (rs.next()) {
      String constraintName = castNonNull(rs.getString(6)); // get the constraintName
      if (lastConstraintName == null || !lastConstraintName.equals(constraintName)) {
        if (lastConstraintName != null) {
          if (i == numPKcolumns && numPKcolumns > 0) {
            break;
          }
          connection.getLogger().log(Level.FINE, "no of keys={0} from constraint {1}", new Object[]{i, lastConstraintName});
        }
        i = 0;
        numPKcolumns = 0;

        primaryKeys.clear();
        lastConstraintName = constraintName;
      }
      numPKcolumns++;

      String columnName = castNonNull(rs.getString(4)); // get the columnName
      int index = findColumnIndex(columnName);

      /* make sure that the user has included the primary key in the resultset */
      if (index > 0) {
        i++;
        primaryKeys.add(new PrimaryKey(index, columnName)); // get the primary key information
      }
    }

    rs.close();
    connection.getLogger().log(Level.FINE, "no of keys={0} from constraint {1}", new Object[]{i, lastConstraintName});
    ...

Also, after doing this change the mehod public synchronized void updateRow() throws SQLException; has to be changed to ensure the 'WHERE' does not do = but does IS NULL instead (below shows my patch to that method) - this would need to be checked on the other update methods as well... one option is to have this where condition creation become part of the 'PrimaryKey' class now that the isPrimary is defined at that level, reducing the duplications needed to generate this differentiation in the code

  public synchronized void updateRow() throws SQLException {
    ...
    for (int i = 0; i < numKeys; i++) {
      PrimaryKey primaryKey = primaryKeys.get(i);
      Utils.escapeIdentifier(updateSQL, primaryKey.name);
      updateSQL.append(primaryKey.getValue() != null ? " = ?" : " IS NULL");

      if (i < numKeys - 1) {
        updateSQL.append(" and ");
      }
    }
    ...
      for (int j = 0; j < numKeys; j++ ) {
        Object keyValue = primaryKeys.get(j).getValue();
        if (keyValue != null) {
          updateStatement.setObject(++i, keyValue);
        }
      }
    ...

[davecramer]

ah, good point, thx

[chalmagr]

I have made changes in a forked repo for me to use in the meantime. Changes are visible here - since there are other changes (boolean indicator for primary key) I wouldn't create a PR for it as changes may be conflicting

[davecramer]

@chalmagr can you confirm this works for you ?

[chalmagr]

@davecramer based on the changes I see, this is still not going to work since we have some cases were the table has PK as well as UK, but only the UK fields are part of the ResultSet and the logic seems to be removing all non-primary key elements if that is the case, making the resultset non-updateable.

Also, in the event that a table has multiple unique keys (don't think we have one in our case) the logic is combining them and will require all of them to be present in the resultset for it to be updateable instead of having just one.

I am not seeing the logic changes to have proper filters on the nullable fields of the UK as mentioned in the patch before... this will definitely not work since the update will not have proper filters and will fail to perform updates

Let me see if I can update my forked copy to apply some of your changes and I can create the PR with both? I have confirmed for some time now that my changes are working as expected (ongoing testing since my changes were made)

[davecramer]

@chalmagr thanks, that would be great