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

feat: add avg, count, min and max ops in runner #1627

Merged
merged 5 commits into from
Apr 19, 2022

Conversation

zhanghaohit
Copy link
Collaborator

close #1244
close #1245
close #1246
close #1247

@zhanghaohit zhanghaohit added the enhancement New feature or request label Apr 14, 2022
@zhanghaohit zhanghaohit added this to the v0.5 milestone Apr 14, 2022
@zhanghaohit zhanghaohit self-assigned this Apr 14, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Apr 14, 2022

SDK Test Report

  72 files    72 suites   6m 6s ⏱️
172 tests 169 ✔️ 3 💤 0
212 runs  209 ✔️ 3 💤 0

Results for commit 77ee9bf.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Apr 14, 2022

Linux Test Report

       56 files       183 suites   48m 45s ⏱️
  8 218 tests   8 218 ✔️ 0 💤 0
12 134 runs  12 134 ✔️ 0 💤 0

Results for commit 77ee9bf.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Apr 14, 2022

HybridSE Linux Test Report

       66 files       234 suites   5m 41s ⏱️
18 923 tests 18 921 ✔️ 2 💤 0

Results for commit 77ee9bf.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Apr 14, 2022

HybridSE Mac Test Report

       66 files       234 suites   6m 47s ⏱️
18 923 tests 18 921 ✔️ 2 💤 0

Results for commit 77ee9bf.

♻️ This comment has been updated with latest results.

@zhanghaohit
Copy link
Collaborator Author

@zhanghaohit zhanghaohit force-pushed the feat/more-aggregator-op branch from ecb2dad to b3f6280 Compare April 18, 2022 02:44
@codecov
Copy link

codecov bot commented Apr 18, 2022

Codecov Report

Merging #1627 (77ee9bf) into main (c139d6d) will increase coverage by 0.33%.
The diff coverage is 92.80%.

@@             Coverage Diff              @@
##               main    #1627      +/-   ##
============================================
+ Coverage     67.42%   67.76%   +0.33%     
  Complexity      323      323              
============================================
  Files           607      608       +1     
  Lines        115890   116954    +1064     
  Branches       1000     1000              
============================================
+ Hits          78141    79250    +1109     
+ Misses        37542    37497      -45     
  Partials        207      207              
Impacted Files Coverage Δ
hybridse/include/codec/fe_row_codec.h 86.95% <ø> (ø)
hybridse/include/vm/schemas_context.h 100.00% <ø> (ø)
hybridse/src/vm/runner.h 92.80% <ø> (ø)
hybridse/src/vm/runner.cc 67.78% <86.36%> (+0.25%) ⬆️
hybridse/src/vm/aggregator.h 87.66% <86.89%> (+1.39%) ⬆️
hybridse/src/vm/aggregator_vm_test.cc 88.65% <88.65%> (ø)
src/catalog/tablet_catalog_test.cc 95.95% <92.85%> (+0.04%) ⬆️
src/cmd/sql_cmd_test.cc 99.53% <98.92%> (-0.47%) ⬇️
hybridse/src/codec/fe_row_codec.cc 78.49% <100.00%> (+0.10%) ⬆️
...ridse/src/passes/physical/long_window_optimized.cc 79.19% <100.00%> (+0.28%) ⬆️
... and 46 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 c139d6d...77ee9bf. Read the comment docs.

@@ -231,7 +231,9 @@ bool LongWindowOptimized::OptimizeWithPreAggr(vm::PhysicalAggrerationNode* in, i
LOG(ERROR) << "Fail to create PhysicalReduceAggregationNode: " << status;
return false;
}
LOG(INFO) << "[LongWindowOptimized] Before transform sql:\n" << (*output)->GetTreeString();
Copy link
Collaborator

Choose a reason for hiding this comment

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

DEBUG log ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Forgot to discuss this in the PR review.

For testing and verification, we need some log information to see whether we are really using long_window optimization or not.

Since the log will only print during deploy, it is kind of acceptable. What do you think?

If it is too annoying, can change to Debug log.

Copy link
Collaborator

Choose a reason for hiding this comment

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

if only in deploy, it's no matter

@zhanghaohit zhanghaohit merged commit e2b0b51 into 4paradigm:main Apr 19, 2022
@lumianph lumianph mentioned this pull request May 13, 2022
39 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
3 participants