[
https://issues.apache.org/jira/browse/CASSANDRA-9901?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14709292#comment-14709292
]
Sylvain Lebresne commented on CASSANDRA-9901:
---------------------------------------------
The approach looks good but I still have a few small remarks:
* I think calling {{checkComparable}} in {{CFMetadata.rebuild}} is enough (no
need to duplicate the call in the builder, since this will end up in rebuild
anyway).
* I'd left {{ClusteringComparator.compareComponent}} as is (before the patch).
{{AbstractType.compare}} is now monomorphic (and we should make it {{final}} to
ensure it stays true), so I'd expect it to be inlined and the "indirection" to
not cost us anything really. This would avoid the imo slightly ugly form of the
new condition in particular.
* I'd make {{AbstractType.compareCustom()}} throw an
{{UnsupportedOperationException}} by default, thus not requiring it in
{{BYTE_ORDER}} implementations. This going more in the idea of making it easier
for {{BYTE_ORDER}} implementations, since that's what we want to favor. A
small javadoc on that method would be nice too.
* Since we keep a {{isByteOrderComparable}} boolean as a field in
{{AbstractType}}, it'd make more sense to return that in
{{isByteOrderComparable}}. If we want to ensure {{compare()}} is not called for
a {{NOT_COMPARABLE}}, then an assert in {{compare()}} is probably better.
* The comments on {{ComparisonType}} should probably be slightly updated. On
{{BYTE_ORDER}} for instance, the comment should probably just say "The values
of this type are compared by their sequence of unsigned bytes", since this is
now enforced. Similarly, {{CUSTOM}} comment needs update.
> Make AbstractType.isByteOrderComparable abstract
> ------------------------------------------------
>
> Key: CASSANDRA-9901
> URL: https://issues.apache.org/jira/browse/CASSANDRA-9901
> Project: Cassandra
> Issue Type: Improvement
> Components: Core
> Reporter: Benedict
> Assignee: Benedict
> Fix For: 3.0 beta 2
>
>
> I can't recall _precisely_ what was agreed at the NGCC, but I'm reasonably
> sure we agreed to make this method abstract, put some javadoc explaining we
> may require fields to yield true in the near future, and potentially log a
> warning on startup if a user-defined type returns false.
> This should make it into 3.0, IMO, so that we can look into migrating to
> byte-order comparable types in the post-3.0 world.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
