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

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(a)redhat.com&gt; 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(a)redhat.com&gt; wrote: > > > Missed something. Please see below... > > > > On Wed, Mar 2, 2016 at 11:49 PM, Gail Badner <gbadner(a)redhat.com&gt; > 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... > >> > > > > > _______________________________________________ > hibernate-dev mailing list > hibernate-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/hibernate-dev >