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, 17 Aug 2015 16:18:46 GMT

    [ https://issues.apache.org/jira/browse/CASSANDRA-9901?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14699767#comment-14699767
] 

Sylvain Lebresne commented on CASSANDRA-9901:
---------------------------------------------

>From a quick look, it looks like the patch will log warning for every internal type that
is not byte comparable, which is not what we want. Also, logging in {{CFMetadaData.rebuild}}
is going to be more noisy than necessary since that's called reasonably often. Ideally we'd
want to only warn when the type is used in the first place.

On a less important note, but I'm not a fan of using an enum. I'm not convinced it'll add
clarity for the user, but on the other side, we don't validate that what the enum said is
consistent with what the compare method does which feels error prone to me. I also find it
more clunky (than just making {{isByteOrderComparable}} abstract) but that's probably more
a question of personal taste.

What I could suggest, on top of making {{isByteOrderComparable}} abstract, is to create some
{{compareValue()}} (or some other name) that would be the existing {{compare()}}, and the
{{compare()}} we actually used would basically be:
{noformat}
public final int compare(ByteBuffer b1, ByteBuffer b2)
{
    return isByteBufferComparable() ? ByteBufferUtil.compareUnsigned(b1, b2) : compareValue(b1,
b2);
}
{noformat}
And {{compareValue}} would be abstract, throwing {{UnsupportedOperationException}} by default,
and only the implementations that are not bytes comparable would have to provide it.



> 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.0 rc1
>
>
> 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