tustvold commented on code in PR #8662:
URL: https://github.com/apache/arrow-datafusion/pull/8662#discussion_r1437033317
##########
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(
+ &self.orderings,
+ orderings,
+ &get_sort_options(&self.ordering_req),
+ )?
+ .is_lt()
+ {
+ self.last = value.clone();
+ self.orderings = orderings.to_vec();
+ self.is_set = true;
+ }
+ Ok(())
+ }
+
+ fn get_last_idx(&self, values: &[ArrayRef]) -> Result<Option<usize>> {
+ let value = &values[0];
+ let ordering_values = &values[1..];
+ assert_eq!(ordering_values.len(), self.ordering_req.len());
+ if self.ordering_req.is_empty() {
+ return Ok((!value.is_empty()).then_some(value.len() - 1));
+ }
+ let sort_columns = ordering_values
+ .iter()
+ .zip(self.ordering_req.iter())
+ .map(|(values, req)| {
+ // Take reverse ordering requirement this would enable us to
use fetch=1 for last value.
+ SortColumn {
+ values: values.clone(),
+ options: Some(!req.options),
+ }
+ })
+ .collect::<Vec<_>>();
+ let indices = lexsort_to_indices(&sort_columns, Some(1))?;
Review Comment:
I'm not aware of a min/max kernel that returns the ordinal position of the
min/max
--
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]
