This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-25238-5313ae9be2f66a95c4a43f0337e1ef5cd6ad6933 in repository https://gitbox.apache.org/repos/asf/datafusion.git
commit 9a1ad7c792967adf315bf64b157ab584156802fc Author: RIchard Baah <[email protected]> AuthorDate: Sat Sep 12 23:32:09 2026 +0000 refactor: widen `GroupColumn::vectorized_equal_to` to `&mut self` (#25238) ## Which issue does this PR close? <!-- We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123. --> - works towards #25219. ## Rationale for this change Mechanical signature change across the trait and all 8 impls as the first step toward letting `DictionaryGroupValuesColumn` reuse its cached `val_to_inner` map in `vectorized_equal_to` instead of rebuilding it per batch (#25219). <!-- Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed. Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes. Please explain the problem you are trying to solve in terms of the user-visible behavior, rather than the implementation. For example, "The code in `foo.rs` doesn't handle nulls" is a symptom of the implementation. "COUNT(DISTINCT) returns wrong results when the column contains nulls" is the user-visible problem. --> ## What changes are included in this PR? `&self` -> `&mut self` <!-- There is no need to duplicate the description in the issue here, but it is sometimes worth providing a summary of the individual changes in this PR. --> ## What is the testing strategy for this PR? n/a <!-- We typically require tests for all PRs in order to: 1. Prevent the code from being accidentally broken by subsequent changes 2. Serve as another way to document the expected behavior of the code Briefly describe how this PR is tested, and point to the specific tests you added. For example: 'This new feature is covered by the `sqllogictest` cases added in `foo.slt`'. If this PR does not add tests, explain why. For example, if the change is already covered by existing tests, please mention it. You should also check the `codecov` bot reply on this PR to confirm the changed code is exercised. --> ## Are there any user-facing changes? no <!-- If there are user-facing changes then we may require documentation to be updated before approving the PR. If there are any breaking changes to public APIs, please add the `api change` label. --> Co-authored-by: Claude Sonnet 4.6 <[email protected]> --- .../aggregates/group_values/multi_group_by/boolean.rs | 18 +++++++++--------- .../aggregates/group_values/multi_group_by/bytes.rs | 10 +++++----- .../group_values/multi_group_by/bytes_view.rs | 10 +++++----- .../group_values/multi_group_by/dictionary.rs | 2 +- .../group_values/multi_group_by/fixed_size_binary.rs | 10 +++++----- .../src/aggregates/group_values/multi_group_by/list.rs | 2 +- .../src/aggregates/group_values/multi_group_by/mod.rs | 4 ++-- .../group_values/multi_group_by/primitive.rs | 18 +++++++++--------- .../group_values/multi_group_by/row_backed.rs | 2 +- 9 files changed, 38 insertions(+), 38 deletions(-) diff --git a/datafusion/physical-plan/src/aggregates/group_values/multi_group_by/boolean.rs b/datafusion/physical-plan/src/aggregates/group_values/multi_group_by/boolean.rs index a90c4379c1..da656e0a85 100644 --- a/datafusion/physical-plan/src/aggregates/group_values/multi_group_by/boolean.rs +++ b/datafusion/physical-plan/src/aggregates/group_values/multi_group_by/boolean.rs @@ -79,7 +79,7 @@ impl<const NULLABLE: bool> GroupColumn for BooleanGroupValueBuilder<NULLABLE> { } fn vectorized_equal_to( - &self, + &mut self, lhs_rows: &[usize], array: &ArrayRef, rhs_rows: &[usize], @@ -235,7 +235,7 @@ mod tests { }; let equal_to = - |builder: &BooleanGroupValueBuilder<true>, + |builder: &mut BooleanGroupValueBuilder<true>, lhs_rows: &[usize], input_array: &ArrayRef, rhs_rows: &[usize], @@ -261,7 +261,7 @@ mod tests { }; let equal_to = - |builder: &BooleanGroupValueBuilder<true>, + |builder: &mut BooleanGroupValueBuilder<true>, lhs_rows: &[usize], input_array: &ArrayRef, rhs_rows: &[usize], @@ -281,7 +281,7 @@ mod tests { where A: FnMut(&mut BooleanGroupValueBuilder<true>, &ArrayRef, &[usize]), E: FnMut( - &BooleanGroupValueBuilder<true>, + &mut BooleanGroupValueBuilder<true>, &[usize], &ArrayRef, &[usize], @@ -332,7 +332,7 @@ mod tests { // Check let mut equal_to_results = make_true_buffer(builder.len()); equal_to( - &builder, + &mut builder, &[0, 1, 2, 3, 4, 5], &input_array, &[0, 1, 2, 3, 4, 5], @@ -359,7 +359,7 @@ mod tests { }; let equal_to = - |builder: &BooleanGroupValueBuilder<false>, + |builder: &mut BooleanGroupValueBuilder<false>, lhs_rows: &[usize], input_array: &ArrayRef, rhs_rows: &[usize], @@ -385,7 +385,7 @@ mod tests { }; let equal_to = - |builder: &BooleanGroupValueBuilder<false>, + |builder: &mut BooleanGroupValueBuilder<false>, lhs_rows: &[usize], input_array: &ArrayRef, rhs_rows: &[usize], @@ -405,7 +405,7 @@ mod tests { where A: FnMut(&mut BooleanGroupValueBuilder<false>, &ArrayRef, &[usize]), E: FnMut( - &BooleanGroupValueBuilder<false>, + &mut BooleanGroupValueBuilder<false>, &[usize], &ArrayRef, &[usize], @@ -437,7 +437,7 @@ mod tests { // Check let mut equal_to_results = make_true_buffer(builder.len()); equal_to( - &builder, + &mut builder, &[0, 1, 2, 3], &input_array, &[0, 1, 2, 3], diff --git a/datafusion/physical-plan/src/aggregates/group_values/multi_group_by/bytes.rs b/datafusion/physical-plan/src/aggregates/group_values/multi_group_by/bytes.rs index 94d2e48ad3..3032536657 100644 --- a/datafusion/physical-plan/src/aggregates/group_values/multi_group_by/bytes.rs +++ b/datafusion/physical-plan/src/aggregates/group_values/multi_group_by/bytes.rs @@ -280,7 +280,7 @@ where } fn vectorized_equal_to( - &self, + &mut self, lhs_rows: &[usize], array: &ArrayRef, rhs_rows: &[usize], @@ -581,7 +581,7 @@ mod tests { }; let equal_to = - |builder: &ByteGroupValueBuilder<i32>, + |builder: &mut ByteGroupValueBuilder<i32>, lhs_rows: &[usize], input_array: &ArrayRef, rhs_rows: &[usize], @@ -607,7 +607,7 @@ mod tests { }; let equal_to = - |builder: &ByteGroupValueBuilder<i32>, + |builder: &mut ByteGroupValueBuilder<i32>, lhs_rows: &[usize], input_array: &ArrayRef, rhs_rows: &[usize], @@ -689,7 +689,7 @@ mod tests { where A: FnMut(&mut ByteGroupValueBuilder<i32>, &ArrayRef, &[usize]), E: FnMut( - &ByteGroupValueBuilder<i32>, + &mut ByteGroupValueBuilder<i32>, &[usize], &ArrayRef, &[usize], @@ -741,7 +741,7 @@ mod tests { // Check let mut equal_to_results = make_true_buffer(builder.len()); equal_to( - &builder, + &mut builder, &[0, 1, 2, 3, 4, 5], &input_array, &[0, 1, 2, 3, 4, 5], diff --git a/datafusion/physical-plan/src/aggregates/group_values/multi_group_by/bytes_view.rs b/datafusion/physical-plan/src/aggregates/group_values/multi_group_by/bytes_view.rs index 190438f3c8..8153b67d6c 100644 --- a/datafusion/physical-plan/src/aggregates/group_values/multi_group_by/bytes_view.rs +++ b/datafusion/physical-plan/src/aggregates/group_values/multi_group_by/bytes_view.rs @@ -584,7 +584,7 @@ impl<B: ByteViewType> GroupColumn for ByteViewGroupValueBuilder<B> { } fn vectorized_equal_to( - &self, + &mut self, group_indices: &[usize], array: &ArrayRef, rows: &[usize], @@ -714,7 +714,7 @@ mod tests { }; let equal_to = - |builder: &ByteViewGroupValueBuilder<StringViewType>, + |builder: &mut ByteViewGroupValueBuilder<StringViewType>, lhs_rows: &[usize], input_array: &ArrayRef, rhs_rows: &[usize], @@ -740,7 +740,7 @@ mod tests { }; let equal_to = - |builder: &ByteViewGroupValueBuilder<StringViewType>, + |builder: &mut ByteViewGroupValueBuilder<StringViewType>, lhs_rows: &[usize], input_array: &ArrayRef, rhs_rows: &[usize], @@ -823,7 +823,7 @@ mod tests { where A: FnMut(&mut ByteViewGroupValueBuilder<StringViewType>, &ArrayRef, &[usize]), E: FnMut( - &ByteViewGroupValueBuilder<StringViewType>, + &mut ByteViewGroupValueBuilder<StringViewType>, &[usize], &ArrayRef, &[usize], @@ -908,7 +908,7 @@ mod tests { // Check let mut equal_to_results = make_true_buffer(input_array.len()); equal_to( - &builder, + &mut builder, &[0, 1, 2, 3, 4, 5, 6, 7, 7, 7, 8, 8], &input_array, &[0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11], diff --git a/datafusion/physical-plan/src/aggregates/group_values/multi_group_by/dictionary.rs b/datafusion/physical-plan/src/aggregates/group_values/multi_group_by/dictionary.rs index d73018fbc1..b190c17297 100644 --- a/datafusion/physical-plan/src/aggregates/group_values/multi_group_by/dictionary.rs +++ b/datafusion/physical-plan/src/aggregates/group_values/multi_group_by/dictionary.rs @@ -328,7 +328,7 @@ impl<K: ArrowDictionaryKeyType + Send + Sync> GroupColumn } fn vectorized_equal_to( - &self, + &mut self, lhs_rows: &[usize], array: &ArrayRef, rhs_rows: &[usize], diff --git a/datafusion/physical-plan/src/aggregates/group_values/multi_group_by/fixed_size_binary.rs b/datafusion/physical-plan/src/aggregates/group_values/multi_group_by/fixed_size_binary.rs index 26dd7f8e05..bd59e564c7 100644 --- a/datafusion/physical-plan/src/aggregates/group_values/multi_group_by/fixed_size_binary.rs +++ b/datafusion/physical-plan/src/aggregates/group_values/multi_group_by/fixed_size_binary.rs @@ -140,7 +140,7 @@ impl GroupColumn for FixedSizeBinaryGroupValueBuilder { } fn vectorized_equal_to( - &self, + &mut self, lhs_rows: &[usize], array: &ArrayRef, rhs_rows: &[usize], @@ -295,7 +295,7 @@ mod tests { }; let equal_to = - |builder: &FixedSizeBinaryGroupValueBuilder, + |builder: &mut FixedSizeBinaryGroupValueBuilder, lhs_rows: &[usize], input_array: &ArrayRef, rhs_rows: &[usize], @@ -321,7 +321,7 @@ mod tests { }; let equal_to = - |builder: &FixedSizeBinaryGroupValueBuilder, + |builder: &mut FixedSizeBinaryGroupValueBuilder, lhs_rows: &[usize], input_array: &ArrayRef, rhs_rows: &[usize], @@ -341,7 +341,7 @@ mod tests { where A: FnMut(&mut FixedSizeBinaryGroupValueBuilder, &ArrayRef, &[usize]), E: FnMut( - &FixedSizeBinaryGroupValueBuilder, + &mut FixedSizeBinaryGroupValueBuilder, &[usize], &ArrayRef, &[usize], @@ -388,7 +388,7 @@ mod tests { // Check let mut equal_to_results = make_true_buffer(builder.len()); equal_to( - &builder, + &mut builder, &[0, 1, 2, 3, 4, 5], &input_array, &[0, 1, 2, 3, 4, 5], diff --git a/datafusion/physical-plan/src/aggregates/group_values/multi_group_by/list.rs b/datafusion/physical-plan/src/aggregates/group_values/multi_group_by/list.rs index 8c681698be..b5dbdb3876 100644 --- a/datafusion/physical-plan/src/aggregates/group_values/multi_group_by/list.rs +++ b/datafusion/physical-plan/src/aggregates/group_values/multi_group_by/list.rs @@ -152,7 +152,7 @@ impl<O: OffsetSizeTrait> GroupColumn for ListGroupValueBuilder<O> { } fn vectorized_equal_to( - &self, + &mut self, lhs_rows: &[usize], array: &ArrayRef, rhs_rows: &[usize], diff --git a/datafusion/physical-plan/src/aggregates/group_values/multi_group_by/mod.rs b/datafusion/physical-plan/src/aggregates/group_values/multi_group_by/mod.rs index 6c1926b402..b206fdd94e 100644 --- a/datafusion/physical-plan/src/aggregates/group_values/multi_group_by/mod.rs +++ b/datafusion/physical-plan/src/aggregates/group_values/multi_group_by/mod.rs @@ -87,7 +87,7 @@ pub trait GroupColumn: Send + Sync { /// And if found nth result in `equal_to_results` is already /// `false`, the check for nth row will be skipped. fn vectorized_equal_to( - &self, + &mut self, lhs_rows: &[usize], array: &ArrayRef, rhs_rows: &[usize], @@ -664,7 +664,7 @@ impl<const STREAMING: bool> GroupValuesColumn<STREAMING> { equal_to_results.truncate(0); equal_to_results.append_n(n, true); - for (col_idx, group_col) in self.group_values.iter().enumerate() { + for (col_idx, group_col) in self.group_values.iter_mut().enumerate() { group_col.vectorized_equal_to( &self.vectorized_operation_buffers.equal_to_group_indices, &cols[col_idx], diff --git a/datafusion/physical-plan/src/aggregates/group_values/multi_group_by/primitive.rs b/datafusion/physical-plan/src/aggregates/group_values/multi_group_by/primitive.rs index 441d4b489d..fe160b58db 100644 --- a/datafusion/physical-plan/src/aggregates/group_values/multi_group_by/primitive.rs +++ b/datafusion/physical-plan/src/aggregates/group_values/multi_group_by/primitive.rs @@ -187,7 +187,7 @@ where } fn vectorized_equal_to( - &self, + &mut self, lhs_rows: &[usize], array: &ArrayRef, rhs_rows: &[usize], @@ -341,7 +341,7 @@ mod tests { }; let equal_to = - |builder: &PrimitiveGroupValueBuilder<Float32Type, true>, + |builder: &mut PrimitiveGroupValueBuilder<Float32Type, true>, lhs_rows: &[usize], input_array: &ArrayRef, rhs_rows: &[usize], @@ -367,7 +367,7 @@ mod tests { }; let equal_to = - |builder: &PrimitiveGroupValueBuilder<Float32Type, true>, + |builder: &mut PrimitiveGroupValueBuilder<Float32Type, true>, lhs_rows: &[usize], input_array: &ArrayRef, rhs_rows: &[usize], @@ -387,7 +387,7 @@ mod tests { where A: FnMut(&mut PrimitiveGroupValueBuilder<Float32Type, true>, &ArrayRef, &[usize]), E: FnMut( - &PrimitiveGroupValueBuilder<Float32Type, true>, + &mut PrimitiveGroupValueBuilder<Float32Type, true>, &[usize], &ArrayRef, &[usize], @@ -442,7 +442,7 @@ mod tests { // Check let mut equal_to_results = make_true_buffer(builder.len()); equal_to( - &builder, + &mut builder, &[0, 1, 2, 3, 4, 5, 6], &input_array, &[0, 1, 2, 3, 4, 5, 6], @@ -470,7 +470,7 @@ mod tests { }; let equal_to = - |builder: &PrimitiveGroupValueBuilder<Int64Type, false>, + |builder: &mut PrimitiveGroupValueBuilder<Int64Type, false>, lhs_rows: &[usize], input_array: &ArrayRef, rhs_rows: &[usize], @@ -496,7 +496,7 @@ mod tests { }; let equal_to = - |builder: &PrimitiveGroupValueBuilder<Int64Type, false>, + |builder: &mut PrimitiveGroupValueBuilder<Int64Type, false>, lhs_rows: &[usize], input_array: &ArrayRef, rhs_rows: &[usize], @@ -516,7 +516,7 @@ mod tests { where A: FnMut(&mut PrimitiveGroupValueBuilder<Int64Type, false>, &ArrayRef, &[usize]), E: FnMut( - &PrimitiveGroupValueBuilder<Int64Type, false>, + &mut PrimitiveGroupValueBuilder<Int64Type, false>, &[usize], &ArrayRef, &[usize], @@ -540,7 +540,7 @@ mod tests { // Check let mut equal_to_results = make_true_buffer(builder.len()); equal_to( - &builder, + &mut builder, &[0, 1], &input_array, &[0, 1], diff --git a/datafusion/physical-plan/src/aggregates/group_values/multi_group_by/row_backed.rs b/datafusion/physical-plan/src/aggregates/group_values/multi_group_by/row_backed.rs index 2fa01e1ae2..f488ef79df 100644 --- a/datafusion/physical-plan/src/aggregates/group_values/multi_group_by/row_backed.rs +++ b/datafusion/physical-plan/src/aggregates/group_values/multi_group_by/row_backed.rs @@ -251,7 +251,7 @@ impl GroupColumn for RowsGroupColumn { } fn vectorized_equal_to( - &self, + &mut self, lhs_rows: &[usize], array: &ArrayRef, rhs_rows: &[usize], --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
