Bug #43759 ResultSet.deleteRow() corrupts generated DELETE SQL for binary primary key
Submitted: 19 Mar 2009 19:26 Modified: 16 Sep 2009 13:47
Reporter: Chris Lamprecht Email Updates:
Status: Closed
Category:Connector/J Severity:S2 (Serious)
Version:5.1.7 OS:Any
Assigned to: Mark Matthews Target Version:
Tags: BINARY, character encoding, deleteRow
Triage: D2 (Serious)

[19 Mar 2009 19:26] Chris Lamprecht
Description:
Calling ResultSet.deleteRow() on a table with a primary key of type BINARY(8) was
silently failing to actually delete the row, but only in some (repeatable) cases.  I
logged the generated SQL to a file using autoGenerateTestcaseScript=true, and determined
that the DELETE statement being generated had corrupted part of the primary key data. 
Specifically, one of the bytes was changed from 0x90 to 0x9D, although the corruption
seemed to be different on Windows vs. Linux (probably due to Java's default character
encoding).

Note that this could also cause a DELETE statement that actually deletes another
(incorrect) row in the database, leading to lost data.

I looked at the source code of com.mysql.jdbc.UpdatableResultSet, and in the deleteRow()
method, at lines 487-504, it appears to be using a String, with some character encoding,
to generate the DELETE SQL statement.  Our binary data is not a string (i.e., it has no
character encoding).  At line 482, there is a code comment:

//FIXME: Use internal routines where possible for character
//conversion!

Compare this code to com.mysql.jdbc.PreparedStatement.setBytes(), which seems to work
correctly. 

How to repeat:

call deleteRow() on the ResultSet for a table where the primary key is a BINARY(8)
column.  In addition, the data must have a value such that it gets corrupted, which
doesn't always happen.  I found that the byte 0x90 seems to get corrupted both on Windows
and Linux, but it's possible you'll have to try other data, depending on your default
character encoding (I think).

I'll paste a complete Java testcase below, which shows a case of deleteRow() failing, and
a case of it succeeding.

import java.sql.*;

/**
 * Testcase for MySQL Connector/J 5.1.7 bug report.
 *
 * I think the bug is in UpdatableResultSet.deleteRow(), where the binary data is being
corrupted
 * because it is going through a String (with some character encoding), rather than being
treated
 * as raw binary data.  Compare to com.mysql.jdbc.PreparedStatement.setBytes(), which
seems to work
 * correctly.
 */
public class TestDeleteRowBinaryColumn {

