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 2024/02/01 18:11:42 UTC
[PR] [multistage][test] adding tests for IN/NOT-IN operation [pinot]
walterddr opened a new pull request, #12349:
URL: https://github.com/apache/pinot/pull/12349
follow up with #12305
--
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][test] adding tests for IN/NOT-IN operation [pinot]
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr merged PR #12349:
URL: https://github.com/apache/pinot/pull/12349
--
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][test] adding tests for IN/NOT-IN operation [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #12349:
URL: https://github.com/apache/pinot/pull/12349#issuecomment-1922070292
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/12349?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: `55 lines` in your changes are missing coverage. Please review.
> Comparison is base [(`17db0fd`)](https://app.codecov.io/gh/apache/pinot/commit/17db0fd17b688fe609b82697a5c3b3117d93cdba?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 61.69% compared to head [(`e16f071`)](https://app.codecov.io/gh/apache/pinot/pull/12349?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 61.62%.
> Report is 3 commits behind head on master.
| [Files](https://app.codecov.io/gh/apache/pinot/pull/12349?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...e/assignment/instance/InstanceTagPoolSelector.java](https://app.codecov.io/gh/apache/pinot/pull/12349?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL2Fzc2lnbm1lbnQvaW5zdGFuY2UvSW5zdGFuY2VUYWdQb29sU2VsZWN0b3IuamF2YQ==) | 32.35% | [22 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12349?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...nstance/InstanceReplicaGroupPartitionSelector.java](https://app.codecov.io/gh/apache/pinot/pull/12349?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL2Fzc2lnbm1lbnQvaW5zdGFuY2UvSW5zdGFuY2VSZXBsaWNhR3JvdXBQYXJ0aXRpb25TZWxlY3Rvci5qYXZh) | 94.58% | [9 Missing and 6 partials :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12349?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...query/runtime/operator/operands/FilterOperand.java](https://app.codecov.io/gh/apache/pinot/pull/12349?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS9vcGVyYXRvci9vcGVyYW5kcy9GaWx0ZXJPcGVyYW5kLmphdmE=) | 41.17% | [10 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12349?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...elix/core/periodictask/ControllerPeriodicTask.java](https://app.codecov.io/gh/apache/pinot/pull/12349?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3BlcmlvZGljdGFzay9Db250cm9sbGVyUGVyaW9kaWNUYXNrLmphdmE=) | 83.33% | [1 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12349?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ime/operator/operands/TransformOperandFactory.java](https://app.codecov.io/gh/apache/pinot/pull/12349?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS9vcGVyYXRvci9vcGVyYW5kcy9UcmFuc2Zvcm1PcGVyYW5kRmFjdG9yeS5qYXZh) | 50.00% | [0 Missing and 2 partials :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12349?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...mmon/assignment/InstanceAssignmentConfigUtils.java](https://app.codecov.io/gh/apache/pinot/pull/12349?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vYXNzaWdubWVudC9JbnN0YW5jZUFzc2lnbm1lbnRDb25maWdVdGlscy5qYXZh) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12349?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...e/pinot/controller/helix/SegmentStatusChecker.java](https://app.codecov.io/gh/apache/pinot/pull/12349?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9TZWdtZW50U3RhdHVzQ2hlY2tlci5qYXZh) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12349?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...assignment/instance/InstancePartitionSelector.java](https://app.codecov.io/gh/apache/pinot/pull/12349?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL2Fzc2lnbm1lbnQvaW5zdGFuY2UvSW5zdGFuY2VQYXJ0aXRpb25TZWxlY3Rvci5qYXZh) | 50.00% | [0 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12349?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #12349 +/- ##
============================================
- Coverage 61.69% 61.62% -0.08%
Complexity 207 207
============================================
Files 2424 2424
Lines 132340 132500 +160
Branches 20436 20479 +43
============================================
+ Hits 81651 81654 +3
- Misses 44693 44847 +154
- Partials 5996 5999 +3
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/12349/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/12349/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <0.00%> (-0.01%)` | :arrow_down: |
| [integration](https://app.codecov.io/gh/apache/pinot/pull/12349/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <0.00%> (-0.01%)` | :arrow_down: |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/12349/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <0.00%> (-0.01%)` | :arrow_down: |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/12349/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <0.00%> (ø)` | |
| [java-11](https://app.codecov.io/gh/apache/pinot/pull/12349/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `27.69% <81.17%> (-7.34%)` | :arrow_down: |
| [java-21](https://app.codecov.io/gh/apache/pinot/pull/12349/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `61.62% <84.55%> (+0.04%)` | :arrow_up: |
| [skip-bytebuffers-false](https://app.codecov.io/gh/apache/pinot/pull/12349/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `61.58% <84.55%> (-0.11%)` | :arrow_down: |
| [skip-bytebuffers-true](https://app.codecov.io/gh/apache/pinot/pull/12349/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `61.60% <84.55%> (+0.04%)` | :arrow_up: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/12349/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `61.62% <84.55%> (-0.08%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/12349/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `61.62% <84.55%> (-0.08%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/12349/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `46.78% <48.00%> (-0.14%)` | :arrow_down: |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/12349/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `27.68% <81.17%> (+0.05%)` | :arrow_up: |
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/12349?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][test] adding tests for IN/NOT-IN operation [pinot]
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #12349:
URL: https://github.com/apache/pinot/pull/12349#discussion_r1475300114
##########
pinot-query-runtime/src/test/resources/queries/FilterAggregates.json:
##########
@@ -75,6 +75,9 @@
},
{
"sql": "SELECT bool_col, COALESCE(min(double_col) FILTER (WHERE string_col = 'a' OR string_col = 'b'), 0), COALESCE(max(double_col) FILTER (WHERE string_col = 'a' OR int_col > 10), 0), avg(double_col), sum(double_col), count(double_col), count(distinct(double_col)) FILTER (WHERE string_col = 'b' OR int_col > 10), count(string_col) FROM {tbl} WHERE string_col='b' GROUP BY bool_col"
+ },
+ {
+ "sql": "SELECT string_col, count(bool_col) FILTER ( WHERE double_col NOT IN (1, 3, 5, 7)) FROM {tbl} WHERE double_col < 10 AND int_col BETWEEN 1 AND 1 AND int_col <> 1 GROUP BY string_col"
Review Comment:
this is a super corner case situation:
- Where clause reduces the filter into a no-match (`xxx between 1 and 1 and xxx <> 1`)
- A futher agg filter on top of an already filtered logic (e.g. `yyy < 10` and `yyy NOT IN (1, 3, 5, 7)`
then it triggered some rule that didn't merge the filter pushdown and causes the NOT_IN to be eval as an actual method.
--
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