Thanks for the comments Timo.
1. Consistency:
Could we rename the `canXX` methods to `supportsXX` methods?
> This might fit better to the existing methods such as
> `FunctionDefinition.supportsConstantFoloding()` and other ability
> interfaces that we use in connectors.
Sounds good. I c
Thank you for the update Alan. I just have some last minor comments,
mostly around naming:
1. Consistency:
Could we rename the `canXX` methods to `supportsXX` methods?
This might fit better to the existing methods such as
`FunctionDefinition.supportsConstantFoloding()` and other abili
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
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
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
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
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
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
