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 17:27:47 GMT

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

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

bq. One of the advantages of using an enum that I did not enumerate was the possibility of
performing more efficient despatch for "mixed" clustering data

Alright, fair enough. I guess one of the thing that put me off is that {{COMPARE_COMPARABLE}}
sounds really weird/unintuitive to me. If we go with the {{compareValue}} change I discuss
above, then I'd suggest renaming the enum to something like:
{noformat}
enum ComparisonType { UNCOMPARABLE, BYTE_ORDER, CUSTOM }
{noformat}
and the {{compareValue}} is discussed above could be instead {{compareCustom()}}. We'd also
wouldn't need {{isByteOrderComparable}} I believe since it would be directly handled by the
{{compare}} (which won't be a virtual call).

bq. Admittedly I haven't confirmed this, but it looks fine to me

I read that too quickly and missed the package check, my bad. I guess it's note entirely full-proof,
but we probably can't do much better short of having a white-list which would be ugly so I'm
fine with that.

bq. I prefer to log more often than less, since there's more chance of it being spotted. I
don't think we rebuild so often - just during schema changes, no?

{{rebuild}} happens every time a {{CFMetaData}} is created and validated, which means at least
on every startup and multiple times per schema change (since it's called during validation),
and that's not counting the case I forget.

A bit of context is also that I strongly suspect that while there is likely people already
using custom types, I don't think there is all that many created new custom types now that
we provide a relatively rich amount of types out of the box (that was not always the case).
So that I'm nore worried about annoying people that have existing custom types, for which
the message is basically useless since it's not currently actionable and they can't miss the
change anyway since they'll have to update their own code.

In fact, I'm not even really sure a warning is necessary in the first place. As said, for
people already having a custom type, the warning is mostly annoyance.  And for new user than
might decide to write a custom type, I think being extra clear on the javadoc of the {{compareCustom()}}
method that you should not implement it in newly created types would be fair enough warnings
(we can additional add to the {{AbstractType}} javadoc that creating custom subclasses is
frown upon nowadays).


> 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