Skip to content
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

Fix all typed ArrayList elements() method usage #13354

Merged
merged 1 commit into from
Jun 11, 2024

Conversation

xiangfu0
Copy link
Contributor

@xiangfu0 xiangfu0 commented Jun 10, 2024

Bug fixing for {Int|Long|Float|Double|Object}ArrayList#elements() call usage.

The ArrayList#elements() returned an array may be longer than the actual size of the ArrayList, and the actual size of the ArrayList can be retrieved using ArrayList#size().

Created a util class ArrayListUtils to best effort extract the given ArrayList to the corresponding primitive array without copying the elements.

E.g. this method ArrayListUtils#toIntArray() checks the length of the returned int array and returns the same if it is equal to the size of the input IntArrayList, otherwise, it copies the elements to a new int array and returns it.

@codecov-commenter
Copy link

codecov-commenter commented Jun 10, 2024

Codecov Report

Attention: Patch coverage is 0% with 37 lines in your changes missing coverage. Please review.

Project coverage is 0.00%. Comparing base (59551e4) to head (1357dc8).
Report is 599 commits behind head on master.

Files Patch % Lines
.../org/apache/pinot/common/utils/ArrayListUtils.java 0.00% 16 Missing ⚠️
...java/org/apache/pinot/common/utils/DataSchema.java 0.00% 5 Missing ⚠️
...erator/blocks/results/AggregationResultsBlock.java 0.00% 5 Missing ⚠️
...e/operator/blocks/results/GroupByResultsBlock.java 0.00% 5 Missing ⚠️
.../pinot/query/runtime/operator/utils/TypeUtils.java 0.00% 5 Missing ⚠️
...egation/function/HistogramAggregationFunction.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             master   #13354       +/-   ##
=============================================
- Coverage     61.75%    0.00%   -61.76%     
=============================================
  Files          2436     2473       +37     
  Lines        133233   136279     +3046     
  Branches      20636    21171      +535     
=============================================
- Hits          82274        0    -82274     
- Misses        44911   136279    +91368     
+ Partials       6048        0     -6048     
Flag Coverage Δ
custom-integration1 ?
integration 0.00% <0.00%> (-0.01%) ⬇️
integration1 ?
integration2 0.00% <0.00%> (ø)
java-11 ?
java-21 0.00% <0.00%> (-61.63%) ⬇️
skip-bytebuffers-false 0.00% <0.00%> (-61.75%) ⬇️
skip-bytebuffers-true 0.00% <0.00%> (-27.73%) ⬇️
temurin 0.00% <0.00%> (-61.76%) ⬇️
unittests ?
unittests1 ?
unittests2 ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@Jackie-Jiang Jackie-Jiang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest adding a util class to handle these checks and return array without copy if possible

@xiangfu0 xiangfu0 force-pushed the fixing-arraylist-elements-usage branch 2 times, most recently from 6eb394a to a7f52f2 Compare June 11, 2024 00:44
@xiangfu0 xiangfu0 changed the title Fix [Type]ArrayList elements() method usage Fix all typed ArrayList elements() method usage Jun 11, 2024
@xiangfu0 xiangfu0 force-pushed the fixing-arraylist-elements-usage branch from a7f52f2 to 1357dc8 Compare June 11, 2024 08:04
@xiangfu0 xiangfu0 merged commit 5ca1d97 into apache:master Jun 11, 2024
20 checks passed
@xiangfu0 xiangfu0 deleted the fixing-arraylist-elements-usage branch June 11, 2024 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants