[ https://issues.apache.org/jira/browse/CASSANDRA-8527?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16322908#comment-16322908 ]
Jon Haddad commented on CASSANDRA-8527: --------------------------------------- Overall, nice patch, I've got a few nits though: * The complexity of {{UnfilteredPartitionIterator.applyToRow}} has increased enough where it would be really helpful to add some comments explaining what is going on. Please describe the _intent_ of the code, not what it does. * Please reorder this: {code} else if (!row.primaryKeyLivenessInfo().isLive(ReadCommand.this.nowInSec()) && row.hasDeletion(ReadCommand.this.nowInSec()) && !hasTombstones) {code} to this: {code} else if ( !hasTombstones && !row.primaryKeyLivenessInfo().isLive(ReadCommand.this.nowInSec()) && row.hasDeletion(ReadCommand.this.nowInSec())) {code} The check on the bool is cheaper than the other checks and if that fails we can short circuit the other checks. * Please add some comments on the tests you've created. There's about 150 lines of code and no comments to explain what's going on or what's being tested. When you've revised the patch, could you link a CircleCI build result? See https://cassandra.apache.org/doc/latest/development/testing.html Thanks, Jon > Account for range tombstones wherever we account for tombstones > --------------------------------------------------------------- > > Key: CASSANDRA-8527 > URL: https://issues.apache.org/jira/browse/CASSANDRA-8527 > Project: Cassandra > Issue Type: Improvement > Reporter: Sylvain Lebresne > Assignee: Alexander Dejanovski > Fix For: 4.x > > Attachments: CASSANDRA-8527-4.x.diff > > > As discussed in CASSANDRA-8477, we should make sure the tombstone thresholds > also apply to range tombstones, since they poses the same problems than cell > tombstones. -- This message was sent by Atlassian JIRA (v6.4.14#64029) --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org