mustafasrepo commented on code in PR #6695:
URL: https://github.com/apache/arrow-datafusion/pull/6695#discussion_r1231892829
##########
datafusion/physical-expr/src/window/aggregate.rs:
##########
@@ -155,8 +155,7 @@ impl WindowExpr for PlainAggregateWindowExpr {
}
fn uses_bounded_memory(&self) -> bool {
- self.aggregate.supports_bounded_execution()
- && !self.window_frame.end_bound.is_unbounded()
+ !self.window_frame.end_bound.is_unbounded()
Review Comment:
After thinking through this logic. Actually, as long as `end_bound` is not
bounded (not `UNBOUNDED FOLLOWING` such as in the form `N FOLLOWING`). We can
produce results without waiting for the whole data to come (If accumulator do
not support `retract_batch` method. We wouldn't be able to run queries in the
form `M PRECEDING and N FOLLOWING`, in this case we will give an error
anyway.). Hence here we do not need to check for
`self.aggregate.supports_bounded_execution()` (`acc.supports_retract_batch()`
method with the new API.)
##########
datafusion/physical-expr/src/window/sliding_aggregate.rs:
##########
@@ -139,8 +139,7 @@ impl WindowExpr for SlidingAggregateWindowExpr {
}
fn uses_bounded_memory(&self) -> bool {
- self.aggregate.supports_bounded_execution()
- && !self.window_frame.end_bound.is_unbounded()
+ !self.window_frame.end_bound.is_unbounded()
Review Comment:
Similar to the case above.
--
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]
