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.

Sign up for GitHub

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

Jump to bottom

Apache kylin 3359 #138

Merged
merged 10 commits into from Jun 14, 2018
Merged

Apache kylin 3359 #138

merged 10 commits into from Jun 14, 2018

Conversation

kyotoYaho
Copy link
Contributor

No description provided.

@asfgit
Copy link

asfgit commented May 7, 2018

Can one of the admins verify this patch?

@coveralls
Copy link

coveralls commented May 7, 2018 •
edited

Pull Request Test Coverage Report for Build 3159

  • 338 of 1357 (24.91%) changed or added relevant lines in 52 files are covered.
  • 136 unchanged lines in 16 files lost coverage.
  • Overall coverage increased (+0.4%) to 24.246%
Changes Missing Coverage Covered Lines Changed/Added Lines %
core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java 0 1 0.0%
core-metadata/src/main/java/org/apache/kylin/metadata/datatype/DataType.java 0 1 0.0%
core-storage/src/main/java/org/apache/kylin/storage/gtrecord/SegmentCubeTupleIterator.java 0 1 0.0%
core-metadata/src/main/java/org/apache/kylin/metadata/model/FunctionDesc.java 0 1 0.0%
core-cube/src/main/java/org/apache/kylin/cube/gridtable/CubeCodeSystem.java 4 5 80.0%
core-storage/src/main/java/org/apache/kylin/storage/gtrecord/CubeSegmentScanner.java 0 2 0.0%
core-metadata/src/main/java/org/apache/kylin/measure/basic/BigDecimalSumAggregator.java 4 6 66.67%
core-cube/src/main/java/org/apache/kylin/gridtable/GTInfo.java 3 5 60.0%
core-storage/src/main/java/org/apache/kylin/storage/StorageContext.java 0 3 0.0%
core-metadata/src/main/java/org/apache/kylin/metadata/expression/ExpressionCountDistributor.java 51 55 92.73%
Files with Coverage Reduction New Missed Lines %
core-storage/src/main/java/org/apache/kylin/storage/gtrecord/CubeSegmentScanner.java 1 0.0%
query/src/main/java/org/apache/kylin/query/relnode/OLAPProjectRel.java 1 0.0%
query/src/main/java/org/apache/kylin/query/relnode/OLAPUnionRel.java 1 0.0%
query/src/main/java/org/apache/kylin/query/relnode/OLAPTableScan.java 1 0.0%
core-cube/src/main/java/org/apache/kylin/cube/gridtable/CuboidToGridTableMapping.java 1 50.44%
core-storage/src/main/java/org/apache/kylin/storage/gtrecord/GTCubeStorageQueryBase.java 1 0.0%
core-storage/src/main/java/org/apache/kylin/storage/StorageContext.java 1 0.0%
core-metadata/src/main/java/org/apache/kylin/metadata/model/FunctionDesc.java 1 0.0%
core-cube/src/main/java/org/apache/kylin/cube/inmemcubing/MemDiskStore.java 1 78.12%
query/src/main/java/org/apache/kylin/query/relnode/OLAPContext.java 2 0.0%
Totals Coverage Status
Change from base Build 3142: 0.4%
Covered Lines: 14722
Relevant Lines: 60719
💛 - Coveralls

@codecov-io
Copy link

codecov-io commented May 7, 2018 •
edited

Codecov Report

Merging #138 into master will increase coverage by 0.26%.
The diff coverage is 20.7%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #138      +/-   ##
============================================
+ Coverage     21.86%   22.13%   +0.26%     
- Complexity     3879     4003     +124     
============================================
  Files           985     1005      +20     
  Lines         59641    60719    +1078     
  Branches       8608     8765     +157     
============================================
+ Hits          13041    13438     +397     
- Misses        45398    45999     +601     
- Partials       1202     1282      +80
Impacted Files Coverage Δ Complexity Δ
...che/kylin/cube/gridtable/ScanRangePlannerBase.java 0% <ø> (ø) 0 <0> (ø) ⬇️
.../org/apache/kylin/query/relnode/OLAPFilterRel.java 0% <ø> (ø) 0 <0> (ø) ⬇️
.../kylin/metadata/datatype/DynamicDimSerializer.java 0% <0%> (ø) 0 <0> (?)
.../org/apache/kylin/query/relnode/ColumnRowType.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...kylin/cube/gridtable/CuboidToGridTableMapping.java 45.13% <0%> (-0.41%) 17 <0> (ø)
...lin/metadata/expression/StringTupleExpression.java 0% <0%> (ø) 0 <0> (?)
...org/apache/kylin/query/relnode/OLAPProjectRel.java 0% <0%> (ø) 0 <0> (ø) ⬇️
.../org/apache/kylin/metadata/model/FunctionDesc.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...che/kylin/storage/gtrecord/CubeSegmentScanner.java 0% <0%> (ø) 0 <0> (ø) ⬇️
.../java/org/apache/kylin/storage/StorageContext.java 0% <0%> (ø) 0 <0> (ø) ⬇️
... and 80 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 5611760...388d3c0. Read the comment docs.

@kyotoYaho kyotoYaho force-pushed the APACHE-KYLIN-3359 branch 2 times, most recently from e7fea71 to 12914b7 Compare May 14, 2018 12:28
shaofengshi
shaofengshi reviewed May 20, 2018
View reviewed changes
core-metadata/src/main/java/org/apache/kylin/metadata/model/SumDynamicFunctionDesc.java
public SumDynamicFunctionDesc(ParameterDesc parameter, TupleExpression tupleExpression) {
super(parameter, tupleExpression);
setExpression(FUNC_SUM);
setReturnType("decimal");
Copy link
Contributor

Choose a reason for hiding this comment

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

Is "decimal" hard-coded?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it's hard coded. For simplification, all of the datatype for the number expressions are changed to bigdecimal. Finally, the returned value will be converted in {{Tuple.setMeasureValue()}} according to the return type.

shaofengshi
shaofengshi reviewed May 21, 2018
View reviewed changes
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.

CI passed with "kylin.query.enable-dynamic-column=true" http://34.226.50.254:8081/job/kylin-full-regression/134/

core-metadata/src/main/java/org/apache/kylin/metadata/expression/ExpressionVisitor.java Outdated
package org.apache.kylin.metadata.expression;

public interface ExpressionVisitor {
public TupleExpression visitNumber(NumberTupleExpression numExpr);
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't need "public" for the method in an interface.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll remove the public.

shaofengshi
shaofengshi approved these changes Jun 14, 2018
View reviewed changes
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.

Passed auto test and manual test, don't see major issue now.

@shaofengshi shaofengshi merged commit a9557a6 into apache:master Jun 14, 2018
hit-lacus pushed a commit to hit-lacus/kylin that referenced this pull request Jun 15, 2020
@RupengWang @nichunen
apache#138 Fix query full build cube got NullPointerException
e940ea2
hit-lacus pushed a commit that referenced this pull request Jun 15, 2020
@RupengWang @hit-lacus
#138 Fix query full build cube got NullPointerException
80e88ae
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shaofengshi shaofengshi shaofengshi approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
5 participants
@kyotoYaho @asfgit @coveralls @codecov-io @shaofengshi