You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2024/11/20 21:28:43 UTC
[I] Implement GroupColumn Decimal128Array [datafusion]
alamb opened a new issue, #13505:
URL: https://github.com/apache/datafusion/issues/13505
(no comment)
--
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-unsubscribe@datafusion.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] Implement GroupColumn Decimal128Array [datafusion]
Posted by "jonathanc-n (via GitHub)" <gi...@apache.org>.
jonathanc-n commented on issue #13505:
URL: https://github.com/apache/datafusion/issues/13505#issuecomment-2489649451
take
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] Implement GroupColumn Decimal128Array [datafusion]
Posted by "jonathanc-n (via GitHub)" <gi...@apache.org>.
jonathanc-n commented on issue #13505:
URL: https://github.com/apache/datafusion/issues/13505#issuecomment-2492551341
@alamb For this pr, will it need its own custom column implementation for decimal128 similar to how byte, byteview, stringview, etc. are dealt with? I am thinking that due to the parameters
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] Implement GroupColumn Decimal128Array [datafusion]
Posted by "jonathanc-n (via GitHub)" <gi...@apache.org>.
jonathanc-n commented on issue #13505:
URL: https://github.com/apache/datafusion/issues/13505#issuecomment-2494696505
Yep, thanks!
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] Implement GroupColumn Decimal128Array [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #13505:
URL: https://github.com/apache/datafusion/issues/13505#issuecomment-2493611071
> @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?
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] Implement GroupColumn Decimal128Array [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #13505:
URL: https://github.com/apache/datafusion/issues/13505#issuecomment-2489641641
BTW we can verify that this is working as expected after merging
- https://github.com/apache/datafusion/pull/13506
Then you can do
```sql
cd datafusion-cli
RUST_LOG=debug cargo run -- -c "create or replace table foo(x decimal(10,3), y int) as values (10.0, 100), (21.2, 200), (33.0, 300); select count(*) from foo group by x, y";
```
You should not see any lines about `Creating GroupValuesRows `. Here is what is printed out on main
```
[2024-11-20T22:08:58Z DEBUG datafusion_physical_plan::aggregates::group_values::row] Creating GroupValuesRows for schema: Field { name: "x", data_type: Decimal128(10, 3), nullable: true, dict_id: 0, dict_is_ordered: false, metadata: {} }, Field { name: "y", data_type: Int32, nullable: true, dict_id: 0, dict_is_ordered: false, metadata: {} }
```
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] Implement GroupColumn Decimal128Array [datafusion]
Posted by "jayzhan211 (via GitHub)" <gi...@apache.org>.
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] Implement GroupColumn Decimal128Array [datafusion]
Posted by "jayzhan211 (via GitHub)" <gi...@apache.org>.
jayzhan211 closed issue #13505: Implement GroupColumn Decimal128Array
URL: https://github.com/apache/datafusion/issues/13505
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] Implement GroupColumn Decimal128Array [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #13505:
URL: https://github.com/apache/datafusion/issues/13505#issuecomment-2495446992
> It seems `adjust_output_array` clone the array, is it better to use `with_data_type` to avoid the clone?
I think they are roughly equivalent - if we can avoid cloning the array that certainly seems better
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org