diff --git a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java index 6e2baf6f52..8e0df451a5 100644 --- a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java @@ -512,7 +512,7 @@ public class SelectStatement implements CQLStatement return ReadQuery.empty(table); ClusteringIndexFilter filter = makeClusteringIndexFilter(options, columnFilter); - if (filter == null) + if (filter == null || filter.isEmpty(table.comparator)) return ReadQuery.empty(table); RowFilter rowFilter = getRowFilter(options); diff --git a/src/java/org/apache/cassandra/db/SinglePartitionReadCommand.java b/src/java/org/apache/cassandra/db/SinglePartitionReadCommand.java index 217f4f19ef..2b5b20b11e 100644 --- a/src/java/org/apache/cassandra/db/SinglePartitionReadCommand.java +++ b/src/java/org/apache/cassandra/db/SinglePartitionReadCommand.java @@ -382,6 +382,9 @@ public class SinglePartitionReadCommand extends ReadCommand implements SinglePar public PartitionIterator execute(ConsistencyLevel consistency, ClientState clientState, long queryStartNanoTime) throws RequestExecutionException { + if (clusteringIndexFilter.isEmpty(metadata().comparator)) + return EmptyIterators.partition(); + return StorageProxy.read(Group.one(this), consistency, clientState, queryStartNanoTime); } diff --git a/src/java/org/apache/cassandra/db/Slice.java b/src/java/org/apache/cassandra/db/Slice.java index 1aaf4301f3..8956bd178e 100644 --- a/src/java/org/apache/cassandra/db/Slice.java +++ b/src/java/org/apache/cassandra/db/Slice.java @@ -195,26 +195,37 @@ public class Slice if (reversed) { int cmp = comparator.compare(lastReturned, start); - if (cmp < 0 || (!inclusive && cmp == 0)) + assert cmp != 0; + // start is > than lastReturned; got nothing to return no more + if (cmp < 0) return null; cmp = comparator.compare(end, lastReturned); - if (cmp < 0 || (inclusive && cmp == 0)) + assert cmp != 0; + if (cmp < 0) return this; - return new Slice(start, inclusive ? ClusteringBound.inclusiveEndOf(lastReturned) : ClusteringBound.exclusiveEndOf(lastReturned)); + Slice slice = new Slice(start, inclusive ? ClusteringBound.inclusiveEndOf(lastReturned) : ClusteringBound.exclusiveEndOf(lastReturned)); + if (slice.isEmpty(comparator)) + return null; + return slice; } else { int cmp = comparator.compare(end, lastReturned); - if (cmp < 0 || (!inclusive && cmp == 0)) + assert cmp != 0; + if (cmp < 0) return null; cmp = comparator.compare(lastReturned, start); - if (cmp < 0 || (inclusive && cmp == 0)) + assert cmp != 0; + if (cmp < 0) return this; - return new Slice(inclusive ? ClusteringBound.inclusiveStartOf(lastReturned) : ClusteringBound.exclusiveStartOf(lastReturned), end); + Slice slice = new Slice(inclusive ? ClusteringBound.inclusiveStartOf(lastReturned) : ClusteringBound.exclusiveStartOf(lastReturned), end); + if (slice.isEmpty(comparator)) + return null; + return slice; } } diff --git a/src/java/org/apache/cassandra/db/filter/AbstractClusteringIndexFilter.java b/src/java/org/apache/cassandra/db/filter/AbstractClusteringIndexFilter.java index c28117c7b3..63c2783e0a 100644 --- a/src/java/org/apache/cassandra/db/filter/AbstractClusteringIndexFilter.java +++ b/src/java/org/apache/cassandra/db/filter/AbstractClusteringIndexFilter.java @@ -42,6 +42,11 @@ public abstract class AbstractClusteringIndexFilter implements ClusteringIndexFi return reversed; } + public boolean isEmpty(ClusteringComparator comparator) + { + return false; + } + protected abstract void serializeInternal(DataOutputPlus out, int version) throws IOException; protected abstract long serializedSizeInternal(int version); diff --git a/src/java/org/apache/cassandra/db/filter/ClusteringIndexFilter.java b/src/java/org/apache/cassandra/db/filter/ClusteringIndexFilter.java index d3c6826c43..6ea0435b54 100644 --- a/src/java/org/apache/cassandra/db/filter/ClusteringIndexFilter.java +++ b/src/java/org/apache/cassandra/db/filter/ClusteringIndexFilter.java @@ -37,7 +37,7 @@ import org.apache.cassandra.schema.TableMetadata; */ public interface ClusteringIndexFilter { - public static Serializer serializer = AbstractClusteringIndexFilter.serializer; + public static final Serializer serializer = AbstractClusteringIndexFilter.serializer; public enum Kind { @@ -64,6 +64,8 @@ public interface ClusteringIndexFilter */ public boolean isReversed(); + public boolean isEmpty(ClusteringComparator comparator); + /** * Returns a filter for continuing the paging of this filter given the last returned clustering prefix. * diff --git a/src/java/org/apache/cassandra/db/filter/ClusteringIndexSliceFilter.java b/src/java/org/apache/cassandra/db/filter/ClusteringIndexSliceFilter.java index 67113253e4..5df98c3e70 100644 --- a/src/java/org/apache/cassandra/db/filter/ClusteringIndexSliceFilter.java +++ b/src/java/org/apache/cassandra/db/filter/ClusteringIndexSliceFilter.java @@ -56,6 +56,13 @@ public class ClusteringIndexSliceFilter extends AbstractClusteringIndexFilter return slices.size() == 1 && !slices.hasLowerBound() && !slices.hasUpperBound(); } + // Whether or not it is guaranteed that slices are empty. Since we'd like to avoid iteration in general case, + // we rely on Slices#forPaging and SelectStatement#makeSlices to skip empty bounds. + public boolean isEmpty(ClusteringComparator comparator) + { + return slices.isEmpty(); + } + public boolean selects(Clustering clustering) { return slices.selects(clustering); diff --git a/test/distributed/org/apache/cassandra/distributed/test/PagingTest.java b/test/distributed/org/apache/cassandra/distributed/test/PagingTest.java index c0d8fe309c..3dcb490801 100644 --- a/test/distributed/org/apache/cassandra/distributed/test/PagingTest.java +++ b/test/distributed/org/apache/cassandra/distributed/test/PagingTest.java @@ -18,15 +18,18 @@ package org.apache.cassandra.distributed.test; +import java.io.IOException; import java.util.Iterator; import com.google.common.collect.Iterators; import org.junit.Test; import org.apache.cassandra.distributed.Cluster; +import org.apache.cassandra.distributed.api.ConsistencyLevel; import static org.apache.cassandra.distributed.api.ConsistencyLevel.QUORUM; import static org.apache.cassandra.distributed.shared.AssertUtils.assertRows; +import static org.apache.cassandra.distributed.shared.AssertUtils.row; public class PagingTest extends TestBaseImpl { @@ -78,4 +81,27 @@ public class PagingTest extends TestBaseImpl } } } + + @Test + public void testPagingWithRangeTombstones() throws IOException + { + try (Cluster cluster = init(Cluster.build(2).start())) + { + cluster.schemaChange("CREATE TABLE " + KEYSPACE + ".tbl (pk int, ck int, regular int, PRIMARY KEY (pk, ck))"); + cluster.coordinator(1).execute("DELETE FROM " + KEYSPACE + ".tbl WHERE pk = 1 AND ck > 1 AND ck < 10", ConsistencyLevel.ALL); + cluster.coordinator(1).execute("insert into " + KEYSPACE + ".tbl (pk, ck, regular) values (1,1,1)", ConsistencyLevel.ALL); + cluster.coordinator(1).execute("insert into " + KEYSPACE + ".tbl (pk, ck, regular) values (1,2,2)", ConsistencyLevel.ALL); + cluster.coordinator(1).execute("insert into " + KEYSPACE + ".tbl (pk, ck, regular) values (1,3,3)", ConsistencyLevel.ALL); + cluster.forEach((node) -> node.flush(KEYSPACE)); + Iterator iter = cluster.coordinator(1).executeWithPaging("SELECT pk,ck,regular FROM " + KEYSPACE + ".tbl " + + "WHERE pk=? AND ck>=? ORDER BY ck DESC;", + ConsistencyLevel.QUORUM, 1, + 1, 1); + + assertRows(iter, + row(1, 3, 3), + row(1, 2, 2), + row(1, 1, 1)); + } + } } diff --git a/test/distributed/org/apache/cassandra/distributed/test/ReadRepairEmptyRangeTombstonesTest.java b/test/distributed/org/apache/cassandra/distributed/test/ReadRepairEmptyRangeTombstonesTest.java index bcc77e0958..c07b128e89 100644 --- a/test/distributed/org/apache/cassandra/distributed/test/ReadRepairEmptyRangeTombstonesTest.java +++ b/test/distributed/org/apache/cassandra/distributed/test/ReadRepairEmptyRangeTombstonesTest.java @@ -135,11 +135,11 @@ public class ReadRepairEmptyRangeTombstonesTest extends TestBaseImpl "WITH CLUSTERING ORDER BY (c %s) AND read_repair='%s'") .mutate(1, "DELETE FROM %s USING TIMESTAMP 1 WHERE k=0 AND c>0 AND c<3") .mutate(1, "INSERT INTO %s (k, s) VALUES (0, 0)") - .assertRowsDistributed("SELECT * FROM %s WHERE k=0 AND c>1 and c<=1", 1) + .assertRowsDistributed("SELECT * FROM %s WHERE k=0 AND c>1 and c<=1", 0) .assertRowsDistributed("SELECT * FROM %s WHERE k=0 AND c>2 and c<=2", 0) - .assertRowsInternal("SELECT * FROM %s", row(0, null, 0)) + .assertRowsInternal("SELECT * FROM %s") .mutate(2, "DELETE FROM %s WHERE k=0 AND c>0 AND c<3") - .assertRowsInternal("SELECT * FROM %s", row(0, null, 0)); + .assertRowsInternal("SELECT * FROM %s"); } /** @@ -155,10 +155,10 @@ public class ReadRepairEmptyRangeTombstonesTest extends TestBaseImpl .mutate(1, "INSERT INTO %s (k, c) VALUES (0, 3)") .mutate(1, "INSERT INTO %s (k, c) VALUES (0, 4)") .assertRowsDistributed("SELECT * FROM %s WHERE k=0 AND c>=2 AND c<=3", - paging ? (flush ? 3 : 2) : 1, + paging ? 2 : 1, row(0, 2), row(0, 3)) .assertRowsDistributed("SELECT * FROM %s WHERE k=0 AND c>=3 AND c<=4", - paging && flush ? 2 : 1, + 1, row(0, 3), row(0, 4)) .assertRowsInternal("SELECT * FROM %s", row(0, 2), row(0, 3), row(0, 4)) .mutate(2, "DELETE FROM %s WHERE k=0 AND c>=1 AND c<=5") @@ -169,7 +169,7 @@ public class ReadRepairEmptyRangeTombstonesTest extends TestBaseImpl * Test range queries asking for a not-empty range targeting rows overlapping with a tombstone range in one replica. */ @Test - public void testRangeQueriesWithRowsOverlappingWithTombstoneRangeStart() + public void testRangeQueriesWithRowsOvetrlappingWithTombstoneRangeStart() { tester().createTable("CREATE TABLE %s(k int, c int, PRIMARY KEY (k, c)) " + "WITH CLUSTERING ORDER BY (c %s) AND read_repair='%s'") @@ -181,10 +181,10 @@ public class ReadRepairEmptyRangeTombstonesTest extends TestBaseImpl .mutate(1, "INSERT INTO %s (k, c) VALUES (0, 5)") .mutate(1, "INSERT INTO %s (k, c) VALUES (0, 6)") .assertRowsDistributed("SELECT c FROM %s WHERE k=0 AND c>=1 AND c<=4", - paging ? (flush && !reverse ? 5 : 4) : 1, + paging ? 4 : 1, row(1), row(2), row(3), row(4)) .assertRowsDistributed("SELECT c FROM %s WHERE k=0 AND c>=2 AND c<=5", - paging && flush && !reverse ? 2 : 1, + 1, row(2), row(3), row(4), row(5)) .assertRowsInternal("SELECT c FROM %s", row(1), row(2), row(3), row(4), row(5)) .mutate(2, "DELETE FROM %s WHERE k=0 AND c>=1 AND c<=6") @@ -207,10 +207,10 @@ public class ReadRepairEmptyRangeTombstonesTest extends TestBaseImpl .mutate(1, "INSERT INTO %s (k, c) VALUES (0, 5)") .mutate(1, "INSERT INTO %s (k, c) VALUES (0, 6)") .assertRowsDistributed("SELECT c FROM %s WHERE k=0 AND c>=2 AND c<=5", - paging ? (flush && reverse ? 5 : 4) : 1, + paging ? 4 : 1, row(2), row(3), row(4), row(5)) .assertRowsDistributed("SELECT c FROM %s WHERE k=0 AND c>=3 AND c<=6", - paging && flush && reverse ? 2 : 1, + 1, row(3), row(4), row(5), row(6)) .assertRowsInternal("SELECT c FROM %s", row(2), row(3), row(4), row(5), row(6)) .mutate(2, "DELETE FROM %s WHERE k=0 AND c>=1 AND c<=6") diff --git a/test/unit/org/apache/cassandra/cql3/validation/operations/SelectTest.java b/test/unit/org/apache/cassandra/cql3/validation/operations/SelectTest.java index d7c1e25100..d0493cf8fc 100644 --- a/test/unit/org/apache/cassandra/cql3/validation/operations/SelectTest.java +++ b/test/unit/org/apache/cassandra/cql3/validation/operations/SelectTest.java @@ -2472,6 +2472,8 @@ public class SelectTest extends CQLTester assertRows(execute("SELECT * FROM %s WHERE pk = 1 AND c1 > 1 AND c2 > 2 AND c3 = 3 AND v = 3 ALLOW FILTERING;"), row(1, 3, 3, 3, 3)); + assertRows(execute("SELECT * FROM %s WHERE pk = 1 AND c1 IN(0,1,2) AND c2 > 1 AND c2 < 1 AND v = 3 ALLOW FILTERING;")); + assertRows(execute("SELECT * FROM %s WHERE pk = 1 AND c1 IN(0,1,2) AND c2 = 1 AND v = 3 ALLOW FILTERING;"), row(1, 1, 1, 3, 3));