You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "walterddr (via GitHub)" <gi...@apache.org> on 2023/11/28 05:56:26 UTC
[PR] [multistage] don't prune project after agg [pinot]
walterddr opened a new pull request, #12058:
URL: https://github.com/apache/pinot/pull/12058
this should fix #12013
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] [multistage] don't prune project after agg during relBuilder [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #12058:
URL: https://github.com/apache/pinot/pull/12058#discussion_r1408566632
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/QueryEnvironment.java:
##########
@@ -122,7 +122,8 @@ public QueryEnvironment(TypeFactory typeFactory, CalciteSchema rootSchema, Worke
// SUB-QUERY Threshold is useless as we are encoding all IN clause in-line anyway
.withInSubQueryThreshold(Integer.MAX_VALUE)
.addRelBuilderConfigTransform(c -> c.withPushJoinCondition(true))
- .addRelBuilderConfigTransform(c -> c.withAggregateUnique(true)))
+ .addRelBuilderConfigTransform(c -> c.withAggregateUnique(true))
+ .addRelBuilderConfigTransform(c -> c.withPruneInputOfAggregate(false)))
Review Comment:
(minor) Can we chain them as `.addRelBuilderConfigTransform(c -> c.withPushJoinCondition(true).withAggregateUnique(true).withPruneInputOfAggregate(false))`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] [multistage] don't prune project after agg during relBuilder [pinot]
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr merged PR #12058:
URL: https://github.com/apache/pinot/pull/12058
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] [multistage] don't prune project after agg [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #12058:
URL: https://github.com/apache/pinot/pull/12058#issuecomment-1829198348
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/12058?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Comparison is base [(`f79b618`)](https://app.codecov.io/gh/apache/pinot/commit/f79b618141e07c140663791959d96956ad91d811?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 61.62% compared to head [(`3f260da`)](https://app.codecov.io/gh/apache/pinot/pull/12058?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 46.74%.
> Report is 2 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #12058 +/- ##
=============================================
- Coverage 61.62% 46.74% -14.89%
+ Complexity 1151 945 -206
=============================================
Files 2386 1788 -598
Lines 129567 93954 -35613
Branches 20053 15192 -4861
=============================================
- Hits 79847 43918 -35929
- Misses 43913 46906 +2993
+ Partials 5807 3130 -2677
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/12058/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/12058/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration](https://app.codecov.io/gh/apache/pinot/pull/12058/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/12058/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/12058/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-11](https://app.codecov.io/gh/apache/pinot/pull/12058/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-21](https://app.codecov.io/gh/apache/pinot/pull/12058/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `46.74% <100.00%> (-14.77%)` | :arrow_down: |
| [skip-bytebuffers-false](https://app.codecov.io/gh/apache/pinot/pull/12058/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `46.74% <100.00%> (-14.87%)` | :arrow_down: |
| [skip-bytebuffers-true](https://app.codecov.io/gh/apache/pinot/pull/12058/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/12058/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `46.74% <100.00%> (-14.89%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/12058/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `46.74% <100.00%> (-14.88%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/12058/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `46.74% <100.00%> (-0.20%)` | :arrow_down: |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/12058/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/pinot/pull/12058?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] [multistage] don't prune project after agg during relBuilder [pinot]
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #12058:
URL: https://github.com/apache/pinot/pull/12058#discussion_r1410047421
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/QueryEnvironment.java:
##########
@@ -122,7 +122,8 @@ public QueryEnvironment(TypeFactory typeFactory, CalciteSchema rootSchema, Worke
// SUB-QUERY Threshold is useless as we are encoding all IN clause in-line anyway
.withInSubQueryThreshold(Integer.MAX_VALUE)
.addRelBuilderConfigTransform(c -> c.withPushJoinCondition(true))
- .addRelBuilderConfigTransform(c -> c.withAggregateUnique(true)))
+ .addRelBuilderConfigTransform(c -> c.withAggregateUnique(true))
+ .addRelBuilderConfigTransform(c -> c.withPruneInputOfAggregate(false)))
Review Comment:
good point. just saw this, will clean up later
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org