Re: [DISCUSS] FLIP-491: BundledAggregateFunction for batched aggregation

2025-02-07 Thread Alan Sheinberg
Hi all, Is there any more feedback I can incorporate before calling a vote? Thanks, Alan On Tue, Jan 28, 2025 at 1:50 PM Alan Sheinberg wrote: > Hi Fabian, > > I addressed your comments below. > > >> * BundledKeySegement >> * should it be accumulatorS instead of accumulator? >> * should ac

Re: [DISCUSS] FLIP-491: BundledAggregateFunction for batched aggregation

2025-01-28 Thread Alan Sheinberg
Hi Fabian, I addressed your comments below. > * BundledKeySegement > * should it be accumulatorS instead of accumulator? > * should accumulators be null or an empty list if no accumulator is > present? Good catch, forgot to update those places. It should be "accumulators" and an empty lis

Re: [DISCUSS] FLIP-491: BundledAggregateFunction for batched aggregation

2025-01-28 Thread Fabian Hüske
Thanks Alan for updating the FLIP! IMO, it looks good. Just a few nits that would be great to fix for consistency: * BundledKeySegement * should it be accumulatorS instead of accumulator? * should accumulators be null or an empty list if no accumulator is present? * update the Group By Examp

Re: [DISCUSS] FLIP-491: BundledAggregateFunction for batched aggregation

2025-01-27 Thread Alan Sheinberg
Hi everyone, Sorry for the delayed response! I appreciate the comments. Addressing Timo's comments: > Correct me if I'm wrong, but for AggregateFunctions implementing a > retract() and merge() is optional. How can a BundledAggregateFunction > communicate whether or not this is supported to the

Re: [DISCUSS] FLIP-491: BundledAggregateFunction for batched aggregation

2025-01-15 Thread Fabian Hüske
Hi Alan, Thanks for working on this FLIP! Overall the document looks very good, but I have a few question / comments: * Why do we need the `canBundle()` function? We can use the interface itself as a marker. A function that can't bundle, shouldn't implement the interface. * the interface could

Re: [DISCUSS] FLIP-491: BundledAggregateFunction for batched aggregation

2025-01-13 Thread Timo Walther
Hi Alan, thanks for sharing this FLIP with us. Sorry for the late reply. I think the FLIP is already in a very good shape. I like the approach that the BundledAggregateFunction is rather a separate interface that can be implemented by advanced users. This matches with the existing Specialize

[DISCUSS] FLIP-491: BundledAggregateFunction for batched aggregation

2024-12-11 Thread Alan Sheinberg
I'd like to start a discussion of FLIP-491: BundledAggregateFunction for batched aggregation [1] This feature proposes adding a new interface BundledAggregateFunction that can be implemented by AggregateFunction UDFs. This allows the use of a batched method call so that users can handle many rows