[ https://issues.apache.org/jira/browse/THRIFT-1035?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12980300#action_12980300 ]
Mathias Herberts commented on THRIFT-1035: ------------------------------------------ If we go the way you suggest, how do you imagine the following scenario: struct A { 1: map<string,binary> bin_map, } A a = new A(); a.putToBin_map("foo", "bar".getBytes()); // At that point you might have wrapped the value into a ByteBuffer, but then: a.getBin_map().put("foo", "foobar".getBytes()); // Won't be caught by Thrift and thus the internal ByteBuffer representation will be broken. I think it's better we use ByteBuffer internally for binary fields that are not included in collections but byte[] for those that are. If you really want to have ByteBuffers, you can always use struct BinaryWrapper { 1: binary content, } and declare collections which contain BinaryWrapper, this will add an additional Object but might still lead to better performance for large arrays. > Container types containing binary data are parameterized with ByteBuffer in > the generated Java code > --------------------------------------------------------------------------------------------------- > > Key: THRIFT-1035 > URL: https://issues.apache.org/jira/browse/THRIFT-1035 > Project: Thrift > Issue Type: Bug > Components: Java - Compiler, Java - Library > Affects Versions: 0.4, 0.5, 0.6, 0.7 > Environment: All > Reporter: Mathias Herberts > Attachments: THRIFT-1035-2.patch, THRIFT-1035.patch > > > Since THRIFT-830, binary fields are internally handled using ByteBuffer. > Release 0.4.0 was the first to expose the ByteBuffer to the outside world > (replacing previous methods returning/accepting byte[]). > THRIFT-882 lead to the methods accepting/returning byte[] being available > again, as it was deemed more reasonable not to expose the ByteBuffer too much > as their use could be cumbersome. This lead to 0.5.0 being backward > compatible with 0.3.0 on the binary fields front. > During that time, nobody noticed that container types that contained binary > data had their generated Java code changed to collections parameterized with > ByteBuffer instead of byte[]. > list<binary> -> List<ByteBuffer> > set<binary> -> Set<ByteBuffer> > map<binary,...> -> Map<ByteBuffer,...> > map<...,binary> -> Map<...,ByteBuffer> > This introduces confusion in the API and still exposes ByteBuffer when > discussion on THRIFT-882 concluded this should be avoided. > We need to provide a way to offer the original parameterization with byte[] > as this will simplify working with that type of collection and thus will > increase the odds of Thrift's adoption. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.