ozankabak commented on code in PR #4777:
URL: https://github.com/apache/arrow-datafusion/pull/4777#discussion_r1061815043
##########
datafusion/core/src/physical_plan/planner.rs:
##########
@@ -614,13 +614,28 @@ impl DefaultPhysicalPlanner {
})
.collect::<Result<Vec<_>>>()?;
- Ok(Arc::new(WindowAggExec::try_new(
- window_expr,
- input_exec,
- physical_input_schema,
- physical_partition_keys,
- physical_sort_keys,
- )?))
+ let uses_bounded_memory = window_expr
Review Comment:
The challenge with doing this in `optimize_sorts.rs` is that it comes after
the pipeline-fixing step. We want the ordinary -> bounded conversion done
before that so we can analyze pipelining correctly.
However, I agree with your general line of thinking. We are currently
experimenting with various ways to simplify/reorganize rules so that this can
happen not in the planner but in a rule (like `OptimizeSorts`). Here is [an
example
attempt-in-progress](https://github.com/synnada-ai/arrow-datafusion/pull/33).
We will submit a follow-on PR when we have something mature.
--
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]
