cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sylvain Lebresne (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CASSANDRA-9901) Make AbstractType.isByteOrderComparable abstract
Date Mon, 24 Aug 2015 13:35:46 GMT

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

Mime
View raw message