Skip to content

Commit ae37c0b

Browse files
feat(physical-plan): FinalizedWindowStateObserver on BoundedWindowAggExec
Invert the API from #24007: instead of storing per-partition state slots inside the plan behind an Arc<[Mutex<_>]>, accept a callback and let the caller own the storage. The exec becomes a pure event source — no FinalStateSlot, no Empty/Single/Multi machine, no getter, no mutex-poison or out-of-range errors, no "at most one PARTITION BY group" doctrine. Multi-group handling is the caller's, so the API generalizes for free. Adds one method on the already-public WindowState (`aggregate_state`) so callers can read Accumulator::state without a `pub use WindowFn` re-export that would leak an internal enum through the public surface. The write site is the top of `prune_state` — every entry with `state.is_end` is published before `prune_out_columns` / `prune_partition_batches` drop it. `WindowAggState::is_end` is copied from `PartitionBatchState::is_end` during `evaluate_stateful`, so all window_expr state maps agree on which keys have closed. API shape per reviewer suggestion on #24007. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent dbcb5c0 commit ae37c0b

2 files changed

Lines changed: 215 additions & 4 deletions

File tree

datafusion/physical-expr/src/window/window_expr.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -610,6 +610,19 @@ pub struct WindowState {
610610
pub state: WindowAggState,
611611
pub window_fn: WindowFn,
612612
}
613+
614+
impl WindowState {
615+
/// `Accumulator::state()` if this window function is an aggregate, `None`
616+
/// otherwise (built-in functions like `row_number`, `rank`, `lead`/`lag`
617+
/// have no serializable accumulator state).
618+
pub fn aggregate_state(&mut self) -> Result<Option<Vec<ScalarValue>>> {
619+
match &mut self.window_fn {
620+
WindowFn::Aggregate(accumulator) => accumulator.state().map(Some),
621+
WindowFn::Builtin(_) => Ok(None),
622+
}
623+
}
624+
}
625+
613626
pub type PartitionWindowAggStates = IndexMap<PartitionKey, WindowState, RandomState>;
614627

615628
/// The IndexMap (i.e. an ordered HashMap) where record batches are separated for each partition.

datafusion/physical-plan/src/windows/bounded_window_agg_exec.rs

Lines changed: 202 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ use datafusion_common::utils::{
5454
evaluate_partition_ranges, get_at_indices, get_row_at_idx,
5555
};
5656
use datafusion_common::{
57-
HashMap, Result, arrow_datafusion_err, exec_datafusion_err, exec_err,
57+
HashMap, Result, ScalarValue, arrow_datafusion_err, exec_datafusion_err, exec_err,
5858
};
5959
use datafusion_execution::TaskContext;
6060
use datafusion_expr::ColumnarValue;
@@ -75,8 +75,18 @@ use hashbrown::hash_table::HashTable;
7575
use indexmap::IndexMap;
7676
use log::debug;
7777

