#816 Fix: ResultSet wasNull throws exception on insert row

src/docs/asciidoc/release_notes.adoc

@@ -42,6 +42,11 @@ This fix was backported from Jaybird 6.
 * Fixed: Error "`Column unknown; IND.RDB$CONDITION_SOURCE`" when calling `DatabaseMetaData.getIndexInfo` on Firebird 5.0 with a Firebird 4.0 (ODS 13.0) database (https://github.com/FirebirdSQL/jaybird/issues/813[#813])
 +
 The partial index metadata support introduced in Jaybird 5.0.5 did not take into account that Firebird 5.0 could also open a Firebird 4.0 database.
+* Fixed: Calling `ResultSet.wasNull()` when on the insert-row throws a `SQLException` instead of reporting the null-state of the last retrieved column (https://github.com/FirebirdSQL/jaybird/issues/816[#816])
++
+As part of this fix, we no longer check if `wasNull()` is called on a row, or after getting a column value, as the JDBC API does not require such check.
+Instead, `false` is reported if not currently on a row, or if no column value was retrieved, and an exception is thrown if the result set is closed.
+The value reported by `wasNull()` is reset to `false` on each cursor move (e.g. `next()`), and modified by each `get__XXX__` call.
 
 [#jaybird-5-0-5-changelog]
 === Jaybird 5.0.5

src/main/org/firebirdsql/jdbc/FBResultSet.java

@@ -65,10 +65,9 @@ public class FBResultSet implements ResultSet, FirebirdResultSet, FBObjectListen
 
     protected RowValue row;
 
-    private boolean wasNull = false;
-    private boolean wasNullValid = false;
+    private boolean wasNull;
     // closed is false until the close method is invoked;
-    private volatile boolean closed = false;
+    private volatile boolean closed;
 
     private SQLWarning firstWarning;
 
@@ -360,12 +359,10 @@ protected void checkScrollable() throws SQLException {
      *         if something wrong happened.
      */
     protected void closeFields() throws SQLException {
-        // TODO See if we can apply completion reason logic (eg no need to close blob on commit)
-        wasNullValid = false;
-
-        // if there are no fields to close, then nothing to do
-        if (closeableFields.isEmpty())
-            return;
+        // TODO See if we can apply completion reason logic (e.g. no need to close blob on commit)
+        wasNull = false;
+        // if there are no fields to close, then nothing else to do
+        if (closeableFields.isEmpty()) return;
 
         SQLExceptionChainBuilder<SQLException> chain = new SQLExceptionChainBuilder<>();
         // close current fields, so that resources are freed.
@@ -458,12 +455,7 @@ void close(boolean notifyListener, CompletionReason completionReason) throws SQL
 
     @Override
     public boolean wasNull() throws SQLException {
-        if (!wasNullValid) {
-            throw new SQLException("Look at a column before testing null.");
-        }
-        if (row == null) {
-            throw new SQLException("No row available for wasNull.");
-        }
+        checkOpen();
         return wasNull;
     }
 
@@ -605,11 +597,8 @@ public Reader getNCharacterStream(int columnIndex) throws SQLException {
      *         If there is an error accessing the field
      */
     public FBField getField(int columnIndex) throws SQLException {
-        final FBField field = getField(columnIndex, true);
-
-        wasNullValid = true;
-        wasNull = row == null || row.getFieldData(columnIndex - 1) == null;
-
+        FBField field = getField(columnIndex, true);
+        wasNull = field.isNull();
         return field;
     }
 
@@ -627,11 +616,7 @@ public FBField getField(int columnIndex, boolean checkRowPosition) throws SQLExc
             throw new SQLException("Invalid column index: " + columnIndex, SQLStateConstants.SQL_STATE_INVALID_COLUMN);
         }
 
-        if (rowUpdater != null) {
-            return rowUpdater.getField(columnIndex - 1);
-        } else {
-            return fields[columnIndex - 1];
-        }
+        return rowUpdater != null ? rowUpdater.getField(columnIndex - 1) : fields[columnIndex - 1];
     }
 
     /**
@@ -643,27 +628,13 @@ public FBField getField(int columnIndex, boolean checkRowPosition) throws SQLExc
      *         if the field cannot be retrieved
      */
     public FBField getField(String columnName) throws SQLException {
-        checkOpen();
-        if (row == null && rowUpdater == null) {
-            throw new SQLException("The result set is not in a row, use next", SQLStateConstants.SQL_STATE_NO_ROW_AVAIL);
-        }
-
-        if (columnName == null) {
-            throw new SQLException("Column identifier must be not null.", SQLStateConstants.SQL_STATE_INVALID_COLUMN);
-        }
-
         Integer fieldNum = colNames.get(columnName);
         // If it is the first time the columnName is used
         if (fieldNum == null) {
             fieldNum = findColumn(columnName);
             colNames.put(columnName, fieldNum);
         }
-        final FBField field = rowUpdater != null
-                ? rowUpdater.getField(fieldNum - 1)
-                : fields[fieldNum - 1];
-        wasNullValid = true;
-        wasNull = row == null || row.getFieldData(fieldNum - 1) == null;
-        return field;
+        return getField(fieldNum);
     }
 
     /**
@@ -837,8 +808,9 @@ public Object getObject(String columnName) throws SQLException {
     // the first instance of the column will be returned
     @Override
     public int findColumn(String columnName) throws SQLException {
-        if (columnName == null || columnName.equals("")) {
-            throw new SQLException("Empty string does not identify column.", SQLStateConstants.SQL_STATE_INVALID_COLUMN);
+        if (columnName == null || columnName.isEmpty()) {
+            throw new SQLException("Empty string or null does not identify a column",
+                    SQLStateConstants.SQL_STATE_INVALID_COLUMN);
         }
         if (columnName.startsWith("\"") && columnName.endsWith("\"")) {
             columnName = columnName.substring(1, columnName.length() - 1);

src/main/org/firebirdsql/jdbc/FBRowUpdater.java

@@ -120,10 +120,10 @@ final class FBRowUpdater implements FirebirdRowUpdater {
             final FieldDataProvider dataProvider = new FieldDataProvider() {
                 @Override
                 public byte[] getFieldData() {
-                    if (!updatedFlags[fieldPos]) {
-                        return oldRow.getFieldData(fieldPos);
-                    } else if (inInsertRow) {
+                    if (inInsertRow) {
                         return insertRow.getFieldData(fieldPos);
+                    } else if (!updatedFlags[fieldPos]) {
+                        return oldRow.getFieldData(fieldPos);
                     } else {
                         return newRow.getFieldData(fieldPos);
                     }

src/test/org/firebirdsql/jdbc/FBResultSetTest.java

@@ -1471,6 +1471,46 @@ static Stream<Arguments> testIsBeforeFirst_isAfterLast_emptyResultSet_bug807() {
         return defaultArguments;
     }
 
+    @ParameterizedTest
+    @MethodSource
+    void wasNull_onInsertRow(int resultSetType, String scrollableCursorPropertyValue) throws Exception {
+        try (Connection connection = createConnection(scrollableCursorPropertyValue)) {
+            executeCreateTable(connection, "create table t1 (i int primary key, v varchar(10))");
+
+            try (PreparedStatement pstmt = connection.prepareStatement("insert into t1 values(?, ?)")) {
+                for (int i = 1; i <= 10; i++) {
+                    pstmt.setInt(1, i);
+                    pstmt.setString(2, i + " s");
+                    pstmt.addBatch();
+                }
+                pstmt.executeBatch();
+            }
+
+            try (Statement stmt = connection.createStatement(resultSetType, CONCUR_UPDATABLE);
+                 ResultSet rs = stmt.executeQuery("select i, v from t1")) {
+                rs.moveToInsertRow();
+                rs.updateNull(1);
+                rs.updateString(2, "11 s");
+
+                assertNull(rs.getObject(1), "column 1");
+                assertTrue(rs.wasNull(), "column 1");
+                assertEquals("11 s", rs.getObject(2), "column 2");
+                assertFalse(rs.wasNull(), "column 2");
+            }
+        }
+    }
+
+    static Stream<Arguments> wasNull_onInsertRow() {
+        Stream<Arguments> defaultArguments = Stream.of(
+                Arguments.of(TYPE_FORWARD_ONLY, SCROLLABLE_CURSOR_EMULATED),
+                Arguments.of(TYPE_SCROLL_INSENSITIVE, SCROLLABLE_CURSOR_EMULATED));
+        if (getDefaultSupportInfo().supportsScrollableCursors()) {
+            return Stream.concat(defaultArguments, Stream.of(
+                    Arguments.of(TYPE_SCROLL_INSENSITIVE, SCROLLABLE_CURSOR_SERVER)));
+        }
+        return defaultArguments;
+    }
+
     static Stream<String> scrollableCursorPropertyValues() {
         // We are unconditionally emitting SERVER, to check if the value behaves appropriately on versions that do
         // not support server-side scrollable cursors