You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "somandal (via GitHub)" <gi...@apache.org> on 2023/04/25 00:32:47 UTC
[GitHub] [pinot] somandal opened a new pull request, #10684: [multistage] Add some additional planner tests for ROW_NUMBER() window function
somandal opened a new pull request, #10684:
URL: https://github.com/apache/pinot/pull/10684
This PR address the review comments on the `ROW_NUMBER()` window functions PR: https://github.com/apache/pinot/pull/10587
cc @siddharthteotia @vvivekiyer @walterddr
--
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
[GitHub] [pinot] siddharthteotia merged pull request #10684: [multistage] Add some additional planner tests for ROW_NUMBER() window function
Posted by "siddharthteotia (via GitHub)" <gi...@apache.org>.
siddharthteotia merged PR #10684:
URL: https://github.com/apache/pinot/pull/10684
--
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
[GitHub] [pinot] somandal commented on a diff in pull request #10684: [multistage] Add some additional planner tests for ROW_NUMBER() window function
Posted by "somandal (via GitHub)" <gi...@apache.org>.
somandal commented on code in PR #10684:
URL: https://github.com/apache/pinot/pull/10684#discussion_r1176734753
##########
pinot-query-planner/src/test/resources/queries/WindowFunctionPlans.json:
##########
@@ -1278,6 +1278,54 @@
"\n"
]
},
+ {
+ "description": "single OVER(ORDER BY) row_number and select col with global order by on different column as inside over",
Review Comment:
Yeah the JOIN one needs a bit more work and I'll add that as part of RANK/DENSE_RANK support or as a separate PR to cover more JOIN scenarios.
--
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
[GitHub] [pinot] siddharthteotia commented on a diff in pull request #10684: [multistage] Add some additional planner tests for ROW_NUMBER() window function
Posted by "siddharthteotia (via GitHub)" <gi...@apache.org>.
siddharthteotia commented on code in PR #10684:
URL: https://github.com/apache/pinot/pull/10684#discussion_r1175934088
##########
pinot-query-planner/src/test/resources/queries/WindowFunctionPlans.json:
##########
@@ -1278,6 +1278,54 @@
"\n"
]
},
+ {
+ "description": "single OVER(ORDER BY) row_number and select col with global order by on different column as inside over",
Review Comment:
What about corresponding runtime tests for these variants ? Especially for the JOIN one ?
Can be done separately if not already covered
--
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
[GitHub] [pinot] codecov-commenter commented on pull request #10684: [multistage] Add some additional planner tests for ROW_NUMBER() window function
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #10684:
URL: https://github.com/apache/pinot/pull/10684#issuecomment-1521042266
## [Codecov](https://codecov.io/gh/apache/pinot/pull/10684?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#10684](https://codecov.io/gh/apache/pinot/pull/10684?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ccc4c74) into [master](https://codecov.io/gh/apache/pinot/commit/a130cb2b1e465ad9ad3bdb4428a32fef5bf54cdd?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a130cb2) will **decrease** coverage by `54.66%`.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## master #10684 +/- ##
=============================================
- Coverage 68.47% 13.82% -54.66%
+ Complexity 6507 439 -6068
=============================================
Files 2108 2054 -54
Lines 113904 111417 -2487
Branches 17192 16895 -297
=============================================
- Hits 77997 15402 -62595
- Misses 30354 94757 +64403
+ Partials 5553 1258 -4295
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| unittests1 | `?` | |
| unittests2 | `13.82% <ø> (-0.01%)` | :arrow_down: |
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=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/10684?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...uery/runtime/operator/WindowAggregateOperator.java](https://codecov.io/gh/apache/pinot/pull/10684?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS9vcGVyYXRvci9XaW5kb3dBZ2dyZWdhdGVPcGVyYXRvci5qYXZh) | `0.00% <ø> (-95.29%)` | :arrow_down: |
... and [1623 files with indirect coverage changes](https://codecov.io/gh/apache/pinot/pull/10684/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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