78+
/// Called by [`BoundedWindowAggExec`] when a PARTITION BY group closes,
79+
/// once per (output-partition-index, PARTITION BY tuple). The third argument
80+
/// is one entry per window expression on the exec, in the same order as
81+
/// [`BoundedWindowAggExec::window_expr`]; `None` for built-in functions
82+
/// (`row_number`, `rank`, `lead`/`lag`, ...), `Some(state)` for aggregates
83+
/// via [`datafusion_expr::Accumulator::state`].
84+
pub type FinalizedWindowStateObserver = Arc<
85+
dyn Fn(usize, &PartitionKey, &[Option<Vec<ScalarValue>>]) -> Result<()> + Send + Sync,
86+
>;
87+
7888
/// Window execution plan
79-
#[derive(Debug, Clone)]
89+
#[derive(Clone)]
8090
pub struct BoundedWindowAggExec {
8191
/// Input plan
8292
input: Arc<dyn ExecutionPlan>,
@@ -99,6 +109,32 @@ pub struct BoundedWindowAggExec {
99109
cache: Arc<PlanProperties>,
100110
/// If `can_rerepartition` is false, partition_keys is always empty.
101111
can_repartition: bool,
112+
/// Invoked at partition-close to publish finalized per-partition window
113+
/// state. Storage and multi-group handling are the caller's; the exec is
114+
/// a pure event source.
115+
finalized_state_observer: Option<FinalizedWindowStateObserver>,
116+
}
117+
118+
impl std::fmt::Debug for BoundedWindowAggExec {
119+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
120+
f.debug_struct("BoundedWindowAggExec")
121+
.field("input", &self.input)
122+
.field("window_expr", &self.window_expr)
123+
.field("schema", &self.schema)
124+
.field("metrics", &self.metrics)
125+
.field("input_order_mode", &self.input_order_mode)
126+
.field(
127+
"ordered_partition_by_indices",
128+
&self.ordered_partition_by_indices,
129+
)
130+
.field("cache", &self.cache)
131+
.field("can_repartition", &self.can_repartition)
132+
.field(
133+
"finalized_state_observer",
134+
&self.finalized_state_observer.as_ref().map(|_| "..."),
135+
)
136+
.finish()
137+
}
102138
}
103139

104140
impl BoundedWindowAggExec {
@@ -139,9 +175,20 @@ impl BoundedWindowAggExec {
139175
ordered_partition_by_indices,
140176
cache: Arc::new(cache),
141177
can_repartition,
178+
finalized_state_observer: None,
142179
})
143180
}
144181

182+
/// Install a callback that receives each PARTITION BY group's finalized
183+
/// window state at partition close. See [`FinalizedWindowStateObserver`].
184+
pub fn with_finalized_state_observer(
185+
mut self,
186+
observer: FinalizedWindowStateObserver,
187+
) -> Self {
188+
self.finalized_state_observer = Some(observer);
189+
self
190+
}
191+
145192
/// Window expressions
146193
pub fn window_expr(&self) -> &[Arc<dyn WindowExpr>] {
147194
&self.window_expr
@@ -345,12 +392,16 @@ impl ExecutionPlan for BoundedWindowAggExec {
345392
children: Vec<Arc<dyn ExecutionPlan>>,
346393
) -> Result<Arc<dyn ExecutionPlan>> {
347394
check_if_same_properties!(self, children);
348-
Ok(Arc::new(BoundedWindowAggExec::try_new(
395+
let mut new = BoundedWindowAggExec::try_new(
349396
self.window_expr.clone(),
350397
Arc::clone(&children[0]),
351398
self.input_order_mode.clone(),
352399
self.can_repartition,
353-
)?))
400+
)?;
401+
if let Some(observer) = &self.finalized_state_observer {
402+
new = new.with_finalized_state_observer(Arc::clone(observer));
403+
}
404+
Ok(Arc::new(new))
354405
}
355406

356407
fn with_new_children_and_same_properties(
@@ -377,6 +428,8 @@ impl ExecutionPlan for BoundedWindowAggExec {
377428
input,
378429
BaselineMetrics::new(&self.metrics, partition),
379430
search_mode,
431+
partition,
432+
self.finalized_state_observer.clone(),
380433
)?);
381434
Ok(stream)
382435
}
@@ -1010,6 +1063,13 @@ pub struct BoundedWindowAggStream {
10101063
/// Search mode for partition columns. This determines the algorithm with
10111064
/// which we group each partition.
10121065
search_mode: Box<dyn PartitionSearcher>,
1066+
/// Output partition index this stream serves; passed as the first
1067+
/// argument to `finalized_state_observer`.
1068+
partition_idx: usize,
1069+
/// If set, invoked at partition close in [`Self::prune_state`] with the
1070+
/// finalized per-window-expression state for every partition key that is
1071+
/// about to be dropped.
1072+
finalized_state_observer: Option<FinalizedWindowStateObserver>,
10131073
}
10141074

