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 3597 fix sonar issues #398

Merged
merged 1 commit into from
Dec 26, 2018
Merged

Kylin 3597 fix sonar issues #398

merged 1 commit into from
Dec 26, 2018

Conversation

matribots
Copy link

No description provided.

@asfgit
Copy link

asfgit commented Dec 17, 2018

Can one of the admins verify this patch?

CachedRowSet crs = new FixedCachedRowSetImpl();
crs.populate(resultSet);
return crs;
try (CachedRowSet crs = new FixedCachedRowSetImpl()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm afraid this will lead to error.

logger.info("Wild estimate of base aggr cache is " + mbEstimateBaseAggrCache + " MB");
int mbEstimateBaseAggrCache = (int) (aggregationScanner.getEstimateSizeOfAggrCache()
/ MemoryBudgetController.ONE_MB);
logger.info("Wild estimate of base aggr cache is " + mbEstimateBaseAggrCache + " MB");
Copy link
Contributor

Choose a reason for hiding this comment

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

We can use the "{}" to format the log output.

@codecov-io
Copy link

Codecov Report

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

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #398      +/-   ##
============================================
- Coverage     24.39%   24.38%   -0.01%     
+ Complexity     4935     4934       -1     
============================================
  Files          1143     1143              
  Lines         69259    69259              
  Branches       9859     9859              
============================================
- Hits          16897    16892       -5     
- Misses        50665    50668       +3     
- Partials       1697     1699       +2
Impacted Files Coverage Δ Complexity Δ
...che/kylin/cube/inmemcubing2/InMemCubeBuilder2.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...he/kylin/dict/lookup/cache/RocksDBLookupTable.java 72.97% <0%> (-5.41%) 6% <0%> (-1%)
...rg/apache/kylin/cube/inmemcubing/MemDiskStore.java 69.3% <0%> (-0.92%) 7% <0%> (ø)

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 1af62e4...37389bf. Read the comment docs.

Copy link
Contributor

@shaofengshi shaofengshi left a comment

Choose a reason for hiding this comment

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

LGTM

@shaofengshi shaofengshi merged commit 338a8f1 into apache:master Dec 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants