avantgardnerio commented on code in PR #24035:
URL: https://github.com/apache/datafusion/pull/24035#discussion_r3730684328
##########
datafusion/physical-plan/src/windows/bounded_window_agg_exec.rs:
##########
@@ -1053,6 +1113,33 @@ impl BoundedWindowAggStream {
// For instance, if `n_out` number of rows are calculated, we can remove
// first `n_out` rows from `self.input_buffer`.
fn prune_state(&mut self, n_out: usize) -> Result<()> {
+ // `WindowAggState::is_end` is copied from
`PartitionBatchState::is_end`
+ // during `evaluate_stateful`, so every window_expr's state map agrees
+ // on which partition keys have just closed. Publish those keys before
+ // the retains in `prune_out_columns` / `prune_partition_batches` drop
+ // them.
+ if let Some(observer) = self.finalized_state_observer.clone()
+ && !self.window_agg_states.is_empty()
+ {
+ let closed_keys: Vec<PartitionKey> = self.window_agg_states[0]
Review Comment:
I made the swap for `PartitionKey`, but the accumulator state itself is not
so easy. For example `HLLAccumulator` calls:
```
fn from(v: &HyperLogLog<T>) -> ScalarValue {
let values = v.as_ref().to_vec(); // 16 KiB register array →
owned Vec<u8>
ScalarValue::Binary(Some(values))
}
```
Allocating a new 16kb state each time (obviously this is worst case, others
like `Sum` are cheap).
But I think this is fundamental to the API - unless someone registers a
`state_observer`, they don't incur the cost. And if they do register one, I
think that is an expected part of the contract.
An option could be to make this only observe the last state of the entire
DataFusion partition, which would fill my needs.
--
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]
