You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by bd...@apache.org on 2018/12/13 22:22:39 UTC
cassandra git commit: Catch empty/invalid bounds in SelectStatement
Repository: cassandra
Updated Branches:
refs/heads/trunk d414c1cd8 -> a41b861fa
Catch empty/invalid bounds in SelectStatement
Patch by Blake Eggleston; Reviewed by Aleksey Yeschenko for CASSANDRA-14849
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/a41b861f
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/a41b861f
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/a41b861f
Branch: refs/heads/trunk
Commit: a41b861fa4d4acfbcce13dd62b1e8f48be22f8ed
Parents: d414c1c
Author: Blake Eggleston <bd...@gmail.com>
Authored: Thu Oct 25 13:12:02 2018 -0700
Committer: Blake Eggleston <bd...@gmail.com>
Committed: Thu Dec 13 14:20:45 2018 -0800
----------------------------------------------------------------------
CHANGES.txt | 1 +
.../cql3/statements/SelectStatement.java | 8 ++-
src/java/org/apache/cassandra/db/Slice.java | 10 +++-
src/java/org/apache/cassandra/db/Slices.java | 5 +-
.../cql3/statements/SelectStatementTest.java | 63 ++++++++++++++++++++
.../apache/cassandra/db/filter/SliceTest.java | 33 ++++++++--
6 files changed, 108 insertions(+), 12 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cassandra/blob/a41b861f/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 9b4ab59..bde5b52 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
4.0
+ * Catch empty/invalid bounds in SelectStatement (CASSANDRA-14849)
* Auto-expand replication_factor for NetworkTopologyStrategy (CASSANDRA-14303)
* Transient Replication: support EACH_QUORUM (CASSANDRA-14727)
* BufferPool: allocating thread for new chunks should acquire directly (CASSANDRA-14832)
http://git-wip-us.apache.org/repos/asf/cassandra/blob/a41b861f/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java
index f847a6e..6e52ab1 100644
--- a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java
+++ b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java
@@ -20,6 +20,7 @@ package org.apache.cassandra.cql3.statements;
import java.nio.ByteBuffer;
import java.util.*;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.MoreObjects;
import org.slf4j.Logger;
@@ -625,7 +626,8 @@ public class SelectStatement implements CQLStatement
return new ClusteringIndexNamesFilter(clusterings, isReversed);
}
- private Slices makeSlices(QueryOptions options)
+ @VisibleForTesting
+ public Slices makeSlices(QueryOptions options)
throws InvalidRequestException
{
SortedSet<ClusteringBound> startBounds = restrictions.getClusteringColumnsBounds(Bound.START, options);
@@ -637,7 +639,7 @@ public class SelectStatement implements CQLStatement
{
ClusteringBound start = startBounds.first();
ClusteringBound end = endBounds.first();
- return table.comparator.compare(start, end) > 0
+ return Slice.isEmpty(table.comparator, start, end)
? Slices.NONE
: Slices.with(table.comparator, Slice.make(start, end));
}
@@ -651,7 +653,7 @@ public class SelectStatement implements CQLStatement
ClusteringBound end = endIter.next();
// Ignore slices that are nonsensical
- if (table.comparator.compare(start, end) > 0)
+ if (Slice.isEmpty(table.comparator, start, end))
continue;
builder.add(start, end);
http://git-wip-us.apache.org/repos/asf/cassandra/blob/a41b861f/src/java/org/apache/cassandra/db/Slice.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/Slice.java b/src/java/org/apache/cassandra/db/Slice.java
index 2517186..44f7ac3 100644
--- a/src/java/org/apache/cassandra/db/Slice.java
+++ b/src/java/org/apache/cassandra/db/Slice.java
@@ -159,7 +159,15 @@ public class Slice
public static boolean isEmpty(ClusteringComparator comparator, ClusteringBound start, ClusteringBound end)
{
assert start.isStart() && end.isEnd();
- return comparator.compare(end, start) <= 0;
+
+ int cmp = comparator.compare(start, end);
+
+ if (cmp < 0)
+ return false;
+ else if (cmp > 0)
+ return true;
+ else
+ return start.isExclusive() || end.isExclusive();
}
/**
http://git-wip-us.apache.org/repos/asf/cassandra/blob/a41b861f/src/java/org/apache/cassandra/db/Slices.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/Slices.java b/src/java/org/apache/cassandra/db/Slices.java
index 9900112..9c7da79 100644
--- a/src/java/org/apache/cassandra/db/Slices.java
+++ b/src/java/org/apache/cassandra/db/Slices.java
@@ -21,6 +21,7 @@ import java.io.IOException;
import java.nio.ByteBuffer;
import java.util.*;
+import com.google.common.base.Preconditions;
import com.google.common.collect.Iterators;
import org.apache.cassandra.schema.ColumnMetadata;
@@ -61,7 +62,7 @@ public abstract class Slices implements Iterable<Slice>
if (slice.start() == ClusteringBound.BOTTOM && slice.end() == ClusteringBound.TOP)
return Slices.ALL;
- assert comparator.compare(slice.start(), slice.end()) <= 0;
+ Preconditions.checkArgument(!slice.isEmpty(comparator));
return new ArrayBackedSlices(comparator, new Slice[]{ slice });
}
@@ -192,7 +193,7 @@ public abstract class Slices implements Iterable<Slice>
public Builder add(Slice slice)
{
- assert comparator.compare(slice.start(), slice.end()) <= 0;
+ Preconditions.checkArgument(!slice.isEmpty(comparator));
if (slices.size() > 0 && comparator.compare(slices.get(slices.size()-1).end(), slice.start()) > 0)
needsNormalizing = true;
slices.add(slice);
http://git-wip-us.apache.org/repos/asf/cassandra/blob/a41b861f/test/unit/org/apache/cassandra/cql3/statements/SelectStatementTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/cql3/statements/SelectStatementTest.java b/test/unit/org/apache/cassandra/cql3/statements/SelectStatementTest.java
new file mode 100644
index 0000000..5856bce
--- /dev/null
+++ b/test/unit/org/apache/cassandra/cql3/statements/SelectStatementTest.java
@@ -0,0 +1,63 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.cql3.statements;
+
+import org.junit.Assert;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import org.apache.cassandra.SchemaLoader;
+import org.apache.cassandra.config.DatabaseDescriptor;
+import org.apache.cassandra.cql3.CQLStatement;
+import org.apache.cassandra.cql3.QueryOptions;
+import org.apache.cassandra.cql3.QueryProcessor;
+import org.apache.cassandra.db.Slices;
+import org.apache.cassandra.schema.KeyspaceParams;
+import org.apache.cassandra.service.ClientState;
+
+public class SelectStatementTest
+{
+
+ private static final String KEYSPACE = "ks";
+
+ @BeforeClass
+ public static void setupClass()
+ {
+ DatabaseDescriptor.daemonInitialization();
+ SchemaLoader.prepareServer();
+ SchemaLoader.createKeyspace(KEYSPACE, KeyspaceParams.simple(1));
+ }
+
+ private static SelectStatement parseSelect(String query)
+ {
+ CQLStatement stmt = QueryProcessor.parseStatement(query).prepare(ClientState.forInternalCalls());
+ assert stmt instanceof SelectStatement;
+ return (SelectStatement) stmt;
+ }
+
+ @Test
+ public void testNonsensicalBounds()
+ {
+ QueryProcessor.executeOnceInternal("CREATE TABLE ks.tbl (k int, c int, v int, primary key (k, c))");
+ QueryProcessor.executeOnceInternal("INSERT INTO ks.tbl (k, c, v) VALUES (100, 10, 0)");
+ Assert.assertEquals(Slices.NONE, parseSelect("SELECT * FROM ks.tbl WHERE k=100 AND c > 10 AND c <= 10").makeSlices(QueryOptions.DEFAULT));
+ Assert.assertEquals(Slices.NONE, parseSelect("SELECT * FROM ks.tbl WHERE k=100 AND c < 10 AND c >= 10").makeSlices(QueryOptions.DEFAULT));
+ Assert.assertEquals(Slices.NONE, parseSelect("SELECT * FROM ks.tbl WHERE k=100 AND c < 10 AND c > 10").makeSlices(QueryOptions.DEFAULT));
+ }
+}
http://git-wip-us.apache.org/repos/asf/cassandra/blob/a41b861f/test/unit/org/apache/cassandra/db/filter/SliceTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/db/filter/SliceTest.java b/test/unit/org/apache/cassandra/db/filter/SliceTest.java
index 9188c94..6c04500 100644
--- a/test/unit/org/apache/cassandra/db/filter/SliceTest.java
+++ b/test/unit/org/apache/cassandra/db/filter/SliceTest.java
@@ -30,6 +30,7 @@ import org.apache.cassandra.db.marshal.AbstractType;
import org.apache.cassandra.db.marshal.Int32Type;
import org.apache.cassandra.utils.ByteBufferUtil;
+import static org.apache.cassandra.db.ClusteringPrefix.Kind.*;
import static org.junit.Assert.*;
public class SliceTest
@@ -43,8 +44,8 @@ public class SliceTest
types.add(Int32Type.instance);
ClusteringComparator cc = new ClusteringComparator(types);
- ClusteringPrefix.Kind sk = ClusteringPrefix.Kind.INCL_START_BOUND;
- ClusteringPrefix.Kind ek = ClusteringPrefix.Kind.INCL_END_BOUND;
+ ClusteringPrefix.Kind sk = INCL_START_BOUND;
+ ClusteringPrefix.Kind ek = INCL_END_BOUND;
// filter falls entirely before sstable
Slice slice = Slice.make(makeBound(sk, 0, 0, 0), makeBound(ek, 1, 0, 0));
@@ -274,8 +275,8 @@ public class SliceTest
types.add(Int32Type.instance);
ClusteringComparator cc = new ClusteringComparator(types);
- ClusteringPrefix.Kind sk = ClusteringPrefix.Kind.INCL_START_BOUND;
- ClusteringPrefix.Kind ek = ClusteringPrefix.Kind.INCL_END_BOUND;
+ ClusteringPrefix.Kind sk = INCL_START_BOUND;
+ ClusteringPrefix.Kind ek = INCL_END_BOUND;
// slice does intersect
Slice slice = Slice.make(makeBound(sk), makeBound(ek));
@@ -323,6 +324,26 @@ public class SliceTest
assertSlicesNormalization(cc, slices(s(-1, 2), s(-1, 3), s(5, 9)), slices(s(-1, 3), s(5, 9)));
}
+ @Test
+ public void testIsEmpty()
+ {
+ List<AbstractType<?>> types = new ArrayList<>();
+ types.add(Int32Type.instance);
+ types.add(Int32Type.instance);
+ ClusteringComparator cc = new ClusteringComparator(types);
+
+ assertFalse(Slice.isEmpty(cc, makeBound(INCL_START_BOUND, 5, 0), makeBound(INCL_END_BOUND, 5, 0)));
+ assertFalse(Slice.isEmpty(cc, makeBound(INCL_START_BOUND, 5, 0), makeBound(EXCL_END_BOUND, 5, 1)));
+ assertFalse(Slice.isEmpty(cc, makeBound(INCL_START_BOUND, 5), makeBound(EXCL_END_BOUND, 5, 1)));
+
+ assertTrue(Slice.isEmpty(cc, makeBound(EXCL_START_BOUND, 5), makeBound(EXCL_END_BOUND, 5)));
+ assertTrue(Slice.isEmpty(cc, makeBound(EXCL_START_BOUND, 5), makeBound(EXCL_END_BOUND, 5, 1)));
+ assertTrue(Slice.isEmpty(cc, makeBound(EXCL_START_BOUND, 5, 1), makeBound(EXCL_END_BOUND, 5, 1)));
+ assertTrue(Slice.isEmpty(cc, makeBound(INCL_START_BOUND, 5, 0), makeBound(INCL_END_BOUND, 4, 0)));
+ assertTrue(Slice.isEmpty(cc, makeBound(INCL_START_BOUND, 5, 0), makeBound(EXCL_END_BOUND, 5)));
+ assertTrue(Slice.isEmpty(cc, makeBound(INCL_START_BOUND, 5, 0), makeBound(EXCL_END_BOUND, 3, 0)));
+ }
+
private static ClusteringBound makeBound(ClusteringPrefix.Kind kind, Integer... components)
{
ByteBuffer[] values = new ByteBuffer[components.length];
@@ -343,8 +364,8 @@ public class SliceTest
private static Slice s(int start, int finish)
{
- return Slice.make(makeBound(ClusteringPrefix.Kind.INCL_START_BOUND, start),
- makeBound(ClusteringPrefix.Kind.INCL_END_BOUND, finish));
+ return Slice.make(makeBound(INCL_START_BOUND, start),
+ makeBound(INCL_END_BOUND, finish));
}
private Slice[] slices(Slice... slices)
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org