I think it makes sense to always generate hashCode but allow overriding it if really needed. Because this requirement to set it manually is a priori usability issue and error prone approach.
TBH, I do not even understand why we allow overriding it at all, if key hashCode is not defined by it's fields, then there are good chances that it will work wrong (current implementations of offheap depends on serialized key equality for example). Sergi 2016-08-06 22:58 GMT+03:00 Alexander Paschenko < alexander.a.pasche...@gmail.com>: > Warning is of little help if there's no way to retrieve object from the > cache by given key later, isn't it? > > — Alex > 6 авг. 2016 г. 8:04 PM пользователь "Dmitriy Setrakyan" < > dsetrak...@apache.org> написал: > > > Sergi, you are right. We keep jumping back and forth on this issue. > > > > How about this suggestion. We don't create any new configuration > > properties. Every time an object is used as a key in a cache, we > > automatically generate hashcode for it. The first time we do it, we print > > out a warning in the log, that the hashcodes will be automatically > > generated, if not provided. > > > > This is as clean as it will ever get. > > > > Thoughts? > > > > D. > > > > > > On Sat, Aug 6, 2016 at 1:25 AM, Sergi Vladykin <sergi.vlady...@gmail.com > > > > wrote: > > > > > Keep in mind that we need to support affinity keys in BinaryObjects. It > > > means that it will have to consist from at least two fields: one for > > exact > > > equality check and another one for hashCode calculation. > > > > > > It looks to me that configuration of cache key is a part of cache > > > configuration. Thus cache key builder must be bound to cache. > > > > > > Sergi > > > > > > 2016-08-06 6:18 GMT+03:00 Dmitriy Setrakyan <dsetrak...@apache.org>: > > > > > > > How about we add a property - auto-generate hashCode() in binary > > > > configuration. If set, then we auto-generate the hashCode() every > time > > a > > > > binary object is built. > > > > > > > > Thoughts? > > > > > > > > On Fri, Aug 5, 2016 at 5:29 AM, Alexander Paschenko < > > > > alexander.a.pasche...@gmail.com> wrote: > > > > > > > > > Denis, > > > > > > > > > > Thank you very much for your proposed solution, I will reflect it > in > > > > > issue's comments and implement this check in code. Most likely this > > > > > has to be an issue by itself. > > > > > > > > > > However, it all does not answer the main question of this thread - > > how > > > > > do we automatically supply hash codes for newly built binary > objects? > > > > > This is very important for convenient use of SQL inserts. Please, > > all, > > > > > share your thoughts. > > > > > > > > > > - Alex > > > > > > > > > > 2016-08-03 3:23 GMT+03:00 Dmitriy Setrakyan <dsetrak...@apache.org > >: > > > > > > On Tue, Aug 2, 2016 at 7:36 AM, Denis Magda <dma...@gridgain.com > > > > > > wrote: > > > > > > > > > > > >> Vova, > > > > > >> > > > > > >> By default hasCode field will be initialized this way in > > > > > >> BinaryObjectBuilderImpl > > > > > >> > > > > > >> /** */ > > > > > >> private static Integer DFLT_HASH_CODE_MAGIC = 0; > > > > > >> > > > > > >> /** */ > > > > > >> private Integer hashCode = DFLT_HASH_CODE_MAGIC; > > > > > >> > > > > > >> Also we will introduce the following flag in BinaryUtils > > > > > >> > > > > > >> /** Flag indicating whether as hashCode was explicitly set or > not. > > > **/ > > > > > >> public static final short EMPTY_HASH_CODE_FLAG = 0x0032; > > > > > >> > > > > > >> At the BinaryObjectBuilder.build() time we will perform the > check > > > > below > > > > > >> and write hashCodeFlag. > > > > > >> > > > > > >> > > > > > >> short hashCodeFlag = hashCode == DFLT_HASH_CODE_MAGIC ? > > > > > >> BinaryUtils.EMPTY_HASH_CODE_FLAG : 0; > > > > > >> > > > > > >> // Write hashCode flag as well. > > > > > >> writer.postWrite(true, registeredType, hashCode, hashCodeFlag); > > > > > >> > > > > > >> Later when a BinaryObject is used as a key we can check the > value > > of > > > > > >> BinaryUtils.EMPTY_HASH_CODE_FLAG > > > > > >> and throw an exception if it’s not empty. > > > > > >> > > > > > >> Makes sense? > > > > > >> > > > > > > > > > > > > It does to me. If there are no objections, then we should list > this > > > in > > > > > the > > > > > > ticket and implement the proposed suggestion of throwing > exception > > > if a > > > > > > binary object without hashcode is used as a key. > > > > > > > > > > > > > > > > > >> > > > > > >> — > > > > > >> Denis > > > > > >> > > > > > >> > On Aug 2, 2016, at 7:09 AM, Vladimir Ozerov < > > voze...@gridgain.com > > > > > > > > > >> wrote: > > > > > >> > > > > > > >> > Denis, > > > > > >> > > > > > > >> > I hardly can imagine how it could work in our binary protocol. > > Can > > > > you > > > > > >> > please elaborate? > > > > > >> > > > > > > >> > On Tue, Aug 2, 2016 at 5:06 PM, Denis Magda < > > dma...@gridgain.com> > > > > > wrote: > > > > > >> > > > > > > >> >> There is a technique we already use to see if a field is > > > > initialized > > > > > by > > > > > >> >> application code. By default a field has to be a reference > to a > > > > > >> predefined > > > > > >> >> object and the reference comparison (not equals) is used to > > check > > > > if > > > > > the > > > > > >> >> field is initialized by the user. > > > > > >> >> > > > > > >> >> Refer to IgniteConfiguration.failureDetectionTimeout and > > > > > >> >> IgniteSpiAdapter.initializeFailureDetectionTimeout for more > > > > details. > > > > > >> >> > > > > > >> >> — > > > > > >> >> Denis > > > > > >> >> > > > > > >> >>> On Aug 2, 2016, at 12:14 AM, Alexander Paschenko < > > > > > >> >> alexander.a.pasche...@gmail.com> wrote: > > > > > >> >>> > > > > > >> >>> Dmitriy, > > > > > >> >>> > > > > > >> >>> Good point, however, currently there's no way to distinguish > > > hash > > > > > code > > > > > >> >>> of zero which is a valid case from missing hash code. We > > > probably > > > > > >> >>> should enhance binary builder for it to handle this case. > > > > > >> >>> > > > > > >> >>> - Alex > > > > > >> >>> > > > > > >> >>> 2016-08-02 9:47 GMT+03:00 Dmitriy Setrakyan < > > > > dsetrak...@apache.org > > > > > >: > > > > > >> >>>> On Mon, Aug 1, 2016 at 11:38 PM, Vladimir Ozerov < > > > > > >> voze...@gridgain.com> > > > > > >> >>>> wrote: > > > > > >> >>>> > > > > > >> >>>>> Andrey, > > > > > >> >>>>> > > > > > >> >>>>> The question is when to print this warning. I doubt we can > > > > print a > > > > > >> >> warning > > > > > >> >>>>> when calling *BinaryObjectBuilder.build() *method, because > > an > > > > > object > > > > > >> >>>>> without a hash code is normal situation. > > > > > >> >>>>> > > > > > >> >>>>> > > > > > >> >>>> I would not only print warning, but throw exception, if an > > > object > > > > > >> >> without a > > > > > >> >>>> hashCode ends up on a put or read operation in cache. > > > > > >> >>>> > > > > > >> >>>> > > > > > >> >>>>> On Tue, Aug 2, 2016 at 9:00 AM, Andrey Gura < > > > ag...@gridgain.com > > > > > > > > > > >> >> wrote: > > > > > >> >>>>> > > > > > >> >>>>>> I think we also should print some warning in case when > > > > hashCode() > > > > > >> >> wasn't > > > > > >> >>>>>> called on BinaryObject explicitly. > > > > > >> >>>>>> > > > > > >> >>>>>> On Tue, Aug 2, 2016 at 2:20 AM, Dmitriy Setrakyan < > > > > > >> >> dsetrak...@apache.org > > > > > >> >>>>>> > > > > > >> >>>>>> wrote: > > > > > >> >>>>>> > > > > > >> >>>>>>> On Mon, Aug 1, 2016 at 10:01 AM, Alexey Goncharuk < > > > > > >> >>>>>>> alexey.goncha...@gmail.com> wrote: > > > > > >> >>>>>>> > > > > > >> >>>>>>>> Dmitriy, > > > > > >> >>>>>>>> > > > > > >> >>>>>>>> The question is how do you calculate the value of the > > > > > hashCode? Do > > > > > >> >>>>> you > > > > > >> >>>>>>> want > > > > > >> >>>>>>>> it to be specified explicitly in INSERT statement? > > > > > >> >>>>>>>> > > > > > >> >>>>>>> > > > > > >> >>>>>>> I think optionally we should allow to specify hashCode > as > > > part > > > > > of > > > > > >> the > > > > > >> >>>>>>> INSERT statement. However, if it is not specified, we > > should > > > > > >> >> calculate > > > > > >> >>>>> it > > > > > >> >>>>>>> automatically based in the key fields defined in the > > > > > schema/type. > > > > > >> >>>>> Agree? > > > > > >> >>>>>>> > > > > > >> >>>>>>> > > > > > >> >>>>>>>> > > > > > >> >>>>>>>> 2016-08-01 19:47 GMT+03:00 Dmitriy Setrakyan < > > > > > >> dsetrak...@apache.org > > > > > >> >>>>>> : > > > > > >> >>>>>>>> > > > > > >> >>>>>>>>> Alex, > > > > > >> >>>>>>>>> > > > > > >> >>>>>>>>> In your case, why not just explicitly set hashcode > every > > > > time > > > > > you > > > > > >> >>>>>>> create > > > > > >> >>>>>>>> an > > > > > >> >>>>>>>>> object? There is BinaryObjectBuilder.hashCode(...) > > > method. > > > > > >> >>>>>>>>> > > > > > >> >>>>>>>>> D. > > > > > >> >>>>>>>>> > > > > > >> >>>>>>>>> On Mon, Aug 1, 2016 at 7:42 AM, al.psc < > > > > > >> >>>>>>> alexander.a.pasche...@gmail.com> > > > > > >> >>>>>>>>> wrote: > > > > > >> >>>>>>>>> > > > > > >> >>>>>>>>>> Guys, > > > > > >> >>>>>>>>>> > > > > > >> >>>>>>>>>> It seems like this problem has become an important > one > > > once > > > > > >> >>>>> again. > > > > > >> >>>>>>>>>> In the course of working on > > > > > >> >>>>>>>>>> https://issues.apache.org/jira/browse/IGNITE-2294 > (DML > > > > > support) > > > > > >> >>>>>>>> there's > > > > > >> >>>>>>>>>> need > > > > > >> >>>>>>>>>> to support binary marshaller. And, although we can > > build > > > > just > > > > > >> >>>>>>>>> BinaryObject > > > > > >> >>>>>>>>>> and put it to cache, without adequate hash code it > > won't > > > be > > > > > >> >>>>> stored > > > > > >> >>>>>>>>>> properly. > > > > > >> >>>>>>>>>> Currently SQL MERGE works simply by deserializing > newly > > > > built > > > > > >> >>>>>> object, > > > > > >> >>>>>>>> but > > > > > >> >>>>>>>>>> it's obviously wrong and is just a workaround rather > a > > > > > solution. > > > > > >> >>>>>>>>>> Has anyone come with possible design proposals for > this > > > > > >> problem's > > > > > >> >>>>>>>>> solution? > > > > > >> >>>>>>>>>> > > > > > >> >>>>>>>>>> Thanks. > > > > > >> >>>>>>>>>> > > > > > >> >>>>>>>>>> - Alex > > > > > >> >>>>>>>>>> > > > > > >> >>>>>>>>>> > > > > > >> >>>>>>>>>> > > > > > >> >>>>>>>>>> -- > > > > > >> >>>>>>>>>> View this message in context: > > > > > >> >>>>>>>>>> > > > > > >> >>>>>>>>> > > > > > >> >>>>>>>> > > > > > >> >>>>>>> > > > > > >> >>>>>> > > > > > >> >>>>> > > > > > >> >> > > > > > >> http://apache-ignite-developers.2346864.n4.nabble. > > > > > com/All-BinaryObjects-created-by-BinaryObjectBuilder-stored- > > > > > at-the-same-partition-by-default-tp8042p10304.html > > > > > >> >>>>>>>>>> Sent from the Apache Ignite Developers mailing list > > > archive > > > > > at > > > > > >> >>>>>>>>> Nabble.com. > > > > > >> >>>>>>>>>> > > > > > >> >>>>>>>>> > > > > > >> >>>>>>>> > > > > > >> >>>>>>> > > > > > >> >>>>>> > > > > > >> >>>>>> > > > > > >> >>>>>> > > > > > >> >>>>>> -- > > > > > >> >>>>>> Andrey Gura > > > > > >> >>>>>> GridGain Systems, Inc. > > > > > >> >>>>>> www.gridgain.com > > > > > >> >>>>>> > > > > > >> >>>>> > > > > > >> >> > > > > > >> >> > > > > > >> > > > > > >> > > > > > > > > > > > > > > >
