[hibernate-dev] HHH-8999/HHH-10413 and Comparable IDs

[Emmanuel Bernard]

Version can be a byte[] ? I thought JPA was restricting version types
actually.

On Thu 2016-03-10 12:19, Gail Badner wrote:
> As I mentioned before, it is not acceptable for the comparator for
> PrimitiveByteArrayTypeDescripter to be IncomparableComparator because a
> version attribute can be of type byte[]. We definitely do not want all
> byte[] version values to compare as equal. A Comparator needs to be
> implemented at least for PrimitiveByteArrayTypeDescripter.
> 
> I'm OK with changing getComparator() for PrimitiveCharacterArrayTypeDescriptor,
> ByteArrayTypeDescriptor, and
> CharacterArrayTypeDescriptor to return IncomparableComparator.INSTANCE. I
> think that is a safer option than B) (making
> IncomparableComparator.INSTANCE the default if the type is not assignable
> to Comparable).
> 
> I'm also OK with using what I implemented in the pull request.
> 
> On Thu, Mar 10, 2016 at 7:27 AM, Steve Ebersole <steve at hibernate.org> wrote:
> 
> > Personally I think using IncomparableComparator or null here makes the
> > most sense.  I'm not really sure what the sort ordering of a byte array
> > would "mean".
> >
> > On Thu, Mar 10, 2016, 1:41 AM Gail Badner <gbadner at redhat.com> wrote:
> >
> >> I've created a pull request implements option A) (creates comparators
> >> for PrimitiveByteArrayTypeDescriptor,
> >> PrimitiveCharacterArrayTypeDescriptor, ByteArrayTypeDescriptor, and
> >> CharacterArrayTypeDescriptor.
> >>
> >> I'm still not sure if it makes sense to have a comparators for arrays.
> >>
> >> Steve, please take a look at the pull request [1] and let me know if this
> >> looks reasonable to you.
> >>
> >> Thanks,
> >> Gail
> >>
> >> [1] https://github.com/hibernate/hibernate-orm/pull/1294
> >>
> >> On Thu, Mar 3, 2016 at 2:44 AM, Gail Badner <gbadner at redhat.com> wrote:
> >>
> >> > Missed something. Please see below...
> >> >
> >> > On Wed, Mar 2, 2016 at 11:49 PM, Gail Badner <gbadner at redhat.com>
> >> wrote:
> >> >
> >> >> ExecutableList#add attempts to keep track of whether the Executable
> >> >> objects are sorted. [1]
> >> >>
> >> >> Except for entity insert actions (which use InsertActionSorter),
> >> >> ExecutableList#add uses the following to determine if adding the
> >> current
> >> >> Executable to the end invalidates the sort:
> >> >>
> >> >> if ( previousLast != null && previousLast.compareTo( executable ) > 0
> >> ) {
> >> >>     sorted = false;
> >> >> }
> >> >>
> >> >> EntityAction#compareTo compares the IDs for the current and previous
> >> >> EntityAction if they have different entity names; similarly,
> >> >> CollectionAction compares the collection keys for the current and
> >> previous
> >> >> CollectionAction if they have different entity names.
> >> >>
> >> >> In most cases, the ID class implements Comparable, although I don't
> >> know
> >> >> of any requirement that says this needs to be true.
> >> >>
> >> >> This breaks down when a byte[] ID is used as described by HHH-8999. The
> >> >> reason is that AbstractTypeDescriptor#comparator is null because it is
> >> >> assigned like this:
> >> >>
> >> >> this.comparator = Comparable.class.isAssignableFrom( type )
> >> >>         ? (Comparator<T>) ComparableComparator.INSTANCE
> >> >>         : null;
> >> >>
> >> >> PrimitiveByteArrayTypeDescriptor does not override
> >> >> AbstractTypeDescriptor#getComparator, so null is returned ultimately
> >> >> causing a NullPointerException in AbstractStandardBasicType#compare:
> >> >>
> >> >> public final int compare(Object x, Object y) {
> >> >>     return javaTypeDescriptor.getComparator().compare( (T) x, (T) y );
> >> >> }
> >> >>
> >> >> The same happens for PrimitiveCharacterArrayTypeDescriptor,
> >> >> ByteArrayTypeDescriptor, and CharacterArrayTypeDescriptor. Are there
> >> others?
> >> >>
> >> >> According to HHH-10413, this also affects version attributes.
> >> >>
> >> >> Here are a couple of alternatives:
> >> >>
> >> >> A) create comparators for PrimitiveCharacterArrayTypeDescriptor,
> >> >> ByteArrayTypeDescriptor, and CharacterArrayTypeDescriptor.
> >> >>
> >> >
> >> > PrimitiveByteArrayTypeDescript should also be in the list for A).
> >> >
> >> >
> >> >>
> >> >>
> >> >> B) Change AbstractTypeDescriptor#comparator to:
> >> >>
> >> >> this.comparator = Comparable.class.isAssignableFrom( type )
> >> >>         ? (Comparator<T>) ComparableComparator.INSTANCE
> >> >>         : IncomparableComparator.INSTANCE;
> >> >>
> >> >> IncomparableComparator#compare always returns 0, so that comparison
> >> would
> >> >> never invalidate the sort.
> >> >>
> >> >>
> >> > B) is not acceptable for PrimitiveByteArrayTypeDescripter because a
> >> > version attribute can be of type byte[]. We definitely do not want all
> >> > byte[] version values to compare as equal. A Comparator needs to be
> >> > implemented at least for PrimitiveByteArrayTypeDescripter.
> >> >
> >> > The following question only applies to
> >> > PrimitiveCharacterArrayTypeDescriptor, ByteArrayTypeDescriptor, and
> >> > CharacterArrayTypeDescriptor.
> >> >
> >> >
> >> >> Does it make sense to sort Executable objects by an ID or collection
> >> key
> >> >> that is an array? In other words, would such IDs be generated in a
> >> natural
> >> >> order? If not, I think B) makes more sense.
> >> >>
> >> >
> >> >> Thoughts?
> >> >>
> >> >> Thanks,
> >> >> Gail
> >> >>
> >> >> [1]
> >> >>
> >> https://github.com/hibernate/hibernate-orm/blob/master/hibernate-core/src/main/java/org/hibernate/engine/spi/ExecutableList.java#L194
> >> >>
> >> >
> >> >
> >> _______________________________________________
> >> hibernate-dev mailing list
> >> hibernate-dev at lists.jboss.org
> >> https://lists.jboss.org/mailman/listinfo/hibernate-dev
> >>
> >
> _______________________________________________
> hibernate-dev mailing list
> hibernate-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/hibernate-dev