Skip to content

Commit 6e56628

Browse files
refactor: hoist observer guard to caller of publish_finalized_states
The method now always mutates when called and takes the observer as a required argument; the "is observer installed?" check moves to the caller in `compute_aggregates`. Removes the "&mut self that only mutates when observer is set" shape. Addresses apache#24035 review comment 3738822701. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent b7aedd7 commit 6e56628

1 file changed

Lines changed: 12 additions & 7 deletions

File tree

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

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1114,12 +1114,15 @@ pub struct BoundedWindowAggStream {
11141114
}
11151115

11161116
impl BoundedWindowAggStream {
1117-
/// Fire the [`WindowStateObserver`] for every partition key whose
1118-
/// `WindowAggState::is_end` is true.
1119-
fn publish_finalized_states(&mut self) -> Result<()> {
1120-
let Some(observer) = &self.state_observer else {
1121-
return Ok(());
1122-
};
1117+
/// Fire `observer` for every partition key whose
1118+
/// [`WindowAggState::is_end`] is true. Always mutates when called:
1119+
/// [`Accumulator::state`] requires `&mut`, which propagates up here.
1120+
/// The caller is responsible for deciding whether to fire (i.e. checking
1121+
/// whether an observer is installed).
1122+
fn publish_finalized_states(
1123+
&mut self,
1124+
observer: &dyn WindowStateObserver,
1125+
) -> Result<()> {
11231126
let Some((first, rest)) = self.window_agg_states.split_first_mut() else {
11241127
return Ok(());
11251128
};
@@ -1222,7 +1225,9 @@ impl BoundedWindowAggStream {
12221225
// already streamed out, so at EOS that call returns `None` and the
12231226
// prune path is skipped — the final partition would otherwise be
12241227
// dropped unobserved.
1225-
self.publish_finalized_states()?;
1228+
if let Some(observer) = self.state_observer.clone() {
1229+
self.publish_finalized_states(observer.as_ref())?;
1230+
}
12261231

12271232
let schema = Arc::clone(&self.schema);
12281233
let window_expr_out = self.search_mode.calculate_out_columns(

0 commit comments

Comments
 (0)