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

KYLIN-4259 fix potential NPE bug reported by FindBugs #946

Merged
merged 2 commits into from Dec 17, 2019

Conversation

@caidezhi
Copy link
Contributor

caidezhi commented Nov 18, 2019

I run FindsBugs against Kylin codebase and find some potential NPE bugs.
related class:
org.apache.kylin.metadata.model.PartitionDesc
org.apache.kylin.engine.mr.common.CuboidStatsReaderUtil
org.apache.kylin.rest.controller.StreamingV2Controller
org.apache.kylin.common.StorageURL

@asf-ci

This comment has been minimized.

Copy link

asf-ci commented Nov 18, 2019

Can one of the admins verify this patch?

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Nov 18, 2019

Codecov Report

Merging #946 into master will decrease coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #946      +/-   ##
============================================
- Coverage     25.58%   25.57%   -0.01%     
+ Complexity     6140     6138       -2     
============================================
  Files          1412     1412              
  Lines         85072    85073       +1     
  Branches      11928    11927       -1     
============================================
- Hits          21762    21754       -8     
- Misses        61205    61211       +6     
- Partials       2105     2108       +3
Impacted Files Coverage Δ Complexity Δ
...e/kylin/rest/controller/StreamingV2Controller.java 0% <0%> (ø) 0 <0> (ø) ⬇️
.../kylin/engine/mr/common/CuboidStatsReaderUtil.java 0% <0%> (ø) 0 <0> (ø) ⬇️
.../main/java/org/apache/kylin/common/StorageURL.java 64.63% <0%> (ø) 20 <0> (ø) ⬇️
...org/apache/kylin/metadata/model/PartitionDesc.java 65.24% <0%> (ø) 18 <0> (ø) ⬇️
...he/kylin/dict/lookup/cache/RocksDBLookupTable.java 72.97% <0%> (-5.41%) 6% <0%> (-1%)
...org/apache/kylin/rest/util/QueryRequestLimits.java 35.71% <0%> (-4.77%) 5% <0%> (-1%)
...he/kylin/job/impl/threadpool/DefaultScheduler.java 74.41% <0%> (-2.33%) 12% <0%> (ø)
.../apache/kylin/cube/cuboid/TreeCuboidScheduler.java 63.84% <0%> (-2.31%) 0% <0%> (ø)
...a/org/apache/kylin/dict/Number2BytesConverter.java 81.74% <0%> (-0.8%) 17% <0%> (-1%)
...core/storage/columnar/GeneralColumnDataReader.java 94.73% <0%> (+5.26%) 7% <0%> (+1%) ⬆️

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 6b853ce...d3e5940. Read the comment docs.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Nov 18, 2019

Pull Request Test Coverage Report for Build 5235

  • 1 of 5 (20.0%) changed or added relevant lines in 4 files are covered.
  • 6 unchanged lines in 4 files lost coverage.
  • Overall coverage decreased (-0.005%) to 28.052%

Changes Missing Coverage Covered Lines Changed/Added Lines %
engine-mr/src/main/java/org/apache/kylin/engine/mr/common/CuboidStatsReaderUtil.java 0 1 0.0%
server-base/src/main/java/org/apache/kylin/rest/controller/StreamingV2Controller.java 0 1 0.0%
core-metadata/src/main/java/org/apache/kylin/metadata/model/PartitionDesc.java 0 2 0.0%
Files with Coverage Reduction New Missed Lines %
core-dictionary/src/main/java/org/apache/kylin/dict/lookup/cache/RocksDBLookupTable.java 1 81.08%
core-metadata/src/main/java/org/apache/kylin/metadata/model/PartitionDesc.java 1 70.73%
core-cube/src/main/java/org/apache/kylin/cube/cuboid/TreeCuboidScheduler.java 2 68.46%
core-job/src/main/java/org/apache/kylin/job/impl/threadpool/DefaultScheduler.java 2 80.23%
Totals Coverage Status
Change from base Build 5229: -0.005%
Covered Lines: 23865
Relevant Lines: 85073

💛 - Coveralls
Copy link
Contributor

nichunen left a comment

LGTM

@nichunen nichunen merged commit eca8c98 into apache:master Dec 17, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.