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

[Gail Badner]

Hibernate supports byte[] versions. You can see that
org.hibernate.type.BinaryType implements VersionType<byte[]>. [1]

The comment in BinaryType#seed says:
		// Note : simply returns null for seed() and next() as the only known
		// 		application of binary types for versioning is for use with the
		// 		TIMESTAMP datatype supported by Sybase and SQL Server, which
		// 		are completely db-generated values...

There is also a unit test,
org.hibernate.test.version.sybase.SybaseTimestampVersioningTest that
maps:

<version name="timestamp" type="binary" generated="always">
    <column name="ts" sql-type="timestamp"/>
</version>

[1] https://github.com/hibernate/hibernate-orm/blob/master/hibernate-core/src/main/java/org/hibernate/type/BinaryType.java


On Fri, Mar 11, 2016 at 7:36 AM, Emmanuel Bernard <emmanuel at hibernate.org>
wrote:

> 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
>