In my view, we should still handle automatic hashcode generation for all
known types, boxed or unboxed. Most of the time, especially in INSERT
statements, the hashcode will be a primitive type or String or Date, etc.
We should automatically handle these types without any additional
configuration.
Other than that, I also agree with Vladimir's proposal. However, I do not
understand point (5). Can you please clarify?
D.
On Sun, Aug 7, 2016 at 11:38 PM, Sergi Vladykin <sergi.vlady...@gmail.com>
wrote:
> I like what Vladimir proposed.
>
> As for `encoded string` implementation, I guess it should be just an
> implementation with a list of fields which have to participate in hashCode
> and equals. I guess in 99% of cases everyone generates hashCode and equals
> with IDE, we will just implement the same logic in our HashResolver. I
> guess IDEA, Eclipse, Netbeans generate this code in the same way, but of
> course we should check.
>
> Sergi
>
>
> 2016-08-08 11:54 GMT+03:00 Alexey Goncharuk <alexey.goncha...@gmail.com>:
>
> > I like the idea with string-based BinaryEqualsHashCodeResolver a lot. I
> > think it should be as flexible as possible. Since EqualsHashCode resolver
> > becomes a part of cache configuration, we get another part of
> configuration
> > which must be deployed on server nodes and defeats benefits of
> > BinaryMarshaller.
> >
> > Also, I think it would be great to have an ability to add Comparable
> > implementation in this interface.
> >
> > 2016-08-08 10:30 GMT+03:00 Vladimir Ozerov <voze...@gridgain.com>:
> >
> > > Dmitriy,
> > >
> > > >>> 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.
> > > We will receive billion questions like "why did I put object to cache,
> > but
> > > cannot retrieve it?" when users created an object using builder
> > (explicitly
> > > or using SQL INSERT), and we auto-generated wrong hash code. "Wrong"
> > means
> > > the one which doesn't match a hash code of a relevant Java class.
> > >
> > > I think we can do the following:
> > > 1) Add "has hash code" flag as Denis suggested.
> > > 2) If object without a hash code is put to cache, throw an exception.
> > > 3) Add *BinaryEqualsHashCodeResolver *interface.
> > > 4) Add default implementation which will auto-generate hash code.
> *Print
> > a
> > > warning when auto-generation occurs*, so that user is aware that he is
> > > likely to have problems with normal GETs/PUTs.
> > > 5) Add another implementation which will use encoded string to
> calculate
> > a
> > > hash code. E.g. *new BinaryEqualsHashCodeResolver("{a} * 31 + {b}")*.
> > > Originally proposed by Yakov some time ago.
> > >
> > > Thoughts?
> > >
> > > Vladimir.
> > >
> > >
> > > On Sun, Aug 7, 2016 at 10:26 PM, Dmitriy Setrakyan <
> > dsetrak...@apache.org>
> > > wrote:
> > >
> > > > On Sat, Aug 6, 2016 at 8:11 PM, Sergi Vladykin <
> > sergi.vlady...@gmail.com
> > > >
> > > > wrote:
> > > >
> > > > > 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.
> > > > >
> > > >
> > > > Agree. Sounds like the best approach. I would still prefer 1 message
> in
> > > the
> > > > log per JVM stating that the system has generated automatic hashCode
> > and
> > > > there is a way to override it manually.
> > > >
> > > >
> > > > >
> > > > > 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).
> > > > >
> > > >
> > > > I think there will be some use cases where users will want to control
> > the
> > > > hash code themselves, perhaps for the types that we don't serialize
> > > > automatically. I think we need to provide that capability.
> > > >
> > > >
> > > > >
> > > > > 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
> > > > > > > > > > >> >>>>>>
> > > > > > > > > > >> >>>>>
> > > > > > > > > > >> >>
> > > > > > > > > > >> >>
> > > > > > > > > > >>
> > > > > > > > > > >>
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
