apache / orc

Apache ORC - the smallest, fastest columnar storage for Hadoop workloads
https://orc.apache.org/
Apache License 2.0
675 stars 480 forks source link

ORC-1614: Use `directOut.put(out)` instead of `directOut.put(out.array())` in `TestBrotli` test #1790

Closed cxzl25 closed 6 months ago

cxzl25 commented 7 months ago

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.

      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

deshanxiao commented 7 months ago

Good catch, Thanks!

In fact, I made a wrong decision to directly put the array into directOut since I misjudged the length of the array.

I prefer to change directOut.put(out.array()); to directOut.put(out);

What do you think? @cxzl25

dongjoon-hyun commented 6 months ago
deshanxiao commented 6 months ago

Thanks @dongjoon-hyun , I have updated the PR title and description.

deshanxiao commented 6 months ago

Merged to main/2.1. Thank you, @cxzl25 @paliwalashish @dongjoon-hyun !

dongjoon-hyun commented 6 months ago

Great! Thank you all!

dongjoon-hyun commented 6 months ago

Hi, @deshanxiao . branch-2.0 didn't have this patch. Did you use this merge script?

Screenshot 2024-02-09 at 19 29 40

dongjoon-hyun commented 6 months ago

Please note that main is Apache ORC 2.1.0.

deshanxiao commented 6 months ago

Thanks @dongjoon-hyun , I have changed the comment and jira.

dongjoon-hyun commented 6 months ago

Oh, we had better backport this to branch-2.0, didn't we?

deshanxiao commented 6 months ago

Thanks @dongjoon-hyun Cherry-pick the fix to branch-2.0