Skip to content

Conversation

@richardstartin
Copy link
Member

@richardstartin richardstartin commented Apr 14, 2022

Stop discovering private scalar functions to avoid unpredictable illegal reflective access.

This is a breaking change; if a user has a non-public method annotated with @ScalarFunction then it will no longer be evaluated. The migration path is for the user to make the appropriate methods public, since they own the code.

@codecov-commenter
Copy link

Codecov Report

Merging #8544 (4596643) into master (900f01f) will decrease coverage by 41.30%.
The diff coverage is 0.00%.

@@              Coverage Diff              @@
##             master    #8544       +/-   ##
=============================================
- Coverage     70.65%   29.35%   -41.31%     
=============================================
  Files          1674     1669        -5     
  Lines         87743    87602      -141     
  Branches      13279    13282        +3     
=============================================
- Hits          61999    25713    -36286     
- Misses        21439    59508    +38069     
+ Partials       4305     2381     -1924     
Flag Coverage Δ
integration1 27.22% <0.00%> (+0.23%) ⬆️
integration2 25.89% <0.00%> (-0.22%) ⬇️
unittests1 ?
unittests2 ?

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

Impacted Files Coverage Δ
...org/apache/pinot/common/function/FunctionInfo.java 87.50% <ø> (-12.50%) ⬇️
.../apache/pinot/common/function/FunctionInvoker.java 67.44% <ø> (-12.11%) ⬇️
...apache/pinot/common/function/FunctionRegistry.java 88.88% <0.00%> (-5.23%) ⬇️
.../java/org/apache/pinot/spi/utils/BooleanUtils.java 0.00% <0.00%> (-100.00%) ⬇️
...java/org/apache/pinot/spi/trace/BaseRecording.java 0.00% <0.00%> (-100.00%) ⬇️
...java/org/apache/pinot/spi/trace/NoOpRecording.java 0.00% <0.00%> (-100.00%) ⬇️
...ava/org/apache/pinot/spi/config/table/FSTType.java 0.00% <0.00%> (-100.00%) ⬇️
...ava/org/apache/pinot/spi/data/MetricFieldSpec.java 0.00% <0.00%> (-100.00%) ⬇️
...va/org/apache/pinot/spi/utils/BigDecimalUtils.java 0.00% <0.00%> (-100.00%) ⬇️
...java/org/apache/pinot/common/tier/TierFactory.java 0.00% <0.00%> (-100.00%) ⬇️
... and 1197 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 900f01f...4596643. Read the comment docs.

@richardstartin
Copy link
Member Author

It appears illegal reflective access is relied on even internally

@richardstartin richardstartin marked this pull request as ready for review April 14, 2022 18:52
@richardstartin richardstartin added release-notes Referenced by PRs that need attention when compiling the next release notes jdk17 breaking-change labels Apr 14, 2022
@richardstartin richardstartin changed the title only discover public functions Only discover public methods annotated with @ScalarFunction Apr 14, 2022
@siddharthteotia siddharthteotia merged commit d503d50 into apache:master Apr 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-change jdk17 release-notes Referenced by PRs that need attention when compiling the next release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants