This is an automated email from the ASF dual-hosted git repository.
ifesdjeen pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/cassandra.git
The following commit(s) were added to refs/heads/trunk by this push:
new 9bc8d0b Avoid executing commands that have guaranteed empty bounds in
slices filter
9bc8d0b is described below
commit 9bc8d0b452aeb7aaa2005e710fc3de0998172738
Author: Alex Petrov <[email protected]>
AuthorDate: Wed Feb 10 15:39:41 2021 +0100
Avoid executing commands that have guaranteed empty bounds in slices filter
Patch by Alex Petrov; reviewed by Marcus Eriksson and Aleksey Yeschenko for
CASSANDRA-16435.
---
.../cassandra/cql3/statements/SelectStatement.java | 2 +-
.../cassandra/db/SinglePartitionReadCommand.java | 3 +++
src/java/org/apache/cassandra/db/Slice.java | 23 ++++++++++++++-----
.../db/filter/AbstractClusteringIndexFilter.java | 5 +++++
.../cassandra/db/filter/ClusteringIndexFilter.java | 4 +++-
.../db/filter/ClusteringIndexSliceFilter.java | 7 ++++++
.../cassandra/distributed/test/PagingTest.java | 26 ++++++++++++++++++++++
.../test/ReadRepairEmptyRangeTombstonesTest.java | 20 ++++++++---------
.../cql3/validation/operations/SelectTest.java | 2 ++
9 files changed, 74 insertions(+), 18 deletions(-)
diff --git a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java
b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java
index 6e2baf6..8e0df45 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 217f4f1..2b5b20b 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 1aaf430..8956bd1 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 c28117c..63c2783 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 d3c6826..6ea0435 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 6711325..5df98c3 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 c0d8fe3..3dcb490 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<Object[]> 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 bcc77e0..c07b128 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 d7c1e25..d0493cf 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));
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
