[ https://issues.apache.org/jira/browse/CASSANDRA-9986?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15112669#comment-15112669 ]
Sylvain Lebresne commented on CASSANDRA-9986: --------------------------------------------- The patch looks good in general. One minor remark is that you've pulled the {{getPosition(key, SSTableReader.Operator.EQ)}} from {{SSTableIterator}} and {{SSTableReversedIterator}} to inside {{BigTableReader}}, which makes sense but should probably be pushed a little further. In particular, we could do the same {{indexEntry == null}} check in the other {{BigTableReader.iterator()}} call and then make the first call simply delegate to the 2nd one. After which we can assert the index entry is not null in {{AbstractSSTableIterator}} and remove the related branch in the ctor. And I'm fine doing those minor changes on commit btw so +1. > Remove SliceableUnfilteredRowIterator > ------------------------------------- > > Key: CASSANDRA-9986 > URL: https://issues.apache.org/jira/browse/CASSANDRA-9986 > Project: Cassandra > Issue Type: Sub-task > Reporter: Benedict > Assignee: Benedict > Priority: Minor > Fix For: 3.x > > > After CASSANDRA-9932, there is only one instance where this class is needed, > and that is {{SSTableIterator}}. It would be much simpler if, like in > {{AbstractBTreePartition}}, the slices were passed into the > {{SSTableIterator}} on construction. This would make the control flow easier > to follow both: > * logically, because: > ** memtables and sstables would have the same access pattern; and > ** our SSTableIterator would not have two modes of (parallel) operation, > which is kind of confusing (the slices are independent of the non-sliced > iteration) > * in the debugger, because we would have one fewer wrapping iterator -- This message was sent by Atlassian JIRA (v6.3.4#6332)