VARBINARY not supported and issue with BINARY

[stefanofornari]

Hello,

the following case does not work with cassandra-jdbc-driver:

// create table atable(id blob primary key)
Connection c = DriverManager.getConnection(DB, null, null);
PreparedStatement s = c.prepareStatement("INSERT INTO atable (id) VALUES (?)");
final byte[] ID1 = new byte[] {
    1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
    1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1
};
s.setObject(1, ID1, Types.VARBINARY);
s.executeUpdate();

throwing the errror:

java.sql.SQLException: Unsupported SQL type: -3
	at com.ing.data.cassandra.jdbc.CassandraPreparedStatement.setObject(CassandraPreparedStatement.java:662)
	at com.ing.data.cassandra.jdbc.CassandraPreparedStatement.setObject(CassandraPreparedStatement.java:538)

I think the issue is in setObject()

cassandra-jdbc-wrapper/src/main/java/com/ing/data/cassandra/jdbc/CassandraPreparedStatement.java

Line 543 in 5f9fda7

public final void setObject(final int parameterIndex, final Object x, final int targetSqlType,

In particular, in the block handling Types.BINARYY

cassandra-jdbc-wrapper/src/main/java/com/ing/data/cassandra/jdbc/CassandraPreparedStatement.java

Lines 556 to 564 in 5f9fda7

	case Types.BINARY:
	final byte[] array = new byte[((ByteArrayInputStream) x).available()];
	try {
	((ByteArrayInputStream) x).read(array);
	} catch (final IOException e) {
	LOG.warn("Exception while setting object of BINARY type.", e);
	}
	this.boundStatement = this.boundStatement.setByteBuffer(parameterIndex - 1, ByteBuffer.wrap(array));
	break;

As per the JDBC documentation, the method should perform a conversion taking into account multiple cases for the input type. However I agree to take a simpler approach at least to start with, but I see to main issues with the current implementation:

1. it does not handle Types.VARBINARY (therefore the exception); this can be easily fixed handling it as a synonym of BINARY
2. it assumes the object to be an InpuStream while instead it can be any type. Leaving apart handling Ref, Blob, Clob, NClob, Struct, java.net.URL, and Array, it should support at least the case the object is a byte array.

I would propose something like this:

case Types.BINARY:
case Types.VARBINARY:
    byte[] array = null;
    if (x instanceof ByteArrayInputStream) {
        array = new byte[((ByteArrayInputStream) x).available()];
        try {
            ((ByteArrayInputStream) x).read(array);
        } catch (final IOException e) {
            LOG.warn("Exception while setting object of BINARY type.", e);
        }
    } else if (x instanceof byte[]) {
        array = (byte[])x;
    } else {
        throw new SQLException("Unsupported parameter type: " + x.getClass());
    }
    this.boundStatement = this.boundStatement.setByteBuffer(parameterIndex - 1, ByteBuffer.wrap(array));
    break;

I won't be able to provide a PR due to the need of a dockered Cassandra... :(

[maximevw]

Hello @stefanofornari,

Thank you for reporting this issue.

You're right, VARBINARY and LONGVARBINARY aren't handled properly.

If you can, alternatives exist to execute this prepared statement until the setObject() method is fixed:

s.setBytes(1, ID1);

or

s.setBlob(1, new SerialBlob(ID1));

or transforming the byte array passed to setObject() into a ByteArrayInputStream (I admit this is neither really optimal nor elegant).

I will take a closer look at this point to provide a solution as complete as possible (the one you suggested is a good basis), but I cannot guarantee a release date for this. As workarounds exist to get similar results, I'll probably include this fix in the next minor version (4.11.0) to release at the end of this year or beginning of 2024.

NB: regarding the tests, you just need a running docker, the image is provided through Testcontainers. But it's ok if you can't run docker for some reason. Using this solution for testing is the more convenient to run tests in an environment as close as possible of real conditions.

[stefanofornari]

Hi @maximevw, thanks for taking care of this. Let me first disclaim I have little/no control on the actual JDBC calls because I am using a ORM which then use cassandra-wrapper.

The proposal solution unfortunately does not work because VARBINARY is currently not supported at all. It would be great if we could have just an hot fix with case Types.VARBINARY: added.

Regarding docker, I do not want to open a debate, but IMHO unit tests of the driver should not relay on the presence of a real server (for multiple reasons, not just the added complexity).

[maximevw]

@stefanofornari, since you can't apply the workaround in your case, I'll try to provide a quick hotfix for VARBINARY handling in setObject() as soon as possible (I'm a little busy at the moment).

Regarding the tests, I agree pure unit testing, in general, shouldn't rely on servers/containers. However, for a JDBC implementation, we are closer to integration testing because we want to be sure the driver is able to connect to a database and to execute queries properly. Mocking the database is not easier in my opinion and error prone. If you look at how JDBC drivers for MS SQL Server (https://github.com/Microsoft/mssql-jdbc/) or PostgreSQL (https://github.com/pgjdbc/pgjdbc) are tested, they are against a real database. So, this is just two examples (but not the least), but it seems that is a good manner to test a JDBC driver.
Feel free to discuss it in a separate topic if you have ideas or proposal to improve that.

[maximevw]

Hello @stefanofornari,

I just released the version 4.10.2 including a fix for this issue. The version will also be available in Maven Central in next hours.

A review of types potentially not handled by setObject() methods will be performed before releasing version 4.11.0 of the JDBC wrapper.

[stefanofornari]

Great stuff! I will give it a try in the tomorrow and let you know.

[stefanofornari]

I confirm it work for me.