Fix updateable result set when there are primary keys and unique keys (…

…#2228)

* fix: change to updatable result set to use correctly primary or unique keys

Fix issues introduced in PR# 2199 causing problems when updating tables with PK and UK.
Originally updatable result sets support only primary keys, with the aforementioned change
and this fix unique keys are also eligible to enable updatable result set. This change will
find the better suited primary or unique key that has all the available columns in the query, but
if no match is found it will continue to fail as it used to.

Closes issue 2196

* fix: changes to updatable result set to disable use of unique keys with nullable columns

Adding logic to ensure all columns in the constraint (primary/unique) are non-nullable (always true for PKs) to guarantee 1-1 updates

* fix: removal of added code after nullable unique keys are excluded

No need to have IS NULL when nullable keys are not allowed to guarantee 1-1 updates

* fix: exclude indices with expressions for updatable result sets

Indices with expressions cannot be used (unless expression parsing / resolution
is implemented properly - currently don't see a way to get individual list elements
from pg_index.indexprs to enable proper expression filtering (pg_get_expr gives the
expression, but unable to map this to a specific attribute in the index)

Co-authored-by: Marquez Grabia, Christian <Christian.MarquezGrabia@sabre.com>

pgjdbc/src/main/java/org/postgresql/jdbc/PgDatabaseMetaData.java

@@ -2182,13 +2182,22 @@ protected ResultSet getPrimaryUniqueKeys(@Nullable String catalog, @Nullable Str
     sql = "SELECT NULL AS TABLE_CAT, n.nspname AS TABLE_SCHEM, "
         + "  ct.relname AS TABLE_NAME, a.attname AS COLUMN_NAME, "
         + "  (information_schema._pg_expandarray(i.indkey)).n AS KEY_SEQ, ci.relname AS PK_NAME, "
-        + "  information_schema._pg_expandarray(i.indkey) AS KEYS, a.attnum AS A_ATTNUM "
+        + "  information_schema._pg_expandarray(i.indkey) AS KEYS, a.attnum AS A_ATTNUM, "
+        + "  a.attnotnull AS IS_NOT_NULL "
         + "FROM pg_catalog.pg_class ct "
         + "  JOIN pg_catalog.pg_attribute a ON (ct.oid = a.attrelid) "
         + "  JOIN pg_catalog.pg_namespace n ON (ct.relnamespace = n.oid) "
         + "  JOIN pg_catalog.pg_index i ON ( a.attrelid = i.indrelid) "
         + "  JOIN pg_catalog.pg_class ci ON (ci.oid = i.indexrelid) "
-        + "WHERE true ";
+        // primary as well as unique keys can be used to uniquely identify a row to update
+        + "WHERE (i.indisprimary OR ( "
+        + "    i.indisunique "
+        + "    AND i.indisvalid "
+        // partial indexes are not allowed - indpred will not be null if this is a partial index
+        + "    AND i.indpred IS NULL "
+        // indexes with expressions are not allowed
+        + "    AND i.indexprs IS NULL "
+        + "  )) ";
 
     if (schema != null && !schema.isEmpty()) {
       sql += " AND n.nspname = " + escapeQuotes(schema);
@@ -2198,14 +2207,14 @@ protected ResultSet getPrimaryUniqueKeys(@Nullable String catalog, @Nullable Str
       sql += " AND ct.relname = " + escapeQuotes(table);
     }
 
-    sql += " AND (i.indisprimary or i.indisunique) ";
     sql = "SELECT "
         + "       result.TABLE_CAT, "
         + "       result.TABLE_SCHEM, "
         + "       result.TABLE_NAME, "
         + "       result.COLUMN_NAME, "
         + "       result.KEY_SEQ, "
-        + "       result.PK_NAME "
+        + "       result.PK_NAME, "
+        + "       result.IS_NOT_NULL "
         + "FROM "
         + "     (" + sql + " ) result"
         + " where "

pgjdbc/src/main/java/org/postgresql/jdbc/PgResultSet.java

@@ -1333,7 +1333,7 @@ public void refreshRow() throws SQLException {
     for (int i = 0; i < numKeys; i++) {
 
       PrimaryKey primaryKey = primaryKeys.get(i);
-      selectSQL.append(primaryKey.name).append("= ?");
+      selectSQL.append(primaryKey.name).append(" = ?");
 
       if (i < numKeys - 1) {
         selectSQL.append(" and ");
@@ -1350,8 +1350,8 @@ public void refreshRow() throws SQLException {
       selectStatement = connection.prepareStatement(sqlText,
           ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_UPDATABLE);
 
-      for (int j = 0, i = 1; j < numKeys; j++, i++) {
-        selectStatement.setObject(i, primaryKeys.get(j).getValue());
+      for (int i = 0; i < numKeys; i++) {
+        selectStatement.setObject(i + 1, primaryKeys.get(i).getValue());
       }
 
       PgResultSet rs = (PgResultSet) selectStatement.executeQuery();
@@ -1626,20 +1626,42 @@ boolean isUpdateable() throws SQLException {
     java.sql.ResultSet rs = ((PgDatabaseMetaData)connection.getMetaData()).getPrimaryUniqueKeys("",
         quotelessSchemaName, quotelessTableName);
 
+    String lastConstraintName = null;
+
     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
+      boolean isNotNull = rs.getBoolean("IS_NOT_NULL");
+
+      /* make sure that only unique keys with all non-null attributes are handled */
+      if (isNotNull) {
+        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}", i);
+    connection.getLogger().log(Level.FINE, "no of keys={0} from constraint {1}", new Object[]{i, lastConstraintName});
 
     /*
     it is only updatable if the primary keys are available in the resultset
@@ -1665,7 +1687,7 @@ boolean isUpdateable() throws SQLException {
     }
 
     if (!updateable) {
-      throw new PSQLException(GT.tr("No primary key found for table {0}.", tableName),
+      throw new PSQLException(GT.tr("No eligible primary or unique key found for table {0}.", tableName),
           PSQLState.INVALID_CURSOR_STATE);
     }
 

pgjdbc/src/test/java/org/postgresql/test/jdbc2/UpdateableResultTest.java

@@ -24,6 +24,8 @@
 import java.io.StringReader;
 import java.io.UnsupportedEncodingException;
 import java.sql.Array;
+import java.sql.Connection;
+import java.sql.Date;
 import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.SQLException;
@@ -39,7 +41,13 @@ public void setUp() throws Exception {
     super.setUp();
     TestUtil.createTable(con, "updateable",
         "id int primary key, name text, notselected text, ts timestamp with time zone, intarr int[]");
+    TestUtil.createTable(con, "hasdate", "id int primary key, dt date unique, name text");
+    TestUtil.createTable(con, "unique_null_constraint", "u1 int unique, name1 text");
+    TestUtil.createTable(con, "uniquekeys", "id int unique not null, id2 int unique, dt date");
+    TestUtil.createTable(con, "partialunique", "subject text, target text, success boolean");
+    TestUtil.execute("CREATE UNIQUE INDEX tests_success_constraint ON partialunique (subject, target) WHERE success", con);
     TestUtil.createTable(con, "second", "id1 int primary key, name1 text");
+    TestUtil.createTable(con, "primaryunique", "id int primary key, name text unique not null, dt date");
     TestUtil.createTable(con, "serialtable", "gen_id serial primary key, name text");
     TestUtil.createTable(con, "compositepktable", "gen_id serial, name text, dec_id serial");
     TestUtil.execute( "alter sequence compositepktable_dec_id_seq increment by 10; alter sequence compositepktable_dec_id_seq restart with 10", con);
@@ -48,16 +56,18 @@ public void setUp() throws Exception {
     TestUtil.createTable(con, "multicol", "id1 int not null, id2 int not null, val text");
     TestUtil.createTable(con, "nopkmulticol", "id1 int not null, id2 int not null, val text");
     TestUtil.createTable(con, "booltable", "id int not null primary key, b boolean default false");
-    TestUtil.execute( "insert into booltable (id) values (1)", con);
+    TestUtil.execute("insert into booltable (id) values (1)", con);
+    TestUtil.execute("insert into uniquekeys(id, id2, dt) values (1, 2, now())", con);
 
     Statement st2 = con.createStatement();
     // create pk for multicol table
     st2.execute("ALTER TABLE multicol ADD CONSTRAINT multicol_pk PRIMARY KEY (id1, id2)");
     // put some dummy data into second
     st2.execute("insert into second values (1,'anyvalue' )");
     st2.close();
-    TestUtil.createTable(con, "uniqueconstraint", "u1 int unique, name1 text");
-    TestUtil.execute("insert into uniqueconstraint values (1, 'dave')", con);
+    TestUtil.execute("insert into unique_null_constraint values (1, 'dave')", con);
+    TestUtil.execute("insert into unique_null_constraint values (null, 'unknown')", con);
+    TestUtil.execute("insert into primaryunique values (1, 'dave', now())", con);
 
   }
 
@@ -70,7 +80,11 @@ public void tearDown() throws SQLException {
     TestUtil.dropTable(con, "stream");
     TestUtil.dropTable(con, "nopkmulticol");
     TestUtil.dropTable(con, "booltable");
-    TestUtil.dropTable(con, "uniqueconstraint");
+    TestUtil.dropTable(con, "unique_null_constraint");
+    TestUtil.dropTable(con, "hasdate");
+    TestUtil.dropTable(con, "uniquekeys");
+    TestUtil.dropTable(con, "partialunique");
+    TestUtil.dropTable(con, "primaryunique");
     super.tearDown();
   }
 
@@ -447,6 +461,26 @@ public void testUpdateable() throws SQLException {
     st.close();
   }
 
+  @Test
+  public void testUpdateDate() throws Exception {
+    Date testDate = Date.valueOf("2021-01-01");
+    TestUtil.execute("insert into hasdate values (1,'2021-01-01'::date)", con);
+    con.setAutoCommit(false);
+    String sql = "SELECT * FROM hasdate where id=1";
+    ResultSet rs = con.createStatement(ResultSet.TYPE_SCROLL_INSENSITIVE,
+        ResultSet.CONCUR_UPDATABLE).executeQuery(sql);
+    assertTrue(rs.next());
+    assertEquals(testDate, rs.getDate("dt"));
+    rs.updateDate("dt", Date.valueOf("2020-01-01"));
+    rs.updateRow();
+    assertEquals(Date.valueOf("2020-01-01"), rs.getDate("dt"));
+    con.commit();
+    rs = con.createStatement().executeQuery("select dt from hasdate where id=1");
+    assertTrue(rs.next());
+    assertEquals(Date.valueOf("2020-01-01"), rs.getDate("dt"));
+    rs.close();
+  }
+
   @Test
   public void test2193() throws Exception {
     Statement st =
@@ -717,24 +751,152 @@ public void testUpdateBoolean() throws Exception {
 
   @Test
   public void testOidUpdatable() throws Exception {
+    Connection privilegedCon = TestUtil.openPrivilegedDB();
+    try {
+      Statement st = privilegedCon.createStatement(ResultSet.TYPE_SCROLL_INSENSITIVE,
+          ResultSet.CONCUR_UPDATABLE);
+      ResultSet rs = st.executeQuery("SELECT oid,* FROM pg_class WHERE relname = 'pg_class'");
+      assertTrue(rs.next());
+      assertTrue(rs.first());
+      rs.updateString("relname", "pg_class");
+      rs.updateRow();
+      rs.close();
+      st.close();
+    } finally {
+      privilegedCon.close();
+    }
+  }
+
+  @Test
+  public void testUniqueWithNullableColumnsNotUpdatable() throws Exception {
+    Statement st = con.createStatement(ResultSet.TYPE_SCROLL_INSENSITIVE,
+        ResultSet.CONCUR_UPDATABLE);
+    ResultSet rs = st.executeQuery("SELECT u1, name1 from unique_null_constraint");
+    assertTrue(rs.next());
+    assertTrue(rs.first());
+    try {
+      rs.updateString("name1", "bob");
+      fail("Should have failed since unique column u1 is nullable");
+    } catch (SQLException ex) {
+      assertEquals("No eligible primary or unique key found for table unique_null_constraint.",
+          ex.getMessage());
+    }
+    rs.close();
+    st.close();
+  }
+
+  @Test
+  public void testPrimaryAndUniqueUpdateableByPrimary() throws Exception {
     Statement st = con.createStatement(ResultSet.TYPE_SCROLL_INSENSITIVE,
         ResultSet.CONCUR_UPDATABLE);
-    ResultSet rs = st.executeQuery("SELECT oid,* FROM pg_class WHERE relname = 'pg_class'");
+    ResultSet rs = st.executeQuery("SELECT id, dt from primaryunique");
     assertTrue(rs.next());
     assertTrue(rs.first());
-    rs.updateString("relname", "pg_class");
+    int id = rs.getInt("id");
+    rs.updateDate("dt", Date.valueOf("1999-01-01"));
+    rs.updateRow();
+    assertFalse(rs.next());
+    rs.close();
+    rs = st.executeQuery("select dt from primaryunique where id = " + id);
+    assertTrue(rs.next());
+    assertEquals(Date.valueOf("1999-01-01"), rs.getDate("dt"));
+    rs.close();
+    st.close();
+  }
+
+  @Test
+  public void testPrimaryAndUniqueUpdateableByUnique() throws Exception {
+    Statement st = con.createStatement(ResultSet.TYPE_SCROLL_INSENSITIVE,
+        ResultSet.CONCUR_UPDATABLE);
+    ResultSet rs = st.executeQuery("SELECT name, dt from primaryunique");
+    assertTrue(rs.next());
+    assertTrue(rs.first());
+    String name = rs.getString("name");
+    rs.updateDate("dt", Date.valueOf("1999-01-01"));
+    rs.updateRow();
+    assertFalse(rs.next());
+    rs.close();
+    rs = st.executeQuery("select dt from primaryunique where name = '" + name + "'");
+    assertTrue(rs.next());
+    assertEquals(Date.valueOf("1999-01-01"), rs.getDate("dt"));
     rs.close();
     st.close();
   }
 
   @Test
-  public void testUniqueUpdatable() throws Exception {
+  public void testUniqueWithNullAndNotNullableColumnUpdateable() throws Exception {
     Statement st = con.createStatement(ResultSet.TYPE_SCROLL_INSENSITIVE,
         ResultSet.CONCUR_UPDATABLE);
-    ResultSet rs = st.executeQuery("SELECT * from uniqueconstraint");
+    int id = 0;
+    int id2 = 0;
+    ResultSet rs = st.executeQuery("SELECT id, id2, dt from uniquekeys");
     assertTrue(rs.next());
     assertTrue(rs.first());
-    rs.updateString("name1", "bob");
+    id = rs.getInt(("id"));
+    id2 = rs.getInt(("id2"));
+    rs.updateDate("dt", Date.valueOf("1999-01-01"));
+    rs.updateRow();
+    rs.close();
+    rs = st.executeQuery("select dt from uniquekeys where id = " + id + " and id2 = " + id2);
+    assertNotNull(rs);
+    assertTrue(rs.next());
+    assertEquals(Date.valueOf("1999-01-01"), rs.getDate("dt"));
+    rs.close();
+    st.close();
+  }
+
+  @Test
+  public void testUniqueWithNotNullableColumnUpdateable() throws Exception {
+    Statement st = con.createStatement(ResultSet.TYPE_SCROLL_INSENSITIVE,
+        ResultSet.CONCUR_UPDATABLE);
+    int id = 0;
+    ResultSet rs = st.executeQuery("SELECT id, dt from uniquekeys");
+    assertTrue(rs.next());
+    assertTrue(rs.first());
+    id = rs.getInt(("id"));
+    rs.updateDate("dt", Date.valueOf("1999-01-01"));
+    rs.updateRow();
+    rs.close();
+    rs = st.executeQuery("select id, dt from uniquekeys where id = " + id);
+    assertNotNull(rs);
+    assertTrue(rs.next());
+    assertEquals(Date.valueOf("1999-01-01"), rs.getDate("dt"));
+    rs.close();
+    st.close();
+  }
+
+  @Test
+  public void testUniqueWithNullableColumnNotUpdateable() throws Exception {
+    Statement st = con.createStatement(ResultSet.TYPE_SCROLL_INSENSITIVE,
+        ResultSet.CONCUR_UPDATABLE);
+    ResultSet rs = st.executeQuery("SELECT id2, dt from uniquekeys");
+    assertTrue(rs.next());
+    assertTrue(rs.first());
+    try {
+      rs.updateDate("dt", Date.valueOf("1999-01-01"));
+      fail("Should have failed since id2 is nullable column");
+    } catch (SQLException ex) {
+      assertEquals("No eligible primary or unique key found for table uniquekeys.",
+          ex.getMessage());
+    }
+    rs.close();
+    st.close();
+  }
+
+  @Test
+  public void testNoUniqueNotUpdateable() throws SQLException {
+    Statement st = con.createStatement(ResultSet.TYPE_SCROLL_INSENSITIVE,
+        ResultSet.CONCUR_UPDATABLE);
+    ResultSet rs = st.executeQuery("SELECT dt from uniquekeys");
+    assertTrue(rs.next());
+    assertTrue(rs.first());
+    try {
+      rs.updateDate("dt", Date.valueOf("1999-01-01"));
+      fail("Should have failed since no UK/PK are in the select statement");
+    } catch (SQLException ex) {
+      assertEquals("No eligible primary or unique key found for table uniquekeys.",
+          ex.getMessage());
+    }
     rs.close();
     st.close();
   }