Skip to content

Conversation

@somandal
Copy link
Contributor

@somandal somandal commented Apr 25, 2023

This PR address the review comments on the ROW_NUMBER() window functions PR: #10587

cc @siddharthteotia @vvivekiyer @walterddr

Overall window functions issue - #7213

@codecov-commenter
Copy link

Codecov Report

Merging #10684 (ccc4c74) into master (a130cb2) will decrease coverage by 54.66%.
The diff coverage is n/a.

@@              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%) ⬇️

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

Impacted Files Coverage Δ
...uery/runtime/operator/WindowAggregateOperator.java 0.00% <ø> (-95.29%) ⬇️

... and 1623 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

]
},
{
"description": "single OVER(ORDER BY) row_number and select col with global order by on different column as inside over",
Copy link
Contributor

Choose a reason for hiding this comment

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

What about corresponding runtime tests for these variants ? Especially for the JOIN one ?

Can be done separately if not already covered

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

@siddharthteotia siddharthteotia merged commit 96d34fd into apache:master Apr 25, 2023
@somandal somandal deleted the row-number-address-review branch April 25, 2023 16:04
@Jackie-Jiang Jackie-Jiang added the multi-stage Related to the multi-stage query engine label Apr 25, 2023
@yashmayya yashmayya added the window-functions Related to SQL window functions on the multi-stage query engine label Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

multi-stage Related to the multi-stage query engine window-functions Related to SQL window functions on the multi-stage query engine

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants