Dandandan commented on code in PR #12996:
URL: https://github.com/apache/datafusion/pull/12996#discussion_r1830810815
##########
datafusion/physical-plan/src/aggregates/group_values/group_column.rs:
##########
@@ -128,6 +157,89 @@ impl<T: ArrowPrimitiveType, const NULLABLE: bool>
GroupColumn
}
}
+ fn vectorized_equal_to(
+ &self,
+ lhs_rows: &[usize],
+ array: &ArrayRef,
+ rhs_rows: &[usize],
+ equal_to_results: &mut [bool],
+ ) {
+ let array = array.as_primitive::<T>();
+
+ let iter = izip!(
+ lhs_rows.iter(),
+ rhs_rows.iter(),
+ equal_to_results.iter_mut(),
+ );
+
+ for (&lhs_row, &rhs_row, equal_to_result) in iter {
+ // Has found not equal to in previous column, don't need to check
+ if !*equal_to_result {
+ continue;
+ }
+
+ // Perf: skip null check (by short circuit) if input is not
nullable
+ if NULLABLE {
+ let exist_null = self.nulls.is_null(lhs_row);
+ let input_null = array.is_null(rhs_row);
+ if let Some(result) = nulls_equal_to(exist_null, input_null) {
+ *equal_to_result = result;
+ continue;
+ }
+ // Otherwise, we need to check their values
+ }
+
+ *equal_to_result = self.group_values[lhs_row] ==
array.value(rhs_row);
+ }
+ }
+
+ fn vectorized_append(&mut self, array: &ArrayRef, rows: &[usize]) {
+ let arr = array.as_primitive::<T>();
+
+ let null_count = array.null_count();
+ let num_rows = array.len();
+ let all_null_or_non_null = if null_count == 0 {
+ Some(true)
+ } else if null_count == num_rows {
+ Some(false)
+ } else {
+ None
+ };
+
+ match (NULLABLE, all_null_or_non_null) {
+ (true, None) => {
+ for &row in rows {
+ if array.is_null(row) {
+ self.nulls.append(true);
+ self.group_values.push(T::default_value());
+ } else {
+ self.nulls.append(false);
+ self.group_values.push(arr.value(row));
+ }
+ }
+ }
+
+ (true, Some(true)) => {
+ self.nulls.append_n(rows.len(), false);
+ for &row in rows {
+ self.group_values.push(arr.value(row));
+ }
+ }
+
+ (true, Some(false)) => {
+ self.nulls.append_n(rows.len(), true);
+ self.group_values
+ .extend(iter::repeat(T::default_value()).take(rows.len()));
+ }
+
+ (false, _) => {
+ for &row in rows {
+ self.group_values.push(arr.value(row));
Review Comment:
I think there are several things that could be done to make the append even
faster:
1. `extend_from_slice` `if rows.len() == array.len()`
2. use `extend` rather than `push` for values
3. Speed up appending nulls (don't append bits one by one)
--
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: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