    public static void main(String[] args) throws SQLException {
        String databaseUrl =
"jdbc:mysql://localhost/test?autoGenerateTestcaseScript=true";
        String databaseUser = "root";
        String databasePassword = "";
        Connection conn = DriverManager.getConnection(databaseUrl, databaseUser,
databasePassword);

        // create the table
        Statement createStmt = conn.createStatement();
        createStmt.executeUpdate("DROP TABLE IF EXISTS testtable_bincolumn");
        createStmt.close();
        createStmt = conn.createStatement();
        createStmt.executeUpdate("CREATE TABLE testtable_bincolumn (" +
                "bincolumn binary(8) NOT NULL, " +
                "PRIMARY KEY (bincolumn)" +
                ") ENGINE=InnoDB");
        createStmt.close();

        String pkValue1 = "0123456789ABCD90";
        String pkValue2 = "0123456789ABCD00";
        // put some data in it
        createStmt = conn.createStatement();
        createStmt.executeUpdate("INSERT INTO testtable_bincolumn (bincolumn) " +
                "VALUES (unhex('"+pkValue1+"')), (unhex('"+pkValue2+"'))");
        createStmt.close();

        // cause the bug
        Statement stmt = conn.createStatement(ResultSet.TYPE_FORWARD_ONLY,
ResultSet.CONCUR_UPDATABLE);
        ResultSet rs = stmt.executeQuery("SELECT * FROM testtable_bincolumn WHERE
bincolumn = unhex('"+pkValue1+"')");
        if (rs.next()) {
            System.out.println("We got a row for "+pkValue1+", about to delete it ...");
            rs.deleteRow();
        }
        rs.close();
        stmt.close();

        // At this point the row with pkValue1 should be deleted.  We'll select it back
to see.
        // If the row comes back, the testcase has failed.

        stmt = conn.createStatement();
        rs = stmt.executeQuery("SELECT * FROM testtable_bincolumn WHERE bincolumn =
unhex('"+pkValue1+"')");
        if (rs.next()) {
            System.out.println("Testcase #1 FAILED, the row was not deleted by
deleteRow()");
        } else {
            System.out.println("Testcase #1 PASSED, the row was deleted by
deleteRow()");
        }

        rs.close();
        stmt.close();

        // Now, show a case where it happens to work, because the binary data is
different
        stmt = conn.createStatement(ResultSet.TYPE_FORWARD_ONLY,
ResultSet.CONCUR_UPDATABLE);
        rs = stmt.executeQuery("SELECT * FROM testtable_bincolumn WHERE bincolumn =
unhex('"+pkValue2+"')");
        if (rs.next()) {
            System.out.println("We got a row for "+pkValue2+", about to delete it ...");
            rs.deleteRow();
        }
        rs.close();
        stmt.close();

        stmt = conn.createStatement();
        rs = stmt.executeQuery("SELECT * FROM testtable_bincolumn WHERE bincolumn =
unhex('"+pkValue2+"')");
        if (rs.next()) {
            System.out.println("Testcase #2 FAILED, the row was not deleted by
deleteRow()");
        } else {
            System.out.println("Testcase #2 PASSED, the row was deleted by
deleteRow()");
        }

        rs.close();
        stmt.close();
        conn.close();
    }
}

Suggested fix:
Update com.mysql.jdbc.UpdatableResultSet to avoid using a Java String for generating the
SQL for binary data.  See com.mysql.jdbc.PreparedStatement.setBytes() for an
implementation that seems to work.
[19 Mar 2009 19:27] Chris Lamprecht
Testcase Java source code

Attachment: TestDeleteRowBinaryColumn.java (application/octet-stream, text), 3.57 KiB.

[20 Mar 2009 18:29] Tonci Grgin
Hi Chris and thanks for excellent report.

I can't shake off feeling there's something wrong with encoding... Can you please attach
my.cnf file and output of SHOW CREATE TABLE for test table. A general query log from
MySQL server at the time of testing would be nice too.
[20 Mar 2009 18:33] Tonci Grgin
Personally, I think this is a bug and if you don't respond I'll mark it verified anyway.

DELETE statement just doesn't look right:
2 Query	DELETE FROM `test`.`bug43759` WHERE
`test`.`bug43759`.`bincolumn`<=>'#Eg��͐'

^ here
[20 Mar 2009 18:35] Tonci Grgin
Ummm, "here" was aimed at 
[20 Mar 2009 18:45] Chris Lamprecht
Hi Tonci,

Thanks for the reply.  I'll attach a my.cnf in a moment, and I'll try to get a general
query log after that.  Here is the show create table for the test table used in my java
test program:

CREATE TABLE `testtable_bincolumn` (
  `bincolumn` binary(8) NOT NULL,
  PRIMARY KEY  (`bincolumn`)
) ENGINE=InnoDB DEFAULT CHARSET=latin1
[20 Mar 2009 18:47] Chris Lamprecht
my.cnf file

Attachment: testcase_my.cnf (, text), 901 bytes.

[20 Mar 2009 19:10] Chris Lamprecht
server log file while running my java testcase

Attachment: gandalf.log (text/plain), 2.58 KiB.

[20 Mar 2009 19:18] Chris Lamprecht
I added the server log file captured while running the testcase.  Since it contains binary
data for the DELETE queries in question, you might need to view it using a utility such as
unix's "od" tool.  I view it like this:

od -t x1z gandalf.log

The delete queries look like this (there are two of them):

DELETE FROM `chris`.`testtable_bincolumn` WHERE
`chris`.`testtable_bincolumn`.`bincolumn`<=>'BINARY_DATA'

In particular, when viewing it like this, here are the bytes that I see for the 
BINARY_DATA for the two DELETE queries -- these are the bytes inside the single quotes:

01 23 45 67 89 ab cd 9d

01 23 45 67 89 ab cd 5c 30

Note that the first series of bytes should be "01 23 45 67 89 ab cd 90", so the last byte
is being corrupted from "90" to "9d".

The second one should be "01 23 45 67 89 ab cd 00", and it looks like the last "00" is
being escaped as "\0".  Since the correct row is being deleted in the DB, this seems to
be correct.

Let me know if I can provide any other information.  thanks
[20 Mar 2009 20:52] Tonci Grgin
Chris, thanks for info. It's almost Saturday here so I'll get back to this after weekend.
[6 Apr 2009 7:34] Tonci Grgin
Chris, you are absolutely right, verified as described with test case attached.

Looking into general query log with hex editor, last byte in 1 Query	DELETE FROM
`test`.`bug43759` WHERE `test`.`bug43759`.`bincolumn`<=> ... *is* 9D and not 90. Test
case works as expected in cl client.
[20 Aug 2009 21:36] Mark Matthews
Fixed for 5.1.9.
[16 Sep 2009 13:47] Tony Bedford
An entry was added to the 5.1.9 changelog:

Calling ResultSet.deleteRow() on a table with a primary key of type BINARY(8) silently
failed to delete the row, but only in some repeatable cases. The generated DELETE
statement generated corrupted part of the primary key data. Specifically, one of the
bytes was changed from 0x90 to 0x9D, although the corruption appeared to be different
depending on whether the application was run on Windows or Linux.