10151075
impl BoundedWindowAggStream {
@@ -1021,6 +1081,33 @@ impl BoundedWindowAggStream {
10211081
// For instance, if `n_out` number of rows are calculated, we can remove
10221082
// first `n_out` rows from `self.input_buffer`.
10231083
fn prune_state(&mut self, n_out: usize) -> Result<()> {
1084+
// `WindowAggState::is_end` is copied from `PartitionBatchState::is_end`
1085+
// during `evaluate_stateful`, so every window_expr's state map agrees
1086+
// on which partition keys have just closed. Publish those keys before
1087+
// the retains in `prune_out_columns` / `prune_partition_batches` drop
1088+
// them.
1089+
if let Some(observer) = self.finalized_state_observer.clone()
1090+
&& !self.window_agg_states.is_empty()
1091+
{
1092+
let closed_keys: Vec<PartitionKey> = self.window_agg_states[0]
1093+
.iter()
1094+
.filter(|(_, ws)| ws.state.is_end)
1095+
.map(|(k, _)| k.clone())
1096+
.collect();
1097+
for key in closed_keys {
1098+
let mut states: Vec<Option<Vec<ScalarValue>>> =
1099+
Vec::with_capacity(self.window_agg_states.len());
1100+
for per_expr in self.window_agg_states.iter_mut() {
1101+
let entry = per_expr.get_mut(&key).ok_or_else(|| {
1102+
exec_datafusion_err!(
1103+
"finalized_state_observer: missing state for closed partition key"
1104+
)
1105+
})?;
1106+
states.push(entry.aggregate_state()?);
1107+
}
1108+
observer(self.partition_idx, &key, &states)?;
1109+
}
1110+
}
10241111
// Prune `self.window_agg_states`:
10251112
self.prune_out_columns();
10261113
// Prune `self.partition_batches`:
@@ -1053,6 +1140,8 @@ impl BoundedWindowAggStream {
10531140
input: SendableRecordBatchStream,
10541141
baseline_metrics: BaselineMetrics,
10551142
search_mode: Box<dyn PartitionSearcher>,
1143+
partition_idx: usize,
1144+
finalized_state_observer: Option<FinalizedWindowStateObserver>,
10561145
) -> Result<Self> {
10571146
let state = window_expr.iter().map(|_| IndexMap::default()).collect();
10581147
let empty_batch = RecordBatch::new_empty(Arc::clone(&schema));
@@ -1066,6 +1155,8 @@ impl BoundedWindowAggStream {
10661155
window_expr,
10671156
baseline_metrics,
10681157
search_mode,
1158+
partition_idx,
1159+
finalized_state_observer,
10691160
})
10701161
}
10711162

@@ -1908,6 +1999,113 @@ mod tests {
19081999
Ok(())
19092000
}
19102001

2002+
#[tokio::test]
2003+
async fn test_finalized_state_observer_fires_at_partition_close() -> Result<()> {
2004+
use crate::windows::bounded_window_agg_exec::FinalizedWindowStateObserver;
2005+
use datafusion_physical_expr::window::PartitionKey;
2006+
use std::sync::Mutex;
2007+
2008+
let task_ctx = Arc::new(TaskContext::default());
2009+
let schema = test_schema();
2010+
2011+
// Two PARTITION BY groups: hash=1 [sn=1,2,3] then hash=2 [sn=4,5,6].
2012+
// Input is sorted by (hash, sn) so we can run in Sorted mode; in that
2013+
// mode `mark_partition_end` closes the leading group mid-stream and
2014+
// EOS closes the tail — both should fire the observer.
2015+
let mut sn_b = UInt64Builder::with_capacity(6);
2016+
let mut hash_b = Int64Builder::with_capacity(6);
2017+
for (sn, hash) in [(1u64, 1i64), (2, 1), (3, 1), (4, 2), (5, 2), (6, 2)] {
2018+
sn_b.append_value(sn);
2019+
hash_b.append_value(hash);
2020+
}
2021+
let batch = RecordBatch::try_new(
2022+
Arc::clone(&schema),
2023+
vec![Arc::new(sn_b.finish()), Arc::new(hash_b.finish())],
2024+
)?;
2025+
let ordering: LexOrdering = [
2026+
PhysicalSortExpr {
2027+
expr: col("hash", &schema)?,
2028+
options: SortOptions::default(),
2029+
},
2030+
PhysicalSortExpr {
2031+
expr: col("sn", &schema)?,
2032+
options: SortOptions::default(),
2033+
},
2034+
]
2035+
.into();
2036+
let source_raw =
2037+
TestMemoryExec::try_new(&[vec![batch]], Arc::clone(&schema), None)?
2038+
.try_with_sort_information(vec![ordering])?;
2039+
let source: Arc<dyn ExecutionPlan> =
2040+
Arc::new(TestMemoryExec::update_cache(&Arc::new(source_raw)));
2041+
2042+
let window_fn = WindowFunctionDefinition::AggregateUDF(count_udaf());
2043+
let args = vec![col("sn", &schema)?];
2044+
let partition_by = vec![col("hash", &schema)?];
2045+
let order_by = vec![PhysicalSortExpr {
2046+
expr: col("sn", &schema)?,
2047+
options: SortOptions::default(),
2048+
}];
2049+
// CURRENT ROW → UNBOUNDED FOLLOWING forces each row's output to wait
2050+
// for partition close (is_causal = false), so hash=2's rows are held
2051+
// until EOS marks its buffer is_end — which is the path we want to
2052+
// exercise for the observer.
2053+
let frame = WindowFrame::new_bounds(
2054+
WindowFrameUnits::Rows,
2055+
WindowFrameBound::CurrentRow,
2056+
WindowFrameBound::Following(ScalarValue::UInt64(None)),
2057+
);
2058+
let expr = create_window_expr(
2059+
&window_fn,
2060+
"cnt".to_string(),
2061+
&args,
2062+
&partition_by,
2063+
&order_by,
2064+
Arc::new(frame),
2065+
source.schema(),
2066+
false,
2067+
false,
2068+
None,
2069+
)?;
2070+
2071+
type Observation = (usize, PartitionKey, Vec<Option<Vec<ScalarValue>>>);
2072+
let observations: Arc<Mutex<Vec<Observation>>> = Arc::new(Mutex::new(vec![]));
2073+
let sink = Arc::clone(&observations);
2074+
let observer: FinalizedWindowStateObserver = Arc::new(move |idx, key, states| {
2075+
sink.lock()
2076+
.unwrap()
2077+
.push((idx, key.clone(), states.to_vec()));
2078+
Ok(())
2079+
});
2080+
2081+
let plan = BoundedWindowAggExec::try_new(
2082+
vec![expr],
2083+
source,
2084+
InputOrderMode::Sorted,
2085+
false,
2086+
)?
2087+
.with_finalized_state_observer(observer);
2088+
2089+
let _ = collect(Arc::new(plan).execute(0, task_ctx)?).await?;
2090+
2091+
let obs = observations.lock().unwrap();
2092+
assert_eq!(obs.len(), 2, "one observation per PARTITION BY group");
2093+
let keys: Vec<i64> = obs
2094+
.iter()
2095+
.map(|(_, k, _)| match &k[0] {
2096+
ScalarValue::Int64(Some(v)) => *v,
2097+
other => panic!("unexpected partition-key element: {other:?}"),
2098+
})
2099+
.collect();
2100+
assert_eq!(keys, vec![1, 2]);
2101+
for (idx, _, states) in obs.iter() {
2102+
assert_eq!(*idx, 0, "single output partition");
2103+
assert_eq!(states.len(), 1, "one window expression");
2104+
assert!(states[0].is_some(), "count() is an aggregate → Some(state)");
2105+
}
2106+
Ok(())
2107+
}
2108+
19112109
#[test]
19122110
fn test_bounded_window_agg_cardinality_effect() -> Result<()> {
19132111
let schema = test_schema();

0 commit comments

Comments
 (0)