#880 Fix batch error "Repeated blob id 0:0 in registerBlob()"

Author: mrotteveel

src/docs/asciidoc/release_notes.adoc

@@ -42,6 +42,7 @@ The following was fixed or changed since Jaybird 6.0.2:
 A large amount (>= 64) of statement closes (or cleanup of leaked statements) without interleaving other server operations could cause a connection to Firebird 4.0 or higher to hang.
 This could also hang the cleaner thread used for closing leaked statements.
 * Fixed: `ResultSet` move incorrectly closes input `Clob` (https://github.com/FirebirdSQL/jaybird/issues/880[#880])
+* Fixed: Batch execution with multiple empty strings resulted in error _"Repeated blob id 0:0 in registerBlob()"_ (https://github.com/FirebirdSQL/jaybird/issues/888[#888])
 
 === Jaybird 6.0.2
 

src/main/org/firebirdsql/gds/ng/wire/version16/V16Statement.java

@@ -41,8 +41,10 @@
 
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
+import java.nio.ByteBuffer;
 import java.sql.SQLException;
 import java.util.Collection;
+import java.util.HashSet;
 import java.util.List;
 
 import static org.firebirdsql.gds.impl.wire.WireProtocolConstants.op_batch_cancel;
@@ -166,10 +168,14 @@ private void registerBlobs(XdrOutputStream xdrOut, RowDescriptor parameterDescri
         List<Integer> blobPositions = blobPositions(parameterDescriptor);
         if (blobPositions.isEmpty()) return;
         FbWireDatabase db = getDatabase();
+        var blobsRegistered = new HashSet<>((int) (rowValues.size() * blobPositions.size() / 0.75f + 1));
         for (RowValue rowValue : rowValues) {
             for (int position : blobPositions) {
                 byte[] fieldData = rowValue.getFieldData(position);
                 if (fieldData == null) continue;
+                // Do not register a blob multiple times
+                if (!blobsRegistered.add(toLong(fieldData))) continue;
+
                 xdrOut.writeInt(WireProtocolConstants.op_batch_regblob);
                 xdrOut.writeInt(getHandle()); // p_batch_statement
                 // register as itself
@@ -181,6 +187,12 @@ private void registerBlobs(XdrOutputStream xdrOut, RowDescriptor parameterDescri
         xdrOut.flush();
     }
 
+    // This is not necessarily the correct endianness, we just want the id bytes expressed as a long.
+    private static long toLong(byte[] bytes) {
+        assert bytes.length == 8 : "expected 8 bytes";
+        return ByteBuffer.wrap(bytes).getLong();
+    }
+
     private List<Integer> blobPositions(RowDescriptor parameterDescriptor) {
         return parameterDescriptor.getFieldDescriptors().stream()
                 .filter(f -> f.isFbType(ISCConstants.SQL_BLOB))

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

@@ -34,6 +34,7 @@
 import java.time.LocalDateTime;
 import java.time.LocalTime;
 import java.util.Arrays;
+import java.util.List;
 import java.util.Properties;
 import java.util.stream.IntStream;
 
@@ -42,6 +43,9 @@
 import static org.firebirdsql.common.FBTestProperties.getDefaultPropertiesForConnection;
 import static org.firebirdsql.common.FBTestProperties.getUrl;
 import static org.firebirdsql.common.FbAssumptions.assumeServerBatchSupport;
+import static org.firebirdsql.common.assertions.ResultSetAssertions.assertNextRow;
+import static org.firebirdsql.common.assertions.ResultSetAssertions.assertNoNextRow;
+import static org.firebirdsql.common.assertions.ResultSetAssertions.assertRowEquals;
 import static org.firebirdsql.common.matchers.SQLExceptionMatchers.message;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsString;
@@ -399,6 +403,36 @@ void testPreparedStatementBatch_65Blobs() throws SQLException {
         }
     }
 
+    /**
+     * Rationale: see <a href="https://github.com/FirebirdSQL/jaybird/issues/888">#888</a>.
+     */
+    @ParameterizedTest(name = "[{index}] useServerBatch = {0}")
+    @ValueSource(booleans = { true, false })
+    void testBatchMultipleEmptyStringsInBlob(boolean useServerBatch) throws Exception {
+        try (var connection = createConnection(useServerBatch);
+             var stmt = connection.createStatement()) {
+            stmt.execute(RECREATE_BATCH_UPDATES_TABLE);
+            connection.setAutoCommit(false);
+            try (var ps = connection.prepareStatement("INSERT INTO batch_updates(id, clob_value) VALUES (?, ?)")) {
+                ps.setInt(1, 1);
+                ps.setString(2, "");
+                ps.addBatch();
+                ps.setInt(1, 2);
+                ps.setString(2, "");
+                ps.addBatch();
+                assertDoesNotThrow(ps::executeBatch);
+            }
+
+            try (var rs = stmt.executeQuery("select ID, CLOB_VALUE from BATCH_UPDATES order by ID")) {
+                assertNextRow(rs);
+                assertRowEquals(rs, List.of(1, ""));
+                assertNextRow(rs);
+                assertRowEquals(rs, List.of(2, ""));
+                assertNoNextRow(rs);
+            }
+        }
+    }
+
     private static Connection createConnection(boolean useServerBatch) throws SQLException {
         Properties props = getDefaultPropertiesForConnection();
         props.setProperty(PropertyNames.useServerBatch, String.valueOf(useServerBatch));