-
Notifications
You must be signed in to change notification settings - Fork 955
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
Adding support for unsigned tinyint values #152
Conversation
Can one of the admins verify this patch? |
It looks like @vamossagar12 hasn't signed our Contributor License Agreement, yet. Appreciation of efforts, clabot |
I can't see the Contributor License Agreement link. |
Sorry about that, you can sign it at http://clabot.confluent.io/cla |
I have signed twice.. I says Thanks for signing.. Would that be sufficient? |
Yes, that's fine |
Hi, Any updates on this? |
The information you provided in the description is for SMALLINT, for TINYINT https://docs.oracle.com/javase/6/docs/technotes/guides/jdbc/getstart/mapping.html notes
While a case can be made for using
What JDBC driver is that with? I'd expect |
Linking #98 |
@shikhar Ah ok That was an oversight. Sorry about that. But yeah i wanted to post about TINYINT. Basically what I have done is check if the value is greater than BYTE.MAX. if it isn't then just pass along a byte otherwise, pass a short. I have cpoied in mysql-connector-java-5.1.21.jar in the classpath. Also, without this change, I get the following exception:
Which is basically ResultSet.getByte(). What I have tried to do is to broaden the net for 8 byte values to ensure that a Short goes only if the value is greater than BYTE.MAX otherwise only bytes go through. Since this has dependency across ConnectSchema and SchemaRegistry, I have made changes and raised separate PRs for those as well. Here's the same for your reference: https://github.com/apache/kafka/pull/2044-> raised to apache kafka. The jenkins build failed here and I have been told that these are being worked upon correctly. https://github.com/confluentinc/schema-registry/pull/432--> raised against schema regisrty. IT does the opposite of the above. Please let me know in case of any issues/concerns. |
hey @shikhar did you get a chance to look at this? |
I think we should change the code to We are thinking along the lines of supporting lightweight transformations in Kafka Connect, and certain integer widening operations seem seem reasonable to me. So one could |
@shikhar So I commented out the code that I had in this PR and this what you suggested:
As you said, the jdbc error got resolved but when I ran the code again on a newly downloaded confluent package(with none of the other changes that I had made in kafka and schema-registry projects) I got the following exception:
This error is at the main kafka project level. Inside the ConnectSchema. While converting it does a check that whether the type of the object passed and schema type match up or not. The
SCHEMA_TYPE_CLASSES.put(Type.INT8, Arrays.asList((Class) Byte.class)) and hence this fails. On a side note, I looked at the way Debezium handles tinyint values, and then seem to handle both signed and unsigned values using just a Short value. So, this could be another option to consider(as you mentioned in one of your previous comments). |
@shikhar Gwen mentioned in one of the tickets in the confluent google groups that there's a plan to add single-message transforms to the connect layer to resolve the kinds of issues we discussed in this PR. So, whatever is being done in this PR wouldn't be needed anymore I guess? Is there anything that I/we can start looking at or maybe we can even contribute? |
I think the solution @andybryant proposed in #165 seems reasonable, since even with single-message-transforms the eventual state is e.g. for an unsigned |
Based upon the theory provided in the official jdbc documentation, I have added a condition to include both Short and Byte as INT8 types in ConnectSchema.
Here's the doc:
8.3.5 SMALLINT
The JDBC type SMALLINT represents a 16-bit signed integer value between -32768 and 32767.
The corresponding SQL type, SMALLINT, is defined in SQL-92 and is supported by all the major databases. The SQL-92 standard leaves the precision of SMALLINT up to the implementation, but in practice, all the major databases support at least 16 bits.
The recommended Java mapping for the JDBC SMALLINT type is as a Java short.
This would allow kafka-connect-jdbc to allow both signed and unsigned values. Currently it fails for unsigned values(i.e values > 127). In this PR, if the value of tinyint value received from DB is greater than 127, then it converts to a short otherwise byte. This has been taken care of in the kafka project and schema-registry project.