tustvold commented on code in PR #8662:
URL: https://github.com/apache/arrow-datafusion/pull/8662#discussion_r1437033784
##########
datafusion/physical-expr/src/aggregate/first_last.rs:
##########
@@ -459,10 +493,52 @@ impl LastValueAccumulator {
}
// Updates state with the values in the given row.
- fn update_with_new_row(&mut self, row: &[ScalarValue]) {
- self.last = row[0].clone();
- self.orderings = row[1..].to_vec();
- self.is_set = true;
+ fn update_with_new_row(&mut self, row: &[ScalarValue]) -> Result<()> {
+ let value = &row[0];
+ let orderings = &row[1..];
+ // Update when
+ // - no value in the state
+ // - There is no specific requirement, but a new value (most recent
entry in terms of execution)
+ // - There is a more recent entry in terms of requirement
+ if !self.is_set
+ || self.orderings.is_empty()
+ || compare_rows(
Review Comment:
I'm sure you are aware but https://docs.rs/arrow-row/latest/arrow_row/ will
be a much faster way to perform row-based comparisons than relying on
ScalarValue
--
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]
