[
https://issues.apache.org/jira/browse/CASSANDRA-3885?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13289300#comment-13289300
]
David Alves commented on CASSANDRA-3885:
----------------------------------------
Thank you for the thorough review, Sylvain.
My notes are:
With regard to bugs that's why theres' extensive testing.
With regard to the symmetry of forward and reverse it is indeed broken but
because the two operations (IMO) are really not symmetric since columns are
always read from left to right. Not adding unnecessary data to blockColumns
prevents having to check ranges for every tuple that comes out of it as was
done before, which i think is an advantage in most cases but that bears the
penalty of making it impossible to add out-of-order values (as was done before
because values were always range-checked twice). Moreover the code is now
naturally more complex because the operation is more complex.
All in all I think that most of what you're saying makes sense, but since
things are working (and commented throughout) I'd prefer to leave it. The whole
cache thing is arguable though and I'd be happy to add the bb checks to make
sure only relevant data is added to it if that's the consensus.
With regard to the Pair<ByteBuffer,ByteBuffer> I agree, I think I jumped the
gun on that. I particularly don't like the the unchecked casts when using
arrays. I'll change that as per your suggestion.
> Support multiple ranges in SliceQueryFilter
> -------------------------------------------
>
> Key: CASSANDRA-3885
> URL: https://issues.apache.org/jira/browse/CASSANDRA-3885
> Project: Cassandra
> Issue Type: Sub-task
> Components: Core
> Reporter: Jonathan Ellis
> Assignee: David Alves
> Fix For: 1.2
>
> Attachments: CASSANDRA-3885.patch, CASSANDRA-3885.patch,
> CASSANDRA-3885.patch
>
>
> This is logically a subtask of CASSANDRA-2710, but Jira doesn't allow
> sub-sub-tasks.
> We need to support multiple ranges in a SliceQueryFilter, and we want
> querying them to be efficient, i.e., one pass through the row to get all of
> the ranges, rather than one pass per range.
> Supercolumns are irrelevant since the goal is to replace them anyway. Ignore
> supercolumn-related code or rip it out, whichever is easier.
> This is ONLY dealing with the storage engine part, not the StorageProxy and
> Command intra-node messages or the Thrift or CQL client APIs. Thus, a unit
> test should be added to ColumnFamilyStoreTest to demonstrate that it works.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
