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-7089) Some problems with typed ByteBuffer comparisons?
Date Fri, 25 Apr 2014 13:34:16 GMT

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

Sylvain Lebresne commented on CASSANDRA-7089:
---------------------------------------------

For the record, looking more closely, 1.2 and 2.0 do use ColumnIdentifier.compareTo (through
CFDefinition#metdata TreeMap). I'll add a specific Comparator instead of the compareTo though
(not entirely convinced a compareTo is more risky in that instance but I don't mind being
too careful).

I'll also note, for the sake of having a complete record, that this only matter for the ordering
of columns in ResultSet metadata, for which we've never really made much ordering promises
( we're not absolutely bound to UTF8Type strictly speaking, we're really just sorting for
the sake of not having a totally random). That said, proper UTF8 ordering is overall better
than the weird ass signed comparison that ByteBuffer does, so I'll change it in 1.2 and 2.0
as well. 

> Some problems with typed ByteBuffer comparisons?
> ------------------------------------------------
>
>                 Key: CASSANDRA-7089
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-7089
>             Project: Cassandra
>          Issue Type: Bug
>          Components: Core
>            Reporter: Benedict
>            Assignee: Sylvain Lebresne
>            Priority: Minor
>             Fix For: 2.1 beta2
>
>         Attachments: 0001-Remove-ColumnIdentifier.compareTo.txt, 0002-Don-t-compare-components-with-equals.txt
>
>
> ColumnIdentifier.compareTo() appears subtly broken: it looks to me that we should be
using ByteBufferUtil.compareUnsigned() instead of bytes.compareTo(), since they are meant
to be UTF8Type. I think it would be nice to drop this compareTo method entirely, as it's only
used by CFMetaData.regularColumnComparator and it seems possible to misuse accidentally at
a later date since it only works for CQL columns, but a ColumnIdentifier is used for thrift
columns as well.
> There's a related problem with CellName.isPrefixOf, where we are using equals() instead
of type.compareTo() == 0, which could break anyone misusing our old friend Boolean as a clustering
column.



--
This message was sent by Atlassian JIRA
(v6.2#6252)

Mime
View raw message