jayzhan211 commented on code in PR #12623:
URL: https://github.com/apache/datafusion/pull/12623#discussion_r1779845711


##########
datafusion/physical-plan/src/aggregates/group_values/group_column.rs:
##########
@@ -62,57 +64,60 @@ pub trait GroupColumn: Send + Sync {
 
 pub struct PrimitiveGroupValueBuilder<T: ArrowPrimitiveType> {
     group_values: Vec<T::Native>,
-    nulls: Vec<bool>,
-    // whether the array contains at least one null, for fast non-null path
-    has_null: bool,
-    nullable: bool,
+    /// Null state (when None, input is guaranteed not to have nulls)
+    nulls: Option<MaybeNullBufferBuilder>,

Review Comment:
   My benchmark shows a slowdown compared to the main branch. I experienced a 
similar slowdown before due to the use of Option, and I suspect that might be 
the case again with this change



-- 
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

Reply via email to