Re: [DISCUSS] KIP-747 Add support for basic aggregation APIs

2021-08-16 Thread Mohan Parthasarathy
On Mon, Aug 16, 2021 at 5:28 PM Matthias J. Sax wrote: > > @Mohan: > > For sum(), I actually think that the return type should be same as the > input type. In the end, sum() is a special case of reduce(). > If you define something like this as you suggested (without defining new interfaces): E s

Re: [DISCUSS] KIP-747 Add support for basic aggregation APIs

2021-08-16 Thread Matthias J. Sax
@Mohan: For sum(), I actually think that the return type should be same as the input type. In the end, sum() is a special case of reduce(). @Alexandre: Not sure if using a `Function` to get a `Comparable` is simpler that to implement a `Comparator`? Can you elaborate on this point? In the end, u

Re: [DISCUSS] KIP-747 Add support for basic aggregation APIs

2021-08-15 Thread Alexandre Brasil
> I am not sure why we would want to pass `Function> > func` into `min()`? I guess I misread/misunderstood your earlier suggestion. My line of thought was that, instead of using a method signature that demands a Comparator in min()/max(), we might use a property extractor (like the FK extractors

Re: [DISCUSS] KIP-747 Add support for basic aggregation APIs

2021-08-08 Thread Mohan Parthasarathy
On Sun, Aug 8, 2021 at 3:56 PM Matthias J. Sax wrote: > >>> Not sure. Also, do you have an opinion on Long vs Double ? > > Not sure what you mean by `Long vs Double` ? Can you elaborate? > > We were discussing the return value for "sum". Alex suggested using "Double" and you had "Long" in one of

Re: [DISCUSS] KIP-747 Add support for basic aggregation APIs

2021-08-08 Thread Matthias J. Sax
>>> Not sure. Also, do you have an opinion on Long vs Double ? Not sure what you mean by `Long vs Double` ? Can you elaborate? -Matthias On 8/8/21 7:41 AM, Mohan Parthasarathy wrote: > On Tue, Aug 3, 2021 at 6:56 PM Matthias J. Sax wrote: > >> I was playing with the code a little bit, but it

Re: [DISCUSS] KIP-747 Add support for basic aggregation APIs

2021-08-08 Thread Mohan Parthasarathy
On Tue, Aug 3, 2021 at 6:56 PM Matthias J. Sax wrote: > I was playing with the code a little bit, but it seems not to be easy to > use generics to enforce that V is `Comparable`... > > We would need to introduce a new interface > > interface ComparableStream> > extends KStream > { > KTa

Re: [DISCUSS] KIP-747 Add support for basic aggregation APIs

2021-08-03 Thread Matthias J. Sax
I was playing with the code a little bit, but it seems not to be easy to use generics to enforce that V is `Comparable`... We would need to introduce a new interface interface ComparableStream> extends KStream { KTable min(); } But it also requires a nasty cast to actually use it:

Re: [DISCUSS] KIP-747 Add support for basic aggregation APIs

2021-06-21 Thread Mohan Parthasarathy
Alex, On Wed, Jun 16, 2021 at 8:07 AM Alexandre Brasil wrote: > Mohan / Mathias, > > > > I think extending min/max to non-numeric types makes sense. Wondering > > > why we should require a `Comparator` or if we should require that the > > > types implement `Comparable` instead? > > > > > Good q

Re: [DISCUSS] KIP-747 Add support for basic aggregation APIs

2021-06-16 Thread Alexandre Brasil
Mohan / Mathias, > > I think extending min/max to non-numeric types makes sense. Wondering > > why we should require a `Comparator` or if we should require that the > > types implement `Comparable` instead? > > > Good question. This is what it would look like: > > KTable min_comparable() > KTable

Re: [DISCUSS] KIP-747 Add support for basic aggregation APIs

2021-06-15 Thread Mohan Parthasarathy
Matthias, On Mon, Jun 14, 2021 at 9:18 PM Matthias J. Sax wrote: > Hi, > > I think extending min/max to non-numeric types makes sense. Wondering > why we should require a `Comparator` or if we should require that the > types implement `Comparable` instead? > > Good question. This is what it woul

Re: [DISCUSS] KIP-747 Add support for basic aggregation APIs

2021-06-14 Thread Matthias J. Sax
Hi, I think extending min/max to non-numeric types makes sense. Wondering why we should require a `Comparator` or if we should require that the types implement `Comparable` instead? I also think, that min/max should not change the value type. Using `Long` for sum() make sense though, and also to

Re: [DISCUSS] KIP-747 Add support for basic aggregation APIs

2021-06-08 Thread Mohan Parthasarathy
Hi Alex, On Tue, Jun 8, 2021 at 2:44 PM Alexandre Brasil wrote: > > My point here is that, when we're only interested in a max/min numeric > value, it doesn't > matter when we have repeated values, since we'd be only forwarding the > number downstream, > so I could disregard when the Comparator

Re: [DISCUSS] KIP-747 Add support for basic aggregation APIs

2021-06-08 Thread Alexandre Brasil
Hi Mohan, > I like your idea though it is odd that min/max returns KTable instead of the > KTable (like in count), but mapValues should do the trick. My line of thought is that min/max could forward the min/max V associated with each K downstream. It'd not be the same as in count, where it'll al

Re: [DISCUSS] KIP-747 Add support for basic aggregation APIs

2021-06-05 Thread Mohan Parthasarathy
Hi Alex, Responses below. On Fri, Jun 4, 2021 at 9:27 AM Alexandre Brasil wrote: > Hi Mohan, > > I like the idea of adding those methods to the API, but I'd like to make a > suggestion: > > Although the most used scenario for min() / max() might possibly be for > numeric values, I think they co

Re: [DISCUSS] KIP-747 Add support for basic aggregation APIs

2021-06-04 Thread Alexandre Brasil
Hi Mohan, I like the idea of adding those methods to the API, but I'd like to make a suggestion: Although the most used scenario for min() / max() might possibly be for numeric values, I think they could also be useful on other objects like Dates, LocalDates or Strings. Why limit the API to Numbe

[DISCUSS] KIP-747 Add support for basic aggregation APIs

2021-06-02 Thread Mohan Parthasarathy
Hi, I have created a proposal for adding some additional aggregation APIs like count. https://cwiki.apache.org/confluence/display/KAFKA/KIP-747+Add+support+for+basic+aggregation+APIs I have noted down some of the issues that need discussion. Thanks to Matthias for helping me with the scope of th