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

[Gail Badner]

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