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

[Steve Ebersole]

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
>