On Sun, Jun 4, 2017 at 8:41 PM, Matthias J. Sax <matth...@confluent.io> wrote:
> We started with this proposal but it does not allow for Lambdas (in case > you want key access). Do you think preserving Lambdas is not important > enough for this case -- I agree that there is some price to be paid to > keep Lambdas. > Not sure if I understand this comment.. My main point is not on changing the proposal but just reduce it scope to `ValueJoinerWithKey` only; and even if we want to keep them all, I'd suggest we just implement the added APIs by using the `KeyValueMapper` for `ValueMapperWithKeys`, etc, which seems simpler to me. Does that affect lambda expression usage? > > About API consistency: I can't remember a concrete user request to have > key access in all operators. But I am pretty sure, if we only add it for > some, this request will pop up quickly. IMHO, it's better to do it all > in one shot. (But I am not strict about it if most people think we > should limit it to what was requested by users.) > > I would suggest not doing it if user pop it up, but rather suggesting them to use `map` etc directly but set the key unchanged rather than providing a new operator for it. To me some syntax sugars like this seems of less valuable than others (like print / writeAsText / foreach that are all depending on peek), and keeping adding them will just make the DSL too “overstaffed”. > > -Matthias > > On 6/4/17 6:23 PM, Guozhang Wang wrote: > > With KIP-159, the added "valueMapperWithKey" and > "valueTransformerWithKey" > > along seem to be just adding a few syntax-sugars? E.g. we can simply > > implement: > > > > mapValues(ValueMapperWithKey mapperWithKey) > > > > as > > > > map((k, v) -> (k, mapperWithKey(k, v)) > > > > ---------------------- > > > > I'm not sure how many users would really need such syntax sugars, and > even > > they do, we can support them easily as the above implementations; > > > > Similarly for "ReducerWithKey", it can be implemented as `Aggregator<K, > V, > > V>`, and people who needs key access can just use `aggregate` directly. > > > > The function which I think is really of valuable is `ValueJoinerWithKey`, > > and that seems to be the original request from users while others are > > coming from "API consistency" concerns. Personally I'd prefer only keep > the > > last one (`InitializerWithKey` might also have some value, but I have not > > seen people widely requesting it in their DSL usage yet; if there is a > > common request we can keep it in this KIP as well). WDYT? > > > > Guozhang > > > > > > On Sun, May 28, 2017 at 10:16 AM, Jeyhun Karimov <je.kari...@gmail.com> > > wrote: > > > >> Thanks for clarification Matthias, now everything is clear. > >> > >> On Sun, May 28, 2017 at 6:21 PM Matthias J. Sax <matth...@confluent.io> > >> wrote: > >> > >>> I don't think we can drop ValueTransformerSupplier. If you don't have > an > >>> supplier, you only get a single instance of your function. But for a > >>> stateful transformation, we need multiple instances (one for each task) > >>> of ValueTransformer. > >>> > >>> We don't need suppliers for functions like "ValueMapper" etc because > >>> those are stateless and thus, we can reuse a single instance over > >>> multiple tasks -- but we can't do this for ValueTransformer (and > >> similar). > >>> > >>> Btw: This reminds me about KIP-159: with regard to the RichFunction we > >>> might need a supplier pattern, too. (I'll comment on the other thread, > >>> too.) > >>> > >>> > >>> -Matthias > >>> > >>> On 5/28/17 5:45 AM, Jeyhun Karimov wrote: > >>>> Hi, > >>>> > >>>> I updated KIP. > >>>> Just to avoid misunderstanding, I meant deprecating > >>> ValueTransformerSupplier > >>>> and I am ok with ValueTransformer. > >>>> So instead of using ValueTransformerSupplier can't we directly use > >>>> ValueTransformer > >>>> or ValueTransformerWithKey? > >>>> > >>>> Btw, in current design all features of ValueTransformer will be > >> available > >>>> in ValueTransformerWithKey interface. > >>>> > >>>> Cheers, > >>>> Jeyhun > >>>> > >>>> On Sun, May 28, 2017 at 6:15 AM Matthias J. Sax < > matth...@confluent.io > >>> > >>>> wrote: > >>>> > >>>>> Thanks Jeyhun. > >>>>> > >>>>> About ValueTransformer: I don't think we can remove it. Note, > >>>>> ValueTransformer allows to attach a state and also allows to register > >>>>> punctuations. Both those features will not be available via withKey() > >>>>> interfaces. > >>>>> > >>>>> -Matthias > >>>>> > >>>>> > >>>>> On 5/27/17 1:25 PM, Jeyhun Karimov wrote: > >>>>>> Hi Matthias, > >>>>>> > >>>>>> Thanks for your comments. > >>>>>> > >>>>>> I tested the deep copy approach. It has significant overhead. > >>> Especially > >>>>>> for "light" and stateless operators it slows down the topology > >>>>>> significantly (> 20% ). I think "warning" users about not-changing > >> the > >>>>> key > >>>>>> is better warning them about possible performance loss. > >>>>>> > >>>>>> About the interfaces, additionally I considered adding > >>>>> InitializerWithKey, > >>>>>> AggregatorWithKey and ValueTransformerWithKey. I think they are > >>> included > >>>>> in > >>>>>> PR but not in KIP. I will also include them in KIP, sorry my bad. > >>>>>> Including ReducerWithKey definitely makes sense. Thanks. > >>>>>> > >>>>>> One thing I want to mention is that, maybe we should deprecate > >> methods > >>>>> with > >>>>>> argument type ValueTransformerSupplier > (KStream.transformValues(...)) > >>> and > >>>>>> and as a whole the ValueTransformerSupplier interface. > >>>>>> We can use ValueTransformer/ValueTransformerWithKey type instead > >>> without > >>>>>> additional supplier layer. > >>>>>> > >>>>>> > >>>>>> Cheers, > >>>>>> Jeyhun > >>>>>> > >>>>>> > >>>>>> On Thu, May 25, 2017 at 1:07 AM Matthias J. Sax < > >> matth...@confluent.io > >>>> > >>>>>> wrote: > >>>>>> > >>>>>>> One more question: > >>>>>>> > >>>>>>> Should we add any of > >>>>>>> - InitizialierWithKey > >>>>>>> - ReducerWithKey > >>>>>>> - ValueTransformerWithKey > >>>>>>> > >>>>>>> To get consistent/complete API, it might be a good idea. Any > >> thoughts? > >>>>>>> > >>>>>>> > >>>>>>> -Matthias > >>>>>>> > >>>>>>> > >>>>>>> On 5/24/17 3:47 PM, Matthias J. Sax wrote: > >>>>>>>> Jeyhun, > >>>>>>>> > >>>>>>>> I was just wondering if you did look into the key-deep-copy idea > we > >>>>>>>> discussed. I am curious to see what the impact might be. > >>>>>>>> > >>>>>>>> > >>>>>>>> -Matthias > >>>>>>>> > >>>>>>>> On 5/20/17 2:03 AM, Jeyhun Karimov wrote: > >>>>>>>>> Hi, > >>>>>>>>> > >>>>>>>>> Thanks for your comments. I rethink about including rich > functions > >>>>> into > >>>>>>>>> this KIP. > >>>>>>>>> I think once we include rich functions in this KIP and then fix > >>>>>>>>> ProcessorContext in another KIP and incorporate with existing > rich > >>>>>>>>> functions, the code will not be backwards compatible. > >>>>>>>>> > >>>>>>>>> I see Damian's and your point more clearly now. > >>>>>>>>> > >>>>>>>>> Conclusion: we include only withKey interfaces in this KIP (I > >>> updated > >>>>>>> the > >>>>>>>>> KIP), I will try also initiate another KIP for rich functions. > >>>>>>>>> > >>>>>>>>> Cheers, > >>>>>>>>> Jeyhun > >>>>>>>>> > >>>>>>>>> On Fri, May 19, 2017 at 10:50 PM Matthias J. Sax < > >>>>> matth...@confluent.io > >>>>>>>> > >>>>>>>>> wrote: > >>>>>>>>> > >>>>>>>>>> With the current KIP, using ValueMapper and ValueMapperWithKey > >>>>>>>>>> interfaces, RichFunction seems to be an independent add-on. To > >> fix > >>>>> the > >>>>>>>>>> original issue to allow key access, RichFunctions are not > >> required > >>>>>>> IMHO. > >>>>>>>>>> > >>>>>>>>>> I initially put the RichFunction idea on the table, because I > was > >>>>>>> hoping > >>>>>>>>>> to get a uniform API. And I think, is was good to consider them > >> -- > >>>>>>>>>> however, the discussion showed that they are not necessary for > >> key > >>>>>>>>>> access. Thus, it seems to be better to handle RichFunctions in > an > >>> own > >>>>>>>>>> KIP. The ProcessorContext/RecordContext issues seems to be a > main > >>>>>>>>>> challenge for this. And introducing RichFunctions with > >>> parameter-less > >>>>>>>>>> init() method, seem not to help too much. We would get an > >>>>>>> "intermediate" > >>>>>>>>>> API that we want to change anyway later on... > >>>>>>>>>> > >>>>>>>>>> As you put already much effort into RichFunction, feel free do > >> push > >>>>>>> this > >>>>>>>>>> further and start a new KIP (we can do this even in parallel) -- > >> we > >>>>>>>>>> don't want to slow you down :) But it make the discussion and > >> code > >>>>>>>>>> review easier, if we separate both IMHO. > >>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> -Matthias > >>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> On 5/19/17 2:25 AM, Jeyhun Karimov wrote: > >>>>>>>>>>> Hi Damian, > >>>>>>>>>>> > >>>>>>>>>>> Thanks for your comments. I think providing to users > *interface* > >>>>>>> rather > >>>>>>>>>>> than *abstract class* should be preferred (Matthias also raised > >>> this > >>>>>>>>>> issue > >>>>>>>>>>> ), anyway I changed the corresponding parts of KIP. > >>>>>>>>>>> > >>>>>>>>>>> Regarding with passing additional contextual information, I > >> think > >>> it > >>>>>>> is a > >>>>>>>>>>> tradeoff, > >>>>>>>>>>> 1) first, we fix the context parameter for *init() *method in > >>>>> another > >>>>>>> PR > >>>>>>>>>>> and solve Rich functions afterwards > >>>>>>>>>>> 2) first, we fix the requested issues on jira ([1-3]) with > >>> providing > >>>>>>>>>>> (not-complete) Rich functions and integrate the context > >> parameters > >>>>> to > >>>>>>>>>> this > >>>>>>>>>>> afterwards (like improvement) > >>>>>>>>>>> > >>>>>>>>>>> To me, the second approach seems more incremental. However you > >> are > >>>>>>> right, > >>>>>>>>>>> the names might confuse the users. > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> [1] https://issues.apache.org/jira/browse/KAFKA-4218 > >>>>>>>>>>> [2] https://issues.apache.org/jira/browse/KAFKA-4726 > >>>>>>>>>>> [3] https://issues.apache.org/jira/browse/KAFKA-3745 > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> Cheers, > >>>>>>>>>>> Jeyhun > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> On Fri, May 19, 2017 at 10:42 AM Damian Guy < > >> damian....@gmail.com > >>>> > >>>>>>>>>> wrote: > >>>>>>>>>>> > >>>>>>>>>>>> Hi, > >>>>>>>>>>>> > >>>>>>>>>>>> I see you've removed the `ProcessorContext` from the > >> RichFunction > >>>>>>> which > >>>>>>>>>> is > >>>>>>>>>>>> good, but why is it a `RichFunction`? I'd have expected it to > >>> pass > >>>>>>> some > >>>>>>>>>>>> additional contextual information, i.e., the `RecordContext` > >> that > >>>>>>>>>> contains > >>>>>>>>>>>> just the topic, partition, timestamp, offset. I'm ok with it > >> not > >>>>>>>>>> passing > >>>>>>>>>>>> this contextual information, but is the naming incorrect? I'm > >> not > >>>>>>> sure, > >>>>>>>>>>>> tbh. I'm wondering if we should drop `RichFunctions` until we > >> can > >>>>> do > >>>>>>> it > >>>>>>>>>>>> properly with the correct context? > >>>>>>>>>>>> > >>>>>>>>>>>> Also, i don't like the abstract classes: RichValueMapper, > >>>>>>>>>> RichValueJoiner, > >>>>>>>>>>>> RichInitializer etc. Why can't they not just be interfaces? > >>>>>>> Generally we > >>>>>>>>>>>> should provide users with Intefaces to code against, not > >> classes. > >>>>>>>>>>>> > >>>>>>>>>>>> Thanks, > >>>>>>>>>>>> Damian > >>>>>>>>>>>> > >>>>>>>>>>>> On Fri, 19 May 2017 at 00:50 Jeyhun Karimov < > >>> je.kari...@gmail.com> > >>>>>>>>>> wrote: > >>>>>>>>>>>> > >>>>>>>>>>>>> Hi, > >>>>>>>>>>>>> > >>>>>>>>>>>>> Thanks. I initiated the PR as well, to get a better overview. > >>>>>>>>>>>>> > >>>>>>>>>>>>> The only reason that I used abstract class instead of > >> interface > >>>>> for > >>>>>>>>>> Rich > >>>>>>>>>>>>> functions is that in future if we will have some > >>>>>>> AbstractRichFunction > >>>>>>>>>>>>> abstract classes, > >>>>>>>>>>>>> we can easily extend: > >>>>>>>>>>>>> > >>>>>>>>>>>>> public abstract class RichValueMapper<K, V, VR> implements > >>>>>>>>>>>>> ValueMapperWithKey<K, V, VR>, RichFunction *extends > >>>>>>>>>>>> AbstractRichFunction*{ > >>>>>>>>>>>>> } > >>>>>>>>>>>>> With interfaces we are only limited to interfaces for > >>>>> inheritance. > >>>>>>>>>>>>> > >>>>>>>>>>>>> However, I think we can easily change it (from abstract class > >> -> > >>>>>>>>>>>> interface) > >>>>>>>>>>>>> if you think interface is a better fit. > >>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>> Cheers, > >>>>>>>>>>>>> Jeyhun > >>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>> On Fri, May 19, 2017 at 1:00 AM Matthias J. Sax < > >>>>>>> matth...@confluent.io > >>>>>>>>>>> > >>>>>>>>>>>>> wrote: > >>>>>>>>>>>>> > >>>>>>>>>>>>>> Thanks for the update and explanations. The KIP is quite > >>> improved > >>>>>>> now > >>>>>>>>>>>> -- > >>>>>>>>>>>>>> great job! > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> One more question: Why are RichValueMapper etc abstract > >> classes > >>>>> and > >>>>>>>>>> not > >>>>>>>>>>>>>> interfaces? > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> Overall, I like the KIP a lot! > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> -Matthias > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> On 5/16/17 7:03 AM, Jeyhun Karimov wrote: > >>>>>>>>>>>>>>> Hi, > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Thanks for your comments. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> I think supporting Lambdas for `withKey` and > >>>>>>> `AbstractRichFunction` > >>>>>>>>>>>>>>>> don't go together, as Lambdas are only supported for > >>> interfaces > >>>>>>>>>>>> AFAIK. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Maybe I misunderstood your comment. > >>>>>>>>>>>>>>> *withKey* and and *withOnlyValue* are interfaces. So they > >>> don't > >>>>>>> have > >>>>>>>>>>>>>> direct > >>>>>>>>>>>>>>> relation with *AbstractRichFunction*. > >>>>>>>>>>>>>>> *withKey* and and *withOnlyValue* interfaces have only one > >>>>>>> method , > >>>>>>>>>>>> so > >>>>>>>>>>>>>> we > >>>>>>>>>>>>>>> can use lambdas. > >>>>>>>>>>>>>>> Where does the *AbstractRichFunction* comes to play? Inside > >>> Rich > >>>>>>>>>>>>>> functions. > >>>>>>>>>>>>>>> And we use Rich functions in 2 places: > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> 1. User doesn't use rich functions. Just regular *withKey* > >> and > >>>>> and > >>>>>>>>>>>>>>> *withOnlyValue* interfaces(both support lambdas) . We get > >>> those > >>>>>>>>>>>>>> interfaces > >>>>>>>>>>>>>>> and wrap into Rich function while building the topology, > and > >>>>> send > >>>>>>> to > >>>>>>>>>>>>>>> Processor. > >>>>>>>>>>>>>>> 2. User does use rich functions (Rich functions implement > >>>>>>> *withKey* > >>>>>>>>>>>>>>> interface). As a result no lamdas here by definition. In > >> this > >>>>>>> case, > >>>>>>>>>>>>> while > >>>>>>>>>>>>>>> building the topology we do a type check if the object type > >> is > >>>>>>>>>>>>>>> *withKey* or *RichFunction*. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> So *AbstractRichFunction* is just syntactic sugar for Rich > >>>>>>> functions > >>>>>>>>>>>>> and > >>>>>>>>>>>>>>> does not affect using lambdas. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Thus, if we want to support Lambdas for `withKey`, we need > >> to > >>>>>>> have a > >>>>>>>>>>>>>>>> interface approach like this > >>>>>>>>>>>>>>>> - RichFunction -> only adding init() and close() > >>>>>>>>>>>>>>>> - ValueMapper > >>>>>>>>>>>>>>>> - ValueMapperWithKey > >>>>>>>>>>>>>>>> - RichValueMapper extends ValueMapperWithKey, > >> RichFunction > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> As I said above, currently we support lambdas for *withKey* > >>>>>>>>>>>> interfaces > >>>>>>>>>>>>> as > >>>>>>>>>>>>>>> well. However, I agree with your idea and I will remove > the > >>>>>>>>>>>>>>> AbstractRichFunction from the design. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> As an alternative, we could argue, that it is sufficient to > >>>>>>> support > >>>>>>>>>>>>>>>> Lambdas for the "plain" API only, but not for any > "extended > >>>>> API". > >>>>>>>>>>>> For > >>>>>>>>>>>>>>>> this, RichFunction could add key+init+close and > >>>>>>> AbstractRichFunction > >>>>>>>>>>>>>>>> would allow to only care about getting the key. > >>>>>>>>>>>>>>>> Not sure, which one is better. I don't like the idea of > >> more > >>>>>>>>>>>>> overloaded > >>>>>>>>>>>>>>>> methods to get Lambdas for `withKey` interfaces too much > >>>>> because > >>>>>>> we > >>>>>>>>>>>>> have > >>>>>>>>>>>>>>>> already so many overlaods. On the other hand, I do see > >> value > >>> in > >>>>>>>>>>>>>>>> supporting Lambdas for `withKey`. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Just to clarify, with current design we have only one extra > >>>>>>>>>>>> overloaded > >>>>>>>>>>>>>>> method per *withOnlyValue* interface: which is *withKey* > >>>>> version > >>>>>>> of > >>>>>>>>>>>>>>> particular interface. > >>>>>>>>>>>>>>> We don't need extra overload for Rich function as Rich > >>> function > >>>>>>>>>>>>>> implements > >>>>>>>>>>>>>>> *withKey* interface as a result they have same type. We > >>>>>>> differentiate > >>>>>>>>>>>>>> them > >>>>>>>>>>>>>>> while building the topology. > >>>>>>>>>>>>>>> We supported lambdas for *withKey* APIs because of the > >>> comment: > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> @Jeyhun: I did not put any thought into this, but can we > >> have > >>> a > >>>>>>>>>>>> design > >>>>>>>>>>>>>>>> that allows for both? Also, with regard to lambdas, it > >> might > >>>>> make > >>>>>>>>>>>>> sense > >>>>>>>>>>>>>>>> to allow for both `V -> newV` and `(K, V) -> newV` ? > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> However, I don't think that this complicates the overall > >>> design > >>>>>>>>>>>>>>> significantly. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Depending on what we want to support, it might make sense > to > >>>>>>>>>>>>>>>> include/exclude RichFunctions from this KIP -- and thus, > >> this > >>>>>>> also > >>>>>>>>>>>>>>>> determines if we should have a "ProcessorContext KIP" > >> before > >>>>>>> driving > >>>>>>>>>>>>>>>> this KIP further. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Based on our discussion I think we should keep Rich > >> functions > >>>>> as I > >>>>>>>>>>>>> don't > >>>>>>>>>>>>>>> think that they bring extra layer of overhead to library. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Any comments are appreciated. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Cheers, > >>>>>>>>>>>>>>> Jeyhun > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> On Tue, May 16, 2017 at 12:10 AM Matthias J. Sax < > >>>>>>>>>>>>> matth...@confluent.io> > >>>>>>>>>>>>>>> wrote: > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> Jeyhun, > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> thanks for the update. > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> I think supporting Lambdas for `withKey` and > >>>>>>> `AbstractRichFunction` > >>>>>>>>>>>>>>>> don't go together, as Lambdas are only supported for > >>> interfaces > >>>>>>>>>>>> AFAIK. > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> Thus, if we want to support Lambdas for `withKey`, we need > >> to > >>>>>>> have a > >>>>>>>>>>>>>>>> interface approach like this > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> - RichFunction -> only adding init() and close() > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> - ValueMapper > >>>>>>>>>>>>>>>> - ValueMapperWithKey > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> - RichValueMapper extends ValueMapperWithKey, > >> RichFunction > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> For this approach, AbstractRichFunction does not make > sense > >>>>>>> anymore, > >>>>>>>>>>>>> as > >>>>>>>>>>>>>>>> the only purpose of `RichFunction` is to allow the > >>>>>>> implementation of > >>>>>>>>>>>>>>>> init() and close() -- if you don't want those, you would > >>>>>>> implement a > >>>>>>>>>>>>>>>> different interface (ie, ValueMapperWithKey) > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> As an alternative, we could argue, that it is sufficient > to > >>>>>>> support > >>>>>>>>>>>>>>>> Lambdas for the "plain" API only, but not for any > "extended > >>>>> API". > >>>>>>>>>>>> For > >>>>>>>>>>>>>>>> this, RichFunction could add key+init+close and > >>>>>>> AbstractRichFunction > >>>>>>>>>>>>>>>> would allow to only care about getting the key. > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> Not sure, which one is better. I don't like the idea of > >> more > >>>>>>>>>>>>> overloaded > >>>>>>>>>>>>>>>> methods to get Lambdas for `withKey` interfaces too much > >>>>> because > >>>>>>> we > >>>>>>>>>>>>> have > >>>>>>>>>>>>>>>> already so many overlaods. On the other hand, I do see > >> value > >>> in > >>>>>>>>>>>>>>>> supporting Lambdas for `withKey`. > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> Depending on what we want to support, it might make sense > >> to > >>>>>>>>>>>>>>>> include/exclude RichFunctions from this KIP -- and thus, > >> this > >>>>>>> also > >>>>>>>>>>>>>>>> determines if we should have a "ProcessorContext KIP" > >> before > >>>>>>> driving > >>>>>>>>>>>>>>>> this KIP further. > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> Thoughts? > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> -Matthias > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> On 5/15/17 11:01 AM, Jeyhun Karimov wrote: > >>>>>>>>>>>>>>>>> Hi, > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> Sorry for super late response. Thanks for your comments. > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> I am not an expert on Lambdas. Can you elaborate a little > >>>>> bit? I > >>>>>>>>>>>>> cannot > >>>>>>>>>>>>>>>>>> follow the explanation in the KIP to see what the > problem > >>> is. > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> - From [1] says "A functional interface is an interface > >> that > >>>>> has > >>>>>>>>>>>> just > >>>>>>>>>>>>>> one > >>>>>>>>>>>>>>>>> abstract method, and thus represents a single function > >>>>>>> contract". > >>>>>>>>>>>>>>>>> So basically once we extend some interface from another > >> (in > >>>>> our > >>>>>>>>>>>> case, > >>>>>>>>>>>>>>>>> ValueMapperWithKey from ValueMapper) we cannot use > lambdas > >>> in > >>>>>>> the > >>>>>>>>>>>>>>>> extended > >>>>>>>>>>>>>>>>> interface. > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> Further comments: > >>>>>>>>>>>>>>>>>> - The KIP get a little hard to read -- can you maybe > >>>>> reformat > >>>>>>> the > >>>>>>>>>>>>>> wiki > >>>>>>>>>>>>>>>>>> page a little bit? I think using `CodeBlock` would help. > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> - I will work on the KIP. > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> - What about KStream-KTable joins? You don't have > >> overlaods > >>>>>>> added > >>>>>>>>>>>>> for > >>>>>>>>>>>>>>>>>> them. Why? (Even if I still hope that we don't need to > >> add > >>>>> any > >>>>>>> new > >>>>>>>>>>>>>>>>>> overloads) > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> - Actually there are more than one Processor and public > >> APIs > >>>>> to > >>>>>>> be > >>>>>>>>>>>>>>>>> changed (KStream-KTable > >>>>>>>>>>>>>>>>> joins is one case). However all of them has similar > >>> structure: > >>>>>>> we > >>>>>>>>>>>>>>>> overload > >>>>>>>>>>>>>>>>> the *method* with *methodWithKey*, > >>>>>>>>>>>>>>>>> wrap it into the Rich function, send to processor and > >> inside > >>>>> the > >>>>>>>>>>>>>>>> processor > >>>>>>>>>>>>>>>>> call *init* and *close* methods of the Rich function. > >>>>>>>>>>>>>>>>> As I wrote in KIP, I wanted to demonstrate the overall > >> idea > >>>>> with > >>>>>>>>>>>> only > >>>>>>>>>>>>>>>>> *ValueMapper* as the same can be applied to all changes. > >>>>>>>>>>>>>>>>> Anyway I will update the KIP. > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> - Why do we need `AbstractRichFunction`? > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> Instead of overriding the *init(ProcessorContext p)* and* > >>>>>>> close()* > >>>>>>>>>>>>>>>> methods > >>>>>>>>>>>>>>>>> in every Rich function with empty body like: > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> @Override > >>>>>>>>>>>>>>>>> void init(ProcessorContext context) {} > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> @Override > >>>>>>>>>>>>>>>>> void close () {} > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> I thought that we can override them once in > >>>>>>> *AbstractRichFunction* > >>>>>>>>>>>>> and > >>>>>>>>>>>>>>>>> extent new Rich functions from *AbstractRichFunction*. > >>>>>>>>>>>>>>>>> Basically this can eliminate code copy-paste and ease the > >>>>>>>>>>>>> maintenance. > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> - What about interfaces Initializer, ForeachAction, > >> Merger, > >>>>>>>>>>>>> Predicate, > >>>>>>>>>>>>>>>>>> Reducer? I don't want to say we should/need to add to > >> all, > >>>>> but > >>>>>>> we > >>>>>>>>>>>>>> should > >>>>>>>>>>>>>>>>>> discuss all of them and add where it does make sense > >> (e.g., > >>>>>>>>>>>>>>>>>> RichForachAction does make sense IMHO) > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> Definitely agree. As I said, the same technique applies > to > >>> all > >>>>>>> this > >>>>>>>>>>>>>>>>> interfaces and I didn't want to explode the KIP, just > >> wanted > >>>>> to > >>>>>>>>>>>> give > >>>>>>>>>>>>>> the > >>>>>>>>>>>>>>>>> overall intuition. > >>>>>>>>>>>>>>>>> However, I will update the KIP as I said. > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> Btw: I like the hierarchy `ValueXX` -- `ValueXXWithKey` > -- > >>>>>>>>>>>>>> `RichValueXX` > >>>>>>>>>>>>>>>>>> in general -- but why can't we do all this with > >> interfaces > >>>>>>> only? > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> Sure we can. However the main intuition is we should not > >>> force > >>>>>>>>>>>> users > >>>>>>>>>>>>> to > >>>>>>>>>>>>>>>>> implement *init(ProcessorContext)* and *close()* > functions > >>>>> every > >>>>>>>>>>>> time > >>>>>>>>>>>>>>>> they > >>>>>>>>>>>>>>>>> use Rich functions. > >>>>>>>>>>>>>>>>> If one needs, she can override the respective methods. > >>>>> However, > >>>>>>> I > >>>>>>>>>>>> am > >>>>>>>>>>>>>> open > >>>>>>>>>>>>>>>>> for discussion. > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> I'd rather not see the use of `ProcessorContext` spread > >> any > >>>>>>>>>>>> further > >>>>>>>>>>>>>> than > >>>>>>>>>>>>>>>>>> it currently is. So maybe we need another KIP that is > >> done > >>>>>>> before > >>>>>>>>>>>>>> this? > >>>>>>>>>>>>>>>>>> Otherwise i think the scope of this KIP is becoming too > >>>>> large. > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> That is good point. I wanted to make > >>> *init(ProcessorContext)* > >>>>>>>>>>>> method > >>>>>>>>>>>>>>>>> persistent among the library (which use ProcessorContext > >> as > >>> an > >>>>>>>>>>>>> input), > >>>>>>>>>>>>>>>>> therefore I put *ProcessorContext* as an input. > >>>>>>>>>>>>>>>>> So the important question is that (as @dguy and @mjsax > >>>>>>> mentioned) > >>>>>>>>>>>>>> whether > >>>>>>>>>>>>>>>>> continue this KIP without providing users an access to > >>>>>>>>>>>>>> *ProcessorContext* > >>>>>>>>>>>>>>>>> (change *init (ProcessorContext)* to * init()* ) or > >>>>>>>>>>>>>>>>> initiate another KIP before this. > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> [1] > >>>>>>>>>>>>>>>>> > >>> http://cr.openjdk.java.net/~mr/se/8/java-se-8-pfd-spec/java- > >>>>>>>>>>>>>>>> se-8-jls-pfd-diffs.pdf > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> Cheers, > >>>>>>>>>>>>>>>>> Jeyhun > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> On Mon, May 15, 2017 at 7:15 PM, Damian Guy < > >>>>>>> damian....@gmail.com> > >>>>>>>>>>>>>>>> wrote: > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> I'd rather not see the use of `ProcessorContext` spread > >>> any > >>>>>>>>>>>> further > >>>>>>>>>>>>>>>> than > >>>>>>>>>>>>>>>>>> it currently is. So maybe we need another KIP that is > >> done > >>>>>>> before > >>>>>>>>>>>>>> this? > >>>>>>>>>>>>>>>>>> Otherwise i think the scope of this KIP is becoming too > >>>>> large. > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> On Mon, 15 May 2017 at 18:06 Matthias J. Sax < > >>>>>>>>>>>> matth...@confluent.io > >>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> wrote: > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> I agree that that `ProcessorContext` interface is too > >>> broad > >>>>> in > >>>>>>>>>>>>>> general > >>>>>>>>>>>>>>>>>>> -- this is even true for transform/process, and it's > >> also > >>>>>>>>>>>> reflected > >>>>>>>>>>>>>> in > >>>>>>>>>>>>>>>>>>> the API improvement list we want to do. > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> https://cwiki.apache.org/confluence/display/KAFKA/ > >>>>>>>>>>>>>>>>>> Kafka+Streams+Discussions > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> So I am wondering, if you question the `RichFunction` > >>>>>>> approach in > >>>>>>>>>>>>>>>>>>> general? Or if you suggest to either extend the scope > of > >>>>> this > >>>>>>> KIP > >>>>>>>>>>>>> to > >>>>>>>>>>>>>>>>>>> include this---or maybe better, do another KIP for it > >> and > >>>>>>> delay > >>>>>>>>>>>>> this > >>>>>>>>>>>>>>>> KIP > >>>>>>>>>>>>>>>>>>> until the other one is done? > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> -Matthias > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> On 5/15/17 2:35 AM, Damian Guy wrote: > >>>>>>>>>>>>>>>>>>>> Thanks for the KIP. > >>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>> I'm not convinced on the `RichFunction` approach. Do > we > >>>>>>> really > >>>>>>>>>>>>> want > >>>>>>>>>>>>>> to > >>>>>>>>>>>>>>>>>>> give > >>>>>>>>>>>>>>>>>>>> every DSL method access to the `ProcessorContext` ? It > >>> has > >>>>> a > >>>>>>>>>>>> bunch > >>>>>>>>>>>>>> of > >>>>>>>>>>>>>>>>>>>> methods on it that seem in-appropriate for some of the > >>> DSL > >>>>>>>>>>>>> methods, > >>>>>>>>>>>>>>>>>> i.e, > >>>>>>>>>>>>>>>>>>>> `register`, `getStateStore`, `forward`, `schedule` > etc. > >>> It > >>>>> is > >>>>>>>>>>>> far > >>>>>>>>>>>>>> too > >>>>>>>>>>>>>>>>>>>> broad. I think it would be better to have a narrower > >>>>>>> interface > >>>>>>>>>>>>> like > >>>>>>>>>>>>>>>> the > >>>>>>>>>>>>>>>>>>>> `RecordContext` - remembering it is easier to add > >>>>>>>>>>>>>> methods/interfaces > >>>>>>>>>>>>>>>>>>> later > >>>>>>>>>>>>>>>>>>>> than to remove them > >>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>> On Sat, 13 May 2017 at 22:26 Matthias J. Sax < > >>>>>>>>>>>>> matth...@confluent.io > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> wrote: > >>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> Jeyhun, > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> I am not an expert on Lambdas. Can you elaborate a > >>> little > >>>>>>> bit? > >>>>>>>>>>>> I > >>>>>>>>>>>>>>>>>> cannot > >>>>>>>>>>>>>>>>>>>>> follow the explanation in the KIP to see what the > >>> problem > >>>>>>> is. > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> For updating the KIP title I don't know -- guess it's > >> up > >>>>> to > >>>>>>>>>>>> you. > >>>>>>>>>>>>>>>>>> Maybe a > >>>>>>>>>>>>>>>>>>>>> committer can comment on this? > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> Further comments: > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> - The KIP get a little hard to read -- can you maybe > >>>>>>> reformat > >>>>>>>>>>>>> the > >>>>>>>>>>>>>>>>>> wiki > >>>>>>>>>>>>>>>>>>>>> page a little bit? I think using `CodeBlock` would > >> help. > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> - What about KStream-KTable joins? You don't have > >>>>> overlaods > >>>>>>>>>>>>> added > >>>>>>>>>>>>>>>> for > >>>>>>>>>>>>>>>>>>>>> them. Why? (Even if I still hope that we don't need > to > >>> add > >>>>>>> any > >>>>>>>>>>>>> new > >>>>>>>>>>>>>>>>>>>>> overloads) > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> - Why do we need `AbstractRichFunction`? > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> - What about interfaces Initializer, ForeachAction, > >>>>> Merger, > >>>>>>>>>>>>>>>>>> Predicate, > >>>>>>>>>>>>>>>>>>>>> Reducer? I don't want to say we should/need to add to > >>> all, > >>>>>>> but > >>>>>>>>>>>> we > >>>>>>>>>>>>>>>>>> should > >>>>>>>>>>>>>>>>>>>>> discuss all of them and add where it does make sense > >>>>> (e.g., > >>>>>>>>>>>>>>>>>>>>> RichForachAction does make sense IMHO) > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> Btw: I like the hierarchy `ValueXX` -- > >> `ValueXXWithKey` > >>> -- > >>>>>>>>>>>>>>>>>> `RichValueXX` > >>>>>>>>>>>>>>>>>>>>> in general -- but why can't we do all this with > >>> interfaces > >>>>>>>>>>>> only? > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> -Matthias > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> On 5/11/17 7:00 AM, Jeyhun Karimov wrote: > >>>>>>>>>>>>>>>>>>>>>> Hi, > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> Thanks for your comments. I think we cannot extend > >> the > >>>>> two > >>>>>>>>>>>>>>>> interfaces > >>>>>>>>>>>>>>>>>>> if > >>>>>>>>>>>>>>>>>>>>> we > >>>>>>>>>>>>>>>>>>>>>> want to keep lambdas. I updated the KIP [1]. Maybe I > >>>>> should > >>>>>>>>>>>>> change > >>>>>>>>>>>>>>>>>> the > >>>>>>>>>>>>>>>>>>>>>> title, because now we are not limiting the KIP to > >> only > >>>>>>>>>>>>>> ValueMapper, > >>>>>>>>>>>>>>>>>>>>>> ValueTransformer and ValueJoiner. > >>>>>>>>>>>>>>>>>>>>>> Please feel free to comment. > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> [1] > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP- > >>>>>>>>>>>>>>>>>> 149%3A+Enabling+key+access+in+ValueTransformer%2C+ > >>>>>>>>>>>>>>>>>> ValueMapper%2C+and+ValueJoiner > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> Cheers, > >>>>>>>>>>>>>>>>>>>>>> Jeyhun > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> On Tue, May 9, 2017 at 7:36 PM Matthias J. Sax < > >>>>>>>>>>>>>>>>>> matth...@confluent.io> > >>>>>>>>>>>>>>>>>>>>>> wrote: > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> If `ValueMapperWithKey` extends `ValueMapper` we > >> don't > >>>>>>> need > >>>>>>>>>>>> the > >>>>>>>>>>>>>> new > >>>>>>>>>>>>>>>>>>>>>>> overlaod. > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> And yes, we need to do one check -- but this > happens > >>>>> when > >>>>>>>>>>>>>> building > >>>>>>>>>>>>>>>>>> the > >>>>>>>>>>>>>>>>>>>>>>> topology. At runtime (I mean after > >> KafkaStream#start() > >>>>> we > >>>>>>>>>>>> don't > >>>>>>>>>>>>>>>> need > >>>>>>>>>>>>>>>>>>> any > >>>>>>>>>>>>>>>>>>>>>>> check as we will always use `ValueMapperWithKey`) > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> -Matthias > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> On 5/9/17 2:55 AM, Jeyhun Karimov wrote: > >>>>>>>>>>>>>>>>>>>>>>>> Hi, > >>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>> Thanks for feedback. > >>>>>>>>>>>>>>>>>>>>>>>> Then we need to overload method > >>>>>>>>>>>>>>>>>>>>>>>> <VR> KStream<K, VR> mapValues(ValueMapper<? > super > >>> V, > >>>>> ? > >>>>>>>>>>>>> extends > >>>>>>>>>>>>>>>>>> VR> > >>>>>>>>>>>>>>>>>>>>>>>> mapper); > >>>>>>>>>>>>>>>>>>>>>>>> with > >>>>>>>>>>>>>>>>>>>>>>>> <VR> KStream<K, VR> > >> mapValues(ValueMapperWithKey<? > >>>>>>> super > >>>>>>>>>>>> V, > >>>>>>>>>>>>> ? > >>>>>>>>>>>>>>>>>>> extends > >>>>>>>>>>>>>>>>>>>>>>> VR> > >>>>>>>>>>>>>>>>>>>>>>>> mapper); > >>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>> and in runtime (inside processor) we still have to > >>>>> check > >>>>>>> it > >>>>>>>>>>>> is > >>>>>>>>>>>>>>>>>>>>>>> ValueMapper > >>>>>>>>>>>>>>>>>>>>>>>> or ValueMapperWithKey before wrapping it into the > >>> rich > >>>>>>>>>>>>> function. > >>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>> Please correct me if I am wrong. > >>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>> Cheers, > >>>>>>>>>>>>>>>>>>>>>>>> Jeyhun > >>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>> On Tue, May 9, 2017 at 10:56 AM Michal Borowiecki > < > >>>>>>>>>>>>>>>>>>>>>>>> michal.borowie...@openbet.com> wrote: > >>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>> +1 :) > >>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>> On 08/05/17 23:52, Matthias J. Sax wrote: > >>>>>>>>>>>>>>>>>>>>>>>>>> Hi, > >>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>> I was reading the updated KIP and I am > wondering, > >>> if > >>>>> we > >>>>>>>>>>>>> should > >>>>>>>>>>>>>>>> do > >>>>>>>>>>>>>>>>>>> the > >>>>>>>>>>>>>>>>>>>>>>>>>> design a little different. > >>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>> Instead of distinguishing between a RichFunction > >>> and > >>>>>>>>>>>>>>>>>>> non-RichFunction > >>>>>>>>>>>>>>>>>>>>>>> at > >>>>>>>>>>>>>>>>>>>>>>>>>> runtime level, we would use RichFunctions all > the > >>>>> time. > >>>>>>>>>>>>> Thus, > >>>>>>>>>>>>>> on > >>>>>>>>>>>>>>>>>>> the > >>>>>>>>>>>>>>>>>>>>>>> DSL > >>>>>>>>>>>>>>>>>>>>>>>>>> entry level, if a user provides a > >> non-RichFunction, > >>>>> we > >>>>>>>>>>>> wrap > >>>>>>>>>>>>> it > >>>>>>>>>>>>>>>>>> by a > >>>>>>>>>>>>>>>>>>>>>>>>>> RichFunction that is fully implemented by > >> Streams. > >>>>> This > >>>>>>>>>>>>>>>>>>> RichFunction > >>>>>>>>>>>>>>>>>>>>>>>>>> would just forward the call omitting the key: > >>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>> Just to sketch the idea (incomplete code > >> snippet): > >>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>> public StreamsRichValueMapper implements > >>>>>>>>>>>> RichValueMapper() > >>>>>>>>>>>>> { > >>>>>>>>>>>>>>>>>>>>>>>>>>> private ValueMapper userProvidedMapper; // > >> set > >>> by > >>>>>>>>>>>>>>>> constructor > >>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>> public VR apply(final K key, final V1 > value1, > >>>>>>> final V2 > >>>>>>>>>>>>>>>>>> value2) > >>>>>>>>>>>>>>>>>>> { > >>>>>>>>>>>>>>>>>>>>>>>>>>> return userProvidedMapper(value1, > >> value2); > >>>>>>>>>>>>>>>>>>>>>>>>>>> } > >>>>>>>>>>>>>>>>>>>>>>>>>>> } > >>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>> From a performance point of view, I am not sure > >> if > >>>>> the > >>>>>>>>>>>>>>>>>>>>>>>>>> "if(isRichFunction)" including casts etc would > >> have > >>>>>>> more > >>>>>>>>>>>>>>>> overhead > >>>>>>>>>>>>>>>>>>>>> than > >>>>>>>>>>>>>>>>>>>>>>>>>> this approach (we would do more nested method > >> call > >>>>> for > >>>>>>>>>>>>>>>>>>>>> non-RichFunction > >>>>>>>>>>>>>>>>>>>>>>>>>> which should be more common than RichFunctions). > >>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>> This approach should not effect lambdas (or do I > >>> miss > >>>>>>>>>>>>>>>> something?) > >>>>>>>>>>>>>>>>>>> and > >>>>>>>>>>>>>>>>>>>>>>>>>> might be cleaner, as we could have one more top > >>> level > >>>>>>>>>>>>>> interface > >>>>>>>>>>>>>>>>>>>>>>>>>> `RichFunction` with methods `init()` and > >> `close()` > >>>>> and > >>>>>>>>>>>> also > >>>>>>>>>>>>>>>>>>>>> interfaces > >>>>>>>>>>>>>>>>>>>>>>>>>> for `RichValueMapper` etc. (thus, no abstract > >>> classes > >>>>>>>>>>>>>> required). > >>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>> Any thoughts? > >>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>> -Matthias > >>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>> On 5/6/17 5:29 PM, Jeyhun Karimov wrote: > >>>>>>>>>>>>>>>>>>>>>>>>>>> Hi, > >>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>> Thanks for comments. I extended PR and KIP to > >>>>> include > >>>>>>>>>>>> rich > >>>>>>>>>>>>>>>>>>>>> functions. > >>>>>>>>>>>>>>>>>>>>>>> I > >>>>>>>>>>>>>>>>>>>>>>>>>>> will still have to evaluate the cost of deep > >>> copying > >>>>>>> of > >>>>>>>>>>>>> keys. > >>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>> Cheers, > >>>>>>>>>>>>>>>>>>>>>>>>>>> Jeyhun > >>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>> On Fri, May 5, 2017 at 8:02 PM Mathieu Fenniak > < > >>>>>>>>>>>>>>>>>>>>>>>>> mathieu.fenn...@replicon.com> > >>>>>>>>>>>>>>>>>>>>>>>>>>> wrote: > >>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>>> Hey Matthias, > >>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>>> My opinion would be that documenting the > >>>>>>> immutability of > >>>>>>>>>>>>> the > >>>>>>>>>>>>>>>>>> key > >>>>>>>>>>>>>>>>>>> is > >>>>>>>>>>>>>>>>>>>>>>> the > >>>>>>>>>>>>>>>>>>>>>>>>>>>> best approach available. Better than > requiring > >>> the > >>>>>>> key > >>>>>>>>>>>> to > >>>>>>>>>>>>>> be > >>>>>>>>>>>>>>>>>>>>>>>>> serializable > >>>>>>>>>>>>>>>>>>>>>>>>>>>> (as with Jeyhun's last pass at the PR), no > >>>>>>> performance > >>>>>>>>>>>>> risk. > >>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>>> It'd be different if Java had immutable type > >>>>>>> constraints > >>>>>>>>>>>>> of > >>>>>>>>>>>>>>>>>> some > >>>>>>>>>>>>>>>>>>>>>>> kind. > >>>>>>>>>>>>>>>>>>>>>>>>> :-) > >>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>>> Mathieu > >>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>>> On Fri, May 5, 2017 at 11:31 AM, Matthias J. > >> Sax > >>> < > >>>>>>>>>>>>>>>>>>>>>>>>> matth...@confluent.io> > >>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote: > >>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> Agreed about RichFunction. If we follow this > >>> path, > >>>>>>> it > >>>>>>>>>>>>>> should > >>>>>>>>>>>>>>>>>>> cover > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> all(?) DSL interfaces. > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> About guarding the key -- I am still not sure > >>> what > >>>>>>> to > >>>>>>>>>>>> do > >>>>>>>>>>>>>>>> about > >>>>>>>>>>>>>>>>>>>>> it... > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> Maybe it might be enough to document it (and > >>> name > >>>>>>> the > >>>>>>>>>>>> key > >>>>>>>>>>>>>>>>>>>>> parameter > >>>>>>>>>>>>>>>>>>>>>>>>> like > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> `readOnlyKey` to make is very clear). > >>> Ultimately, > >>>>> I > >>>>>>>>>>>> would > >>>>>>>>>>>>>>>>>> prefer > >>>>>>>>>>>>>>>>>>>>> to > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> guard against any modification, but I have no > >>> good > >>>>>>> idea > >>>>>>>>>>>>> how > >>>>>>>>>>>>>>>> to > >>>>>>>>>>>>>>>>>>> do > >>>>>>>>>>>>>>>>>>>>>>>>> this. > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> Not sure what others think about the risk of > >>>>>>> corrupted > >>>>>>>>>>>>>>>>>>>>> partitioning > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> (what would be a user error and we could say, > >>>>> well, > >>>>>>> bad > >>>>>>>>>>>>>> luck, > >>>>>>>>>>>>>>>>>>> you > >>>>>>>>>>>>>>>>>>>>>>> got > >>>>>>>>>>>>>>>>>>>>>>>>> a > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> bug in your code, that's not our fault), vs > >> deep > >>>>>>> copy > >>>>>>>>>>>>> with > >>>>>>>>>>>>>> a > >>>>>>>>>>>>>>>>>>>>>>> potential > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> performance hit (that we can't quantity atm > >>>>> without > >>>>>>> any > >>>>>>>>>>>>>>>>>>>>> performance > >>>>>>>>>>>>>>>>>>>>>>>>>>>> test). > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> We do have a performance system test. Maybe > >> it's > >>>>>>> worth > >>>>>>>>>>>>> for > >>>>>>>>>>>>>>>> you > >>>>>>>>>>>>>>>>>>> to > >>>>>>>>>>>>>>>>>>>>>>>>> apply > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> the deep copy strategy and run the test. It's > >>> very > >>>>>>>>>>>> basic > >>>>>>>>>>>>>>>>>>>>> performance > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> test only, but might give some insight. If > you > >>>>> want > >>>>>>> to > >>>>>>>>>>>> do > >>>>>>>>>>>>>>>>>> this, > >>>>>>>>>>>>>>>>>>>>> look > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> into folder "tests" for general test setup, > >> and > >>>>> into > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> "tests/kafaktests/benchmarks/streams" to > find > >>> find > >>>>>>> the > >>>>>>>>>>>>> perf > >>>>>>>>>>>>>>>>>>> test. > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> -Matthias > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> On 5/5/17 8:55 AM, Jeyhun Karimov wrote: > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Hi Matthias, > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I think extending KIP to include > >> RichFunctions > >>>>>>> totally > >>>>>>>>>>>>>>>> makes > >>>>>>>>>>>>>>>>>>>>>>> sense. > >>>>>>>>>>>>>>>>>>>>>>>>>>>> So, > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> we don't want to guard the keys because it > >> is > >>>>>>>>>>>> costly. > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> If we introduce RichFunctions I think it > >> should > >>>>>>> not be > >>>>>>>>>>>>>>>>>> limited > >>>>>>>>>>>>>>>>>>>>> only > >>>>>>>>>>>>>>>>>>>>>>>>>>>> the 3 > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> interfaces proposed in KIP but to wide range > >> of > >>>>>>>>>>>>>> interfaces. > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Please correct me if I am wrong. > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Cheers, > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Jeyhun > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Fri, May 5, 2017 at 12:04 AM Matthias J. > >>> Sax < > >>>>>>>>>>>>>>>>>>>>>>>>> matth...@confluent.io > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote: > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> One follow up. There was this email on the > >>> user > >>>>>>> list: > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>> http://search-hadoop.com/m/Kafka/uyzND17KhCaBzPSZ1?subj= > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>> Shouldn+t+the+initializer+of+a+stream+aggregate+accept+the+ > >>>>>>>>>>>>>>>>>> key+ > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> It might make sense so include > Initializer, > >>>>> Adder, > >>>>>>>>>>>> and > >>>>>>>>>>>>>>>>>>>>> Substractor > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> inferface, too. > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> And we should double check if there are > >> other > >>>>>>>>>>>> interface > >>>>>>>>>>>>>> we > >>>>>>>>>>>>>>>>>>> might > >>>>>>>>>>>>>>>>>>>>>>>>> miss > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> atm. > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> -Matthias > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On 5/4/17 1:31 PM, Matthias J. Sax wrote: > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Thanks for updating the KIP. > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Deep copying the key will work for sure, > >> but > >>> I > >>>>> am > >>>>>>>>>>>>>> actually > >>>>>>>>>>>>>>>>>> a > >>>>>>>>>>>>>>>>>>>>>>> little > >>>>>>>>>>>>>>>>>>>>>>>>>>>> bit > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> worried about performance impact... We > >> might > >>>>>>> want to > >>>>>>>>>>>>> do > >>>>>>>>>>>>>>>>>> some > >>>>>>>>>>>>>>>>>>>>> test > >>>>>>>>>>>>>>>>>>>>>>>>> to > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> quantify this impact. > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Btw: this remind me about the idea of > >>>>>>> `RichFunction` > >>>>>>>>>>>>>>>>>>> interface > >>>>>>>>>>>>>>>>>>>>>>> that > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> would allow users to access record > metadata > >>>>> (like > >>>>>>>>>>>>>>>>>> timestamp, > >>>>>>>>>>>>>>>>>>>>>>>>> offset, > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> partition etc) within DSL. This would be > a > >>>>>>> similar > >>>>>>>>>>>>>>>> concept. > >>>>>>>>>>>>>>>>>>>>>>> Thus, I > >>>>>>>>>>>>>>>>>>>>>>>>>>>> am > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> wondering, if it would make sense to > >> enlarge > >>>>> the > >>>>>>>>>>>> scope > >>>>>>>>>>>>>> of > >>>>>>>>>>>>>>>>>>> this > >>>>>>>>>>>>>>>>>>>>>>> KIP > >>>>>>>>>>>>>>>>>>>>>>>>> by > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> that? WDYT? > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> -Matthias > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On 5/3/17 2:08 AM, Jeyhun Karimov wrote: > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Hi Mathieu, > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Thanks for feedback. I followed similar > >>>>> approach > >>>>>>>>>>>> and > >>>>>>>>>>>>>>>>>> updated > >>>>>>>>>>>>>>>>>>>>> PR > >>>>>>>>>>>>>>>>>>>>>>>>> and > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> KIP > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> accordingly. I tried to guard the key in > >>>>>>> Processors > >>>>>>>>>>>>>>>>>> sending > >>>>>>>>>>>>>>>>>>> a > >>>>>>>>>>>>>>>>>>>>>>> copy > >>>>>>>>>>>>>>>>>>>>>>>>>>>> of > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> an > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> actual key. > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Because I am doing deep copy of an > >> object, I > >>>>>>> think > >>>>>>>>>>>>>> memory > >>>>>>>>>>>>>>>>>>> can > >>>>>>>>>>>>>>>>>>>>> be > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> bottleneck > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> in some use-cases. > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Cheers, > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Jeyhun > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Tue, May 2, 2017 at 5:10 PM Mathieu > >>>>> Fenniak < > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> mathieu.fenn...@replicon.com> > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote: > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Hi Jeyhun, > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> This approach would change ValueMapper > >>>>>>> (...etc) to > >>>>>>>>>>>>> be > >>>>>>>>>>>>>>>>>>>>> classes, > >>>>>>>>>>>>>>>>>>>>>>>>>>>> rather > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> than > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> interfaces, which is also a backwards > >>>>>>> incompatible > >>>>>>>>>>>>>>>>>> change. > >>>>>>>>>>>>>>>>>>>>> An > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> alternative > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> approach that would be backwards > >> compatible > >>>>>>> would > >>>>>>>>>>>> be > >>>>>>>>>>>>>> to > >>>>>>>>>>>>>>>>>>>>> define > >>>>>>>>>>>>>>>>>>>>>>>>> new > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> interfaces, and provide overrides where > >>> those > >>>>>>>>>>>>>> interfaces > >>>>>>>>>>>>>>>>>>> are > >>>>>>>>>>>>>>>>>>>>>>>>> used. > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Unfortunately, making the key parameter > >> as > >>>>>>> "final" > >>>>>>>>>>>>>>>>>> doesn't > >>>>>>>>>>>>>>>>>>>>>>> change > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> much > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> about guarding against key change. It > >> only > >>>>>>>>>>>> prevents > >>>>>>>>>>>>>> the > >>>>>>>>>>>>>>>>>>>>>>>>> parameter > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> variable > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> from being reassigned. If the key type > >> is > >>> a > >>>>>>>>>>>> mutable > >>>>>>>>>>>>>>>>>> object > >>>>>>>>>>>>>>>>>>>>>>> (eg. > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> byte[]), > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> it can still be mutated. (eg. key[0] = > >> 0). > >>>>> But > >>>>>>>>>>>> I'm > >>>>>>>>>>>>>> not > >>>>>>>>>>>>>>>>>>>>> really > >>>>>>>>>>>>>>>>>>>>>>>>> sure > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> there's > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> much that can be done about that. > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Mathieu > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Mon, May 1, 2017 at 5:39 PM, Jeyhun > >>>>> Karimov > >>>>>>> < > >>>>>>>>>>>>>>>>>>>>>>>>>>>> je.kari...@gmail.com > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote: > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Thanks for comments. > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> The concerns makes sense. Although we > >> can > >>>>>>> guard > >>>>>>>>>>>> for > >>>>>>>>>>>>>>>>>>>>> immutable > >>>>>>>>>>>>>>>>>>>>>>>>> keys > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> in > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> current implementation (with few > >>> changes), I > >>>>>>>>>>>> didn't > >>>>>>>>>>>>>>>>>>> consider > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> backward > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> compatibility. > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> In this case 2 solutions come to my > >> mind. > >>> In > >>>>>>> both > >>>>>>>>>>>>>>>> cases, > >>>>>>>>>>>>>>>>>>>>> user > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> accesses > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> the > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> key in Object type, as passing extra > >> type > >>>>>>>>>>>> parameter > >>>>>>>>>>>>>>>> will > >>>>>>>>>>>>>>>>>>>>> break > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> backwards-compatibility. So user has > >> to > >>>>> cast > >>>>>>> to > >>>>>>>>>>>>>> actual > >>>>>>>>>>>>>>>>>>> key > >>>>>>>>>>>>>>>>>>>>>>>>> type. > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>> > >> > >> -- > >> -Cheers > >> > >> Jeyhun > >> > > > > > > > > -- -- Guozhang
