ORC-1614: Use directOut.put(out) instead of directOut.put(out.array()) in TestBrotli test
#1790
Conversation
| out.flip(); | ||
| // copy heap buffer to direct buffer. | ||
| directOut.put(out.array()); | ||
| directOut.flip(); | ||
| directOut.limit(position); |
There was a problem hiding this comment.
May be I didn't understand the issue correctly, but from documentation of flip(), limit is set by it.
"Flips this buffer. The limit is set to the current position and then the position is set to zero"
There was a problem hiding this comment.
out is compressed by brotliCodec, and the length after compression is 217, but the out.array() used here is still the 10000 length of out, so the length that can be read after directOut.flip() is 10000, not 217
There was a problem hiding this comment.
Makes sense. Thanks !
|
Good catch, Thanks! In fact, I made a wrong decision to directly put the array into I prefer to change What do you think? @cxzl25 |
|
deshanxiao
changed the title
TestBrotli testdirectOut.put(out) instead of directOut.put(out.array()) in TestBrotli test
|
Thanks @dongjoon-hyun , I have updated the PR title and description. |
|
Merged to main/2.1. Thank you, @cxzl25 @paliwalashish @dongjoon-hyun ! |
|
Great! Thank you all! |
|
Hi, @deshanxiao .
|
|
Please note that |
|
Thanks @dongjoon-hyun , I have changed the comment and jira. |
|
Oh, we had better backport this to branch-2.0, didn't we? |
…y())` in `TestBrotli` test
### What changes were proposed in this pull request?
Set ByteBuffer limit in `TestBrotli` test
### Why are the changes needed?
`TestBrotli#testDirectDecompress` attempts to put the compressed result in direct ByteBuffer.
When calling `decompress`, we will find that the input buffer length is still `10000` not `217` since we put the array without `limit` on the length.
```java
directOut.put(out.array());
directOut.flip();
```
This PR is aimed to use `directOut.put(out)` instead of `directOut.put(out.array())`
### How was this patch tested?
GA
### Was this patch authored or co-authored using generative AI tooling?
No
Closes #1790 from cxzl25/ORC-1614.
Authored-by: sychen <sychen@ctrip.com>
Signed-off-by: deshanxiao <deshanxiao@microsoft.com>
|
Thanks @dongjoon-hyun Cherry-pick the fix to |
What changes were proposed in this pull request?
Set ByteBuffer limit in
TestBrotlitestWhy are the changes needed?
TestBrotli#testDirectDecompressattempts to put the compressed result in direct ByteBuffer.When calling
decompress, we will find that the input buffer length is still10000not217since we put the array withoutlimiton the length.This PR is aimed to use
directOut.put(out)instead ofdirectOut.put(out.array())How was this patch tested?
GA
Was this patch authored or co-authored using generative AI tooling?
No