Rachelint commented on code in PR #12996:
URL: https://github.com/apache/datafusion/pull/12996#discussion_r1830886618
##########
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 we already could use extend instead of `push`? `extend` on `Vec`
is somewhat faster than push as the capacity check / allocation is done once
instead of once per value.
Ok, I got it, it is really simple to do it!
--
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
