Skip to content

Comments

KYLIN-3847 Flink cubing step : fact distinct#1154

Merged
shaofengshi merged 1 commit intoapache:masterfrom
harveyyue:KYLIN-3847
Apr 28, 2020
Merged

KYLIN-3847 Flink cubing step : fact distinct#1154
shaofengshi merged 1 commit intoapache:masterfrom
harveyyue:KYLIN-3847

Conversation

@harveyyue
Copy link

Refer jira: https://issues.apache.org/jira/browse/KYLIN-3847
Add flink fact distinct columns step, to avoid a mass of duplicated code with Spark and Flink implement self fact distinct step, add base class FactDistinctColumnsBase, extract core methods and variables from mapper/reducer class.
About different engine will implement the abstract method FactDistinctColumnsBase.Visitor.collect(...) to output the result from Spark/Flink engine framework.
Please help to review this code changes

@lgtm-com
Copy link

lgtm-com bot commented Mar 10, 2020

This pull request introduces 1 alert when merging bc5a7b8 into ebc4c26 - view on LGTM.com

new alerts:

  • 1 for Boxed variable is never null

@codecov-io
Copy link

codecov-io commented Mar 10, 2020

Codecov Report

Merging #1154 into master will decrease coverage by 0.14%.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1154      +/-   ##
============================================
- Coverage     25.05%   24.91%   -0.15%     
+ Complexity     6248     6245       -3     
============================================
  Files          1446     1449       +3     
  Lines         88305    88803     +498     
  Branches      12357    12413      +56     
============================================
- Hits          22128    22124       -4     
- Misses        64007    64506     +499     
- Partials       2170     2173       +3
Impacted Files Coverage Δ Complexity Δ
...kylin/engine/mr/steps/FactDistinctColumnsBase.java 0% <0%> (ø) 0 <0> (?)
...ylin/engine/flink/FlinkBatchCubingJobBuilder2.java 0% <0%> (ø) 0 <0> (ø) ⬇️
.../java/org/apache/kylin/common/KylinConfigBase.java 12.24% <0%> (-0.02%) 44 <0> (ø)
.../java/org/apache/kylin/common/util/StringUtil.java 40.81% <0%> (-0.43%) 22 <0> (ø)
...kylin/engine/flink/HadoopMultipleOutputFormat.java 0% <0%> (ø) 0 <0> (?)
...e/kylin/engine/flink/FlinkFactDistinctColumns.java 0% <0%> (ø) 0 <0> (?)
...he/kylin/dict/lookup/cache/RocksDBLookupTable.java 72.97% <0%> (-5.41%) 6% <0%> (-1%)
...a/org/apache/kylin/dict/Number2BytesConverter.java 81.74% <0%> (-0.8%) 17% <0%> (-1%)
...ain/java/org/apache/kylin/dict/TrieDictionary.java 72.55% <0%> (-0.47%) 56% <0%> (-1%)
... and 2 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 ebc4c26...bc5a7b8. Read the comment docs.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 5731

  • 0 of 504 (0.0%) changed or added relevant lines in 6 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.2%) to 27.364%

Changes Missing Coverage Covered Lines Changed/Added Lines %
core-common/src/main/java/org/apache/kylin/common/util/StringUtil.java 0 1 0.0%
core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java 0 6 0.0%
engine-flink/src/main/java/org/apache/kylin/engine/flink/FlinkBatchCubingJobBuilder2.java 0 22 0.0%
engine-flink/src/main/java/org/apache/kylin/engine/flink/HadoopMultipleOutputFormat.java 0 33 0.0%
engine-flink/src/main/java/org/apache/kylin/engine/flink/FlinkFactDistinctColumns.java 0 157 0.0%
engine-mr/src/main/java/org/apache/kylin/engine/mr/steps/FactDistinctColumnsBase.java 0 285 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%
Totals Coverage Status
Change from base Build 5720: -0.2%
Covered Lines: 24300
Relevant Lines: 88803

💛 - Coveralls

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.

+1 LGTM

@shaofengshi shaofengshi merged commit 3b4db4a into apache:master Apr 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants