jayzhan211 commented on issue #13505: URL: https://github.com/apache/datafusion/issues/13505#issuecomment-2495206147
> > @alamb For this pr, will it need its own custom column implementation for decimal128 instead of instantiate_primitive!, similar to how byte, byteview, stringview, etc. are dealt with? I am thinking that due to the parameters > > I think you can use `Vec<u128>` for the underlying storage (aka use the PrimtiveGroup struct), for the underlying primitive type, but then call this function to: > > https://github.com/apache/datafusion/blob/207e855f235401b997269fa858af641cf2a7b81e/datafusion/functions-aggregate-common/src/utils.rs#L51 > > To adjust the type at the end > > For example like this: > > https://github.com/apache/datafusion/blob/207e855f235401b997269fa858af641cf2a7b81e/datafusion/physical-plan/src/aggregates/topk/heap.rs#L156 > > So the idea is that you keep the actual group values as native types (u128 in this case) as we are only comparing their values > > Does that make sense? It seems `adjust_output_array` clone the array, is it better to use `with_data_type` to avoid the clone? https://github.com/apache/datafusion/blob/c0ca4b4e449e07c3bcd6f3593fa31dd31ed5e0c5/datafusion/physical-plan/src/aggregates/group_values/single_group_by/primitive.rs#L211 -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
