You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2020/06/09 16:06:30 UTC

[GitHub] [arrow] liyafan82 commented on pull request #7326: ARROW-9010: [Java] Framework and interface changes for RecordBatch IPC buffer compression

liyafan82 commented on pull request #7326:
URL: https://github.com/apache/arrow/pull/7326#issuecomment-640584596


   Thanks a lot for your good comments. Please see my reply in line. 
   
   > Looks good, few minor comments. It would be good to see this in a test (if that is reasonable/possible) w/ a NoOp compressor or something as its hard for me to envisoin how the compression actually will function.
   
   Sounds good. In fact, I have added some tests for NoOp compressor when preparing the PR. However, the changes are reverted before submitting the PR, because it would involve changes to the Message.fbs file. IMO, we should be careful when changing this file, as it involves protocol changes, and may break the code for other languages. 
   
   > 
   > Also, excuse my ignorance of the compression scheme but is there any negotiation of the compresson scheme or an opt-out for older clients? A client recieving a compressed buffer without taking it into account will likely crash hard.
   
   I agree with you that compression scheme negotiation is a useful and widely used feature. Maybe we will support it in the futrue, but not this this PR. According to the discussion in the ML, this issue only deals with some standard compression schemes, and these schemes are encoded in the protocol and should be supported universally. 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org