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

[Multi-stage] Clean up RelNode to Operator handling #13325

Merged
merged 1 commit into from
Jun 7, 2024

Conversation

Jackie-Jiang
Copy link
Contributor

Clean up the flow from RelNode from planner side to MultiStageOperator on the executor side:

  • Cleanup unnecessary fields and conversions in PlanNode, and keep it close to information from Calcite RelNode
  • Unify the constructor for PlanNode
  • Cleanup plan.proto to reflect the changes in PlanNode
  • Cleanup MultiStageOperator fields and unnecessary conversions
  • Reduce the memory allocation
  • Cleanup all the MultiStageOperator tests

@Jackie-Jiang Jackie-Jiang added enhancement backward-incompat Referenced by PRs that introduce or fix backward compat issues cleanup refactor multi-stage Related to the multi-stage query engine labels Jun 6, 2024
@codecov-commenter
Copy link

codecov-commenter commented Jun 7, 2024

Codecov Report

Attention: Patch coverage is 71.44120% with 323 lines in your changes missing coverage. Please review.

Project coverage is 62.00%. Comparing base (59551e4) to head (eaec88c).
Report is 565 commits behind head on master.

Files Patch % Lines
...inot/query/planner/serde/PlanNodeDeserializer.java 69.14% 41 Missing and 13 partials ⚠️
...hysical/colocated/GreedyShuffleRewriteVisitor.java 0.00% 35 Missing ⚠️
.../pinot/query/planner/serde/PlanNodeSerializer.java 72.95% 26 Missing and 7 partials ⚠️
...che/pinot/query/planner/logical/RexExpression.java 27.27% 6 Missing and 10 partials ⚠️
...che/pinot/query/planner/plannode/ExchangeNode.java 43.47% 13 Missing ⚠️
...not/query/planner/plannode/MailboxReceiveNode.java 57.69% 4 Missing and 7 partials ⚠️
.../pinot/query/planner/plannode/MailboxSendNode.java 54.16% 4 Missing and 7 partials ⚠️
...pache/pinot/query/planner/plannode/WindowNode.java 61.53% 4 Missing and 6 partials ⚠️
...operator/window/value/LeadValueWindowFunction.java 0.00% 10 Missing ⚠️
.../query/planner/logical/RelToPlanNodeConverter.java 91.74% 4 Missing and 5 partials ⚠️
... and 29 more
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #13325      +/-   ##
============================================
+ Coverage     61.75%   62.00%   +0.25%     
+ Complexity      207      198       -9     
============================================
  Files          2436     2545     +109     
  Lines        133233   139570    +6337     
  Branches      20636    21648    +1012     
============================================
+ Hits          82274    86545    +4271     
- Misses        44911    46480    +1569     
- Partials       6048     6545     +497     
Flag Coverage Δ
custom-integration1 <0.01% <0.00%> (-0.01%) ⬇️
integration <0.01% <0.00%> (-0.01%) ⬇️
integration1 <0.01% <0.00%> (-0.01%) ⬇️
integration2 0.00% <0.00%> (ø)
java-11 61.98% <71.44%> (+0.27%) ⬆️
java-21 61.89% <71.44%> (+0.27%) ⬆️
skip-bytebuffers-false 61.99% <71.44%> (+0.25%) ⬆️
skip-bytebuffers-true 61.88% <71.44%> (+34.15%) ⬆️
temurin 62.00% <71.44%> (+0.25%) ⬆️
unittests 62.00% <71.44%> (+0.25%) ⬆️
unittests1 46.52% <71.44%> (-0.37%) ⬇️
unittests2 27.76% <0.00%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Jackie-Jiang Jackie-Jiang merged commit 4356583 into apache:master Jun 7, 2024
19 of 20 checks passed
@Jackie-Jiang Jackie-Jiang deleted the cleanup_plan_ser_de branch June 7, 2024 17:30
gortiz pushed a commit to gortiz/pinot that referenced this pull request Jun 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backward-incompat Referenced by PRs that introduce or fix backward compat issues cleanup enhancement multi-stage Related to the multi-stage query engine refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants