github-actions[bot] commented on code in PR #64864:
URL: https://github.com/apache/doris/pull/64864#discussion_r3479375628
##########
be/src/exprs/aggregate/aggregate_function_window.h:
##########
@@ -623,14 +623,15 @@ struct WindowFunctionNthValueImpl : Data {
this->_frame_total_rows ? this->_frame_start_pose :
real_frame_start;
this->_frame_total_rows += real_frame_end - real_frame_start;
Review Comment:
This still breaks `nth_value` for frames that start at `n FOLLOWING`.
Nereids reverses `ROWS BETWEEN 2 FOLLOWING AND UNBOUNDED FOLLOWING` into an
`UNBOUNDED PRECEDING ... 2 PRECEDING` frame and negates the offset. In that BE
path `_get_next_for_unbounded_rows()` feeds the aggregate delta ranges like
`[-2,-1)`, `[-1,0)`, `[0,1)`, `[1,2)`, ... before the reversed frame reaches
the partition start. The first of those ranges clips to `real_frame_start = 0`
and `real_frame_end = -1`, so this line subtracts from `_frame_total_rows`;
later, for rows that should have a non-empty original frame, the cumulative
state has the wrong count/start and `offset -2` can return NULL. A concrete
case is `nth_value(v, 2) over (order by seq rows between 2 following and
unbounded following)` on five rows: the row with `seq=2` should return the
value from `seq=5`, but the reversed executor path has only counted one row
after the earlier negative deltas. Please clamp empty clipped ranges so they do
not mu
tate `_frame_total_rows`/`_frame_start_pose`, and add a regression case for an
`n FOLLOWING` left boundary.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
