alamb commented on code in PR #6621:
URL: https://github.com/apache/arrow-datafusion/pull/6621#discussion_r1224770664
##########
datafusion/physical-expr/src/window/built_in.rs:
##########
@@ -211,13 +209,7 @@ impl WindowExpr for BuiltInWindowExpr {
state.update(&out_col, partition_batch_state)?;
if self.window_frame.start_bound.is_unbounded() {
- let mut evaluator_state = evaluator.state()?;
- if let BuiltinWindowState::NthValue(nth_value_state) =
- &mut evaluator_state
- {
- memoize_nth_value(state, nth_value_state)?;
- evaluator.set_state(&evaluator_state)?;
- }
+ evaluator.memoize(state)?;
Review Comment:
I think this is actually more readable as well as cleaning up the trait
##########
datafusion/physical-expr/src/window/window_expr.rs:
##########
@@ -327,16 +327,7 @@ pub struct LeadLagState {
pub idx: usize,
}
-#[derive(Debug, Clone, Default)]
Review Comment:
It turns out this structure is not used anywhere else
##########
datafusion/physical-expr/src/window/partition_evaluator.rs:
##########
@@ -100,14 +99,6 @@ pub trait PartitionEvaluator: Debug + Send {
false
}
- /// Returns the internal state of the window function
Review Comment:
The point of the PR is to get of the functions on this trait that serialized
/ set state (which has a single use)
--
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]
