Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for conversion to JdbcValue and store byte[] as binary [DATAJDBC-327] #549

Closed
spring-projects-issues opened this issue Feb 3, 2019 · 7 comments
Assignees

Comments

@spring-projects-issues
Copy link

@spring-projects-issues spring-projects-issues commented Feb 3, 2019

mikleing opened DATAJDBC-327 and commented

Java type byte[] is not being stored properly in the database.  I'm using MySQL on the default driver.  Here is the entity:

public class User {
     @Id
     public Integer id;
     public byte[] value;
}

I save a user with the byte array value 0xFFFFFFFFFFFFFFFF.

User user = new User();
user.value = new byte[] {(byte) 0xff, (byte) 0xff, (byte) 0xff, (byte) 0xff, (byte) 0xff, (byte) 0xff, (byte) 0xff, (byte) 0xff};
user = userRepository.save(user);

The actual value saved in the database is the following.

0x2D31000000000000

The database type is BINARY(8).  I believe (but am not sure) that Spring Data JDBC thinks that the column type in the database is INT.

The interesting thing is that a converter from an object to a byte array works fine.  Here is a working example:

@WritingConverter@WritingConverter
public static class UuidToByteConverter implements Converter<UUID, byte[]> {
     @Override public byte[] convert(UUID source) {
          ByteBuffer byteBuffer = ByteBuffer.wrap(new byte[16]);
          byteBuffer.putLong(source.getMostSignificantBits());
          byteBuffer.putLong(source.getLeastSignificantBits());
          return byteBuffer.array();
     }
}

I think that byte arrays should automatically be stored correctly in the database.  If not, it would be helpful to know how to use a converter to correct the problem


Affects: 1.0.4 (Lovelace SR4)

Referenced from: pull request #123

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Feb 3, 2019

Jens Schauder commented

Support for arrays only comes with DATAJDBC-259 which is still in review. 

Once that is merged a Java array could be saved in a column of type ARRAY in the database.

I'm not sure what you mean by 

it would be helpful to know how to use a converter to correct the problem.

Since you write a couple lines before that:

a converter from an object to a byte array works fine.

Could you clarify what else you are missing?

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Feb 5, 2019

mikleing commented

I would like to store a Java byte[] object into a MySQL BINARY column.  Will the fix associated with DATAJDBC-259 allow me to do that?

You can ignore my question about the converter.  Thank you!!

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Feb 5, 2019

Jens Schauder commented

No, DATAJDBC-259 will store a byte[] in an ARRAY column.
For accessing a BINARY column you'll still need to use converters and a custom class wrapping the byte[]

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Feb 6, 2019

mikleing commented

What I'm hearing you say is that I should create a class like the following:

public class ByteArray {
     public byte[] bytes;
}

Then I could create a converter like the following:

@WritingConverter
public class ByteConverter implements Converter<ByteArray, byte[]> {
     @Override
     public byte[] convert(ByteArray source) {
          return source.bytes
     }
}

This seems like an odd solution.  The result of the converter is a byte[].  That byte[] is being converted properly to MySQL BINARY.  Wouldn't it be better to allow that to happen directly without having to wrap the byte[]?

Also, this solution only partially works.  I believe there is a bug preventing this solution from working when there are One to Many relationships.  Here is a link to that bug: DATAJDBC-326  I'm ultimately trying to get UUIDs to work within Spring Data JDBC

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Feb 6, 2019

Jens Schauder commented

Your assessment is correct.

Wrapping a byte array just to then extract the byte array is an odd solution.
I'm not saying that we don't want to improve on that, but it is what currently works (modulo the bug you mentioned) and I'm not inclined to create some kind of specialized solution for byte[], let alone byte[] for MySql.

Currently, I'm thinking more about extending the conversion infrastructure to specify as what kind of JDBCType a value should get stored. This is ticket DATAJDBC-239.

Apart from not liking specialized solutions I kind of doubt that a byte[] has its place in a domain model if it is to represent binary data.
A proper domain object, for example, a UUID is the better solution in this case.

So what is left for now is fixing DATAJDBC-326 which is high on my list of priorities

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Feb 7, 2019

mikleing commented

I agree that you shouldn't develop specialized solutions.  Naively it seems like byte[] should be supported without a specialized solution.  But if it can't then I agree it isn't worth it.  Furthermore, if you fix DATAJDBC-326 then I will be more than satisfied.  Thank you for working on that bug!  Do you have any sense of when it will be solved?

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Feb 20, 2019

Jens Schauder commented

We had another round of discussion and we will persist byte[] as binary in the future and offer a way to store it as something else, if one wishes so

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants