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-23343-e1b773a29f7be3e9b841bfc1cfd02d246bed1195 in repository https://gitbox.apache.org/repos/asf/datafusion.git
commit 2880e1044c004fd2f4b4f82c9445812ad23f0411 Author: RIchard Baah <[email protected]> AuthorDate: Thu Jul 9 14:05:14 2026 -0400 Perf: Add short circuit for primitive vectorized equal_to (#23343) ## 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 #23342. ## Rationale for this change other `groupColumn` traits follow the pattern of short circuiting the equal_to comparison if a previous `groupColumn` set that bit to zero. This is because all traits needs to assert that the value is the same for each row, otherwise its a new tuple. This avoids an un-needed (potentially expensive) comparison. <!-- 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. --> ## What changes are included in this PR? adds an early check to read the bit at the current index. <!-- 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. --> ## Are these changes tested? yes <!-- 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 If tests are not included in your PR, please explain why (for example, are they covered by existing tests)? --> ## 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. --> --- .../group_values/multi_group_by/primitive.rs | 28 +++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) 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 068b849cb2..148c5697de 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 @@ -83,6 +83,9 @@ where for (i, (&lhs_row, &rhs_row)) in lhs_rows.iter().zip(rhs_rows.iter()).enumerate() { + if !equal_to_results.get_bit(i) { + continue; + } let left = if cfg!(debug_assertions) { self.group_values[lhs_row] } else { @@ -127,7 +130,6 @@ where if !equal_to_results.get_bit(idx) { continue; } - 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) { @@ -293,9 +295,10 @@ mod tests { use crate::aggregates::group_values::multi_group_by::primitive::PrimitiveGroupValueBuilder; use arrow::array::{ - ArrayRef, BooleanBufferBuilder, Float32Array, Int64Array, NullBufferBuilder, + ArrayRef, BooleanBufferBuilder, Float32Array, Int32Array, Int64Array, + NullBufferBuilder, }; - use arrow::datatypes::{DataType, Float32Type, Int64Type}; + use arrow::datatypes::{DataType, Float32Type, Int32Type, Int64Type}; use super::GroupColumn; @@ -594,6 +597,25 @@ mod tests { assert!(results[4]); } + // All bits false: every row must be skipped; accessing any lhs/rhs index would panic. + #[test] + fn test_vectorized_equal_to_skips_false_rows() { + let mut builder = + PrimitiveGroupValueBuilder::<Int32Type, true>::new(DataType::Int32); + let array = Arc::new(Int32Array::from(vec![None::<i32>, None])) as ArrayRef; + builder.vectorized_append(&array, &[0, 1]).unwrap(); + + let mut results = BooleanBufferBuilder::new(2); + results.append_n(2, false); + + builder.vectorized_equal_to( + &[usize::MAX, usize::MAX], + &array, + &[usize::MAX, usize::MAX], + &mut results, + ); + } + #[test] fn test_primitive_take_n() { // drain branch: n * 2 <= len --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
