diff --git a/CHANGES.txt b/CHANGES.txt index d2fc1ba15f..3adaa9be91 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 5.1 + * BETWEEN where token(Y) > token(Z) returns wrong answer (CASSANDRA-20154) * Optimize memtable flush logic (CASSANDRA-21083) * No need to evict already prepared statements, as it creates a race condition between multiple threads (CASSANDRA-17401) * Include Level information for UnifiedCompactionStrategy in nodetool tablestats output (CASSANDRA-20820) diff --git a/src/java/org/apache/cassandra/cql3/Operator.java b/src/java/org/apache/cassandra/cql3/Operator.java index 7661bb7390..b9393054fc 100644 --- a/src/java/org/apache/cassandra/cql3/Operator.java +++ b/src/java/org/apache/cassandra/cql3/Operator.java @@ -788,8 +788,8 @@ public enum Operator public boolean isSatisfiedBy(AbstractType type, ByteBuffer leftOperand, ByteBuffer rightOperand) { List buffers = ListType.getInstance(type, false).unpack(rightOperand); - // We use compare instead of compareForCQL to deal properly with reversed clustering columns - return type.compare(leftOperand, buffers.get(0)) >= 0 && type.compare(leftOperand, buffers.get(1)) <= 0; + AbstractType unwrapped = type.unwrap(); + return unwrapped.compare(leftOperand, buffers.get(0)) >= 0 && unwrapped.compare(leftOperand, buffers.get(1)) <= 0; } @Override @@ -802,11 +802,6 @@ public enum Operator public void restrict(RangeSet rangeSet, List args, IPartitioner partitioner) { assert args.size() == 2 : this + " accepts exactly two values"; - // avoid sorting when working with token restrictions, otherwise we can't know the difference between these queries: - // select * from x.y where token(id) between 0 and MIN_TOKEN - // select * from x.y where token(id) between MIN_TOKEN and 0 - if (!args.get(0).token) - args.sort(ClusteringElements.CQL_COMPARATOR); rangeSet.removeAll(ClusteringElements.lessThan(args.get(0), partitioner)); rangeSet.removeAll(ClusteringElements.greaterThan(args.get(1), partitioner)); } diff --git a/src/java/org/apache/cassandra/cql3/restrictions/SimpleRestriction.java b/src/java/org/apache/cassandra/cql3/restrictions/SimpleRestriction.java index 310a82849c..16141b8b8c 100644 --- a/src/java/org/apache/cassandra/cql3/restrictions/SimpleRestriction.java +++ b/src/java/org/apache/cassandra/cql3/restrictions/SimpleRestriction.java @@ -343,8 +343,7 @@ public final class SimpleRestriction implements SingleRestriction List buffers = bindAndGet(options); if (operator.kind() != Operator.Kind.BINARY) { - // For BETWEEN we support like in SQL reversed bounds - if (operator.kind() == Operator.Kind.TERNARY) + if (operator == Operator.IN && !column.type.isCounter()) buffers.sort(column.type); filter.add(column, operator, multiInputOperatorValues(column, buffers)); } diff --git a/src/java/org/apache/cassandra/index/sai/plan/Expression.java b/src/java/org/apache/cassandra/index/sai/plan/Expression.java index a5cf07e35d..27b791ab87 100644 --- a/src/java/org/apache/cassandra/index/sai/plan/Expression.java +++ b/src/java/org/apache/cassandra/index/sai/plan/Expression.java @@ -246,8 +246,7 @@ public abstract class Expression Value first = new Value(buffers.get(0), indexTermType); Value second = new Value(buffers.get(1), indexTermType); - // SimpleRestriction#addToRowFilter() ensures correct bounds ordering, but SAI enforces a non-arbitrary - // ordering between IPv4 and IPv6 addresses, so correction may still be necessary. + // SAI enforces a non-arbitrary ordering between IPv4 and IPv6 addresses, so correction may still be necessary. boolean outOfOrder = indexTermType.compare(first.encoded, second.encoded) > 0; lower = new Bound(outOfOrder ? second : first, true); upper = new Bound(outOfOrder ? first : second, true); diff --git a/test/distributed/org/apache/cassandra/distributed/test/BetweenInversionTest.java b/test/distributed/org/apache/cassandra/distributed/test/BetweenInversionTest.java new file mode 100644 index 0000000000..fd89bea1b6 --- /dev/null +++ b/test/distributed/org/apache/cassandra/distributed/test/BetweenInversionTest.java @@ -0,0 +1,54 @@ +/* + * 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.distributed.test; + +import org.junit.Test; + +import org.apache.cassandra.distributed.Cluster; +import org.apache.cassandra.distributed.api.ConsistencyLevel; + +import static org.junit.Assert.assertEquals; + +/** + * Tests that the BETWEEN operator respects SQL semantics: + * - Normal BETWEEN (low <= high) returns rows. + * - Inverted BETWEEN (low > high) returns no rows (empty result). + */ +public class BetweenInversionTest extends TestBaseImpl +{ + @Test + public void testBetweenInversion() throws Throwable + { + try (Cluster cluster = init(Cluster.build(1).start())) + { + cluster.schemaChange("CREATE KEYSPACE ks WITH replication = {'class':'SimpleStrategy','replication_factor':1}"); + cluster.schemaChange("CREATE TABLE ks.t1 (pk int PRIMARY KEY, val text)"); + + cluster.coordinator(1).execute("INSERT INTO ks.t1 (pk,val) VALUES (1,'a')", ConsistencyLevel.ALL); + cluster.coordinator(1).execute("INSERT INTO ks.t1 (pk,val) VALUES (2,'b')", ConsistencyLevel.ALL); + + Object[][] rows = cluster.coordinator(1) + .execute("SELECT * FROM ks.t1 WHERE pk BETWEEN 1 AND 2 ALLOW FILTERING", ConsistencyLevel.ALL); + assertEquals(2, rows.length); + + Object[][] inverted = cluster.coordinator(1) + .execute("SELECT * FROM ks.t1 WHERE pk BETWEEN 2 AND 1 ALLOW FILTERING", ConsistencyLevel.ALL); + assertEquals(0, inverted.length); + } + } +} \ No newline at end of file diff --git a/test/distributed/org/apache/cassandra/distributed/test/cql3/SingleNodeTableWalkTest.java b/test/distributed/org/apache/cassandra/distributed/test/cql3/SingleNodeTableWalkTest.java index 938065467e..6c7c58936c 100644 --- a/test/distributed/org/apache/cassandra/distributed/test/cql3/SingleNodeTableWalkTest.java +++ b/test/distributed/org/apache/cassandra/distributed/test/cql3/SingleNodeTableWalkTest.java @@ -349,6 +349,86 @@ public class SingleNodeTableWalkTest extends StatefulASTBase return state.command(rs, select, symbol.detailedName() + (indexed == null ? "" : ", indexed with " + indexed.indexDDL.indexer.name())); } + public Property.Command clusteringBetweenQuery(RandomSource rs, State state) + { + ImmutableUniqueList cks = state.model.factory.clusteringColumns; + if (cks.isEmpty()) + return Property.ignoreCommand(); + + // Select a single partition + BytesPartitionState.Ref ref = rs.pickOrderedSet(state.model.partitionKeys()); + BytesPartitionState partition = state.model.get(ref); + + NavigableSet> clusteringKeys = partition.clusteringKeys(); + + // Use first clustering column for BETWEEN + Symbol ckSymbol = cks.get(0); + + ByteBuffer low, high; + boolean useRealValues = clusteringKeys.size() >= 2 && rs.nextBoolean(); + if (useRealValues) + { + // low and high can match, have high < low, and low < high... this is all expected + // between where low > high should return nothing + // between where low == high should be the same as a ck=? query + low = rs.pickOrderedSet(clusteringKeys).bufferAt(0); + high = rs.pickOrderedSet(clusteringKeys).bufferAt(0); + } + else + { + // Generate random values + Gen bytesGen = toGen(getTypeSupport(ckSymbol.type()).bytesGen()); + low = bytesGen.next(rs); + high = bytesGen.next(rs); + } + + Select.Builder builder = Select.builder().table(state.metadata); + + // Add partition key restrictions + ImmutableUniqueList pks = state.model.factory.partitionColumns; + Clustering pkKey = ref.key; + for (Symbol pk : pks) + builder.value(pk, pkKey.bufferAt(pks.indexOf(pk))); + + + String annotation; + if (rs.nextBoolean()) + { + builder.between(ckSymbol, state.value(rs, low, ckSymbol.type()), state.value(rs, high, ckSymbol.type())); + annotation = "clustering BETWEEN"; + } + else if (rs.nextBoolean()) + { + builder.where(ckSymbol, Inequality.GREATER_THAN_EQ, low); + builder.where(ckSymbol, Inequality.LESS_THAN_EQ, high); + annotation = "clustering >= AND <="; + } + else + { + builder.where(ckSymbol, Inequality.GREATER_THAN, low); + builder.where(ckSymbol, Inequality.LESS_THAN, high); + annotation = "clustering > AND <"; + } + + if (rs.nextBoolean()) + builder.orderByColumn(ckSymbol, rs.pick(Select.OrderBy.Ordering.values())); + + // Check if clustering column is indexed + var indexed = state.indexes.get(ckSymbol); + if (indexed != null) + annotation += " (indexed with " + indexed.indexDDL.indexer.name() + ')'; + + // Check if column is reversed (DESC) + if (ckSymbol.reversed) + annotation += " [DESC]"; + + if (!useRealValues) + annotation += " [random values]"; + + Select select = builder.build(); + return state.command(rs, select, annotation); + } + protected State createState(RandomSource rs, Cluster cluster) { return new State(rs, cluster); @@ -383,6 +463,7 @@ public class SingleNodeTableWalkTest extends StatefulASTBase .addIf(State::allowNonPartitionQuery, this::nonPartitionQuery) .addIf(State::allowNonPartitionMultiColumnQuery, this::multiColumnQuery) .addIf(State::allowPartitionQuery, this::partitionRestrictedQuery) + .addIf(State::allowClusteringBetweenQuery, this::clusteringBetweenQuery) .destroyState(State::close) .commandsTransformer(LoggingCommand.factory()) .onSuccess(onSuccess(logger)) @@ -608,6 +689,11 @@ public class SingleNodeTableWalkTest extends StatefulASTBase return !(model.isEmpty() || searchableNonPartitionColumns.isEmpty()); } + public boolean allowClusteringBetweenQuery() + { + return hasPartitions() && !model.factory.clusteringColumns.isEmpty(); + } + @Override public String toString() { diff --git a/test/distributed/org/apache/cassandra/distributed/test/cql3/SingleNodeTokenConflictTest.java b/test/distributed/org/apache/cassandra/distributed/test/cql3/SingleNodeTokenConflictTest.java index 7c8665f216..e866ed9a34 100644 --- a/test/distributed/org/apache/cassandra/distributed/test/cql3/SingleNodeTokenConflictTest.java +++ b/test/distributed/org/apache/cassandra/distributed/test/cql3/SingleNodeTokenConflictTest.java @@ -46,7 +46,6 @@ import accord.utils.Property; import accord.utils.RandomSource; import org.apache.cassandra.cql3.ColumnIdentifier; -import org.apache.cassandra.cql3.KnownIssue; import org.apache.cassandra.cql3.ast.Conditional.Where.Inequality; import org.apache.cassandra.cql3.ast.FunctionCall; import org.apache.cassandra.cql3.ast.Mutation; @@ -134,13 +133,6 @@ public class SingleNodeTokenConflictTest extends StatefulASTBase ByteBuffer left = state.pkGen.next(rs); ByteBuffer right = state.betweenEqGen.next(rs) ? left : state.pkGen.next(rs); int rc = PK_TYPE.compare(left, right); - if (rc > 0 && IGNORED_ISSUES.contains(KnownIssue.BETWEEN_START_LARGER_THAN_END)) - { - ByteBuffer tmp = left; - left = right; - right = tmp; - rc = PK_TYPE.compare(left, right); - } Select select = Select.builder() .table(state.tableRef) .between(PK, state.pkValue(rs, left), state.pkValue(rs, right)) @@ -205,16 +197,6 @@ public class SingleNodeTokenConflictTest extends StatefulASTBase LongToken start = Murmur3Partitioner.instance.getToken(left); LongToken end = Murmur3Partitioner.instance.getToken(right); int rc = start.compareTo(end); - if (rc > 0 && IGNORED_ISSUES.contains(KnownIssue.BETWEEN_START_LARGER_THAN_END)) - { - ByteBuffer tmp = left; - left = right; - right = tmp; - LongToken tmp2 = start; - start = end; - end = tmp2; - rc = start.compareTo(end); - } Select select = Select.builder() .table(state.tableRef) .between(FunctionCall.tokenByColumns(PK), diff --git a/test/distributed/org/apache/cassandra/distributed/test/cql3/StatefulASTBase.java b/test/distributed/org/apache/cassandra/distributed/test/cql3/StatefulASTBase.java index 0771b8f847..cec98dcbea 100644 --- a/test/distributed/org/apache/cassandra/distributed/test/cql3/StatefulASTBase.java +++ b/test/distributed/org/apache/cassandra/distributed/test/cql3/StatefulASTBase.java @@ -371,13 +371,6 @@ public class StatefulASTBase extends TestBaseImpl } else { - // it's possible that the range was flipped, which is known bug with BETWEEN, so - // make sure the range is not flipped until that bug is fixed - if (IGNORED_ISSUES.contains(KnownIssue.BETWEEN_START_LARGER_THAN_END)) - { - min = Literal.of(key.token.getLongValue()); - max = Literal.of(Long.MIN_VALUE); - } select = Select.builder(state.metadata) .between(tokenCall, min, max) .build(); diff --git a/test/harry/main/org/apache/cassandra/harry/model/ASTSingleTableModel.java b/test/harry/main/org/apache/cassandra/harry/model/ASTSingleTableModel.java index 52c30be260..eb130e6c77 100644 --- a/test/harry/main/org/apache/cassandra/harry/model/ASTSingleTableModel.java +++ b/test/harry/main/org/apache/cassandra/harry/model/ASTSingleTableModel.java @@ -44,6 +44,7 @@ import java.util.stream.Stream; import javax.annotation.Nullable; import com.google.common.base.Preconditions; +import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.common.collect.Sets; @@ -1580,7 +1581,7 @@ public class ASTSingleTableModel ImmutableUniqueList selectOrder = factory.selectionOrder; ImmutableUniqueList targetOrder = columns(select); if (select.where.isEmpty()) - return SelectResult.ordered(targetOrder, filter(getRowsAsByteBuffer(applyLimits(all(), select.perPartitionLimit, select.limit)), selectOrder, targetOrder)); + return SelectResult.ordered(targetOrder, filter(getRowsAsByteBuffer(applyLimits(all(), select.perPartitionLimit, select.limit, select.orderBy)), selectOrder, targetOrder)); LookupContext ctx = context(select); List primaryKeys; if (ctx.unmatchable) @@ -1606,13 +1607,17 @@ public class ASTSingleTableModel // partial tested (handles many columns, tests are single column) primaryKeys = search(ctx); } - primaryKeys = applyLimits(primaryKeys, select.perPartitionLimit, select.limit); + primaryKeys = applyLimits(primaryKeys, select.perPartitionLimit, select.limit, select.orderBy); //TODO (correctness): now that we have the rows we need to handle the selections/aggregation/limit/group-by/etc. return new SelectResult(targetOrder, filter(getRowsAsByteBuffer(primaryKeys), selectOrder, targetOrder), ctx.unordered); } - private List applyLimits(List primaryKeys, Optional perPartitionLimitOpt, Optional limitOpt) + private List applyLimits(List primaryKeys, + Optional perPartitionLimitOpt, Optional limitOpt, + Optional orderBy) { + if (orderBy.isPresent() && shouldReverse(orderBy.get())) + primaryKeys = Lists.reverse(primaryKeys); if (perPartitionLimitOpt.isPresent()) { int limit = Int32Type.instance.compose(eval(perPartitionLimitOpt.get())); @@ -1640,6 +1645,18 @@ public class ASTSingleTableModel return primaryKeys; } + private boolean shouldReverse(Select.OrderBy orderBy) + { + for (var block : orderBy.ordered) + { + Symbol col = (Symbol) block.expression; //TODO (coverage): do we support anything other than symbol? + col = factory.clusteringColumns.get(col); // switch to table symbol so we know if its reversed or not + if (col.reversed != (block.ordering == Select.OrderBy.Ordering.DESC)) + return true; + } + return false; + } + private List all() { List primaryKeys = new ArrayList<>(); diff --git a/test/unit/org/apache/cassandra/cql3/CQLTester.java b/test/unit/org/apache/cassandra/cql3/CQLTester.java index 9f0369b79c..1dad1a5ae7 100644 --- a/test/unit/org/apache/cassandra/cql3/CQLTester.java +++ b/test/unit/org/apache/cassandra/cql3/CQLTester.java @@ -2698,13 +2698,13 @@ public abstract class CQLTester return rows; } - protected void assertEmpty(UntypedResultSet result) throws Throwable + protected void assertEmpty(UntypedResultSet result) { if (result != null && !result.isEmpty()) throw new AssertionError(String.format("Expected empty result but got %d rows: %s \n", result.size(), makeRowStrings(result))); } - protected void assertInvalid(String query, Object... values) throws Throwable + protected void assertInvalid(String query, Object... values) { assertInvalidMessage(null, query, values); } @@ -2778,7 +2778,7 @@ public abstract class CQLTester : replaceValues(query, values); } - protected void assertValidSyntax(String query) throws Throwable + protected void assertValidSyntax(String query) { try { @@ -2791,12 +2791,12 @@ public abstract class CQLTester } } - protected void assertInvalidSyntax(String query, Object... values) throws Throwable + protected void assertInvalidSyntax(String query, Object... values) { assertInvalidSyntaxMessage(null, query, values); } - protected void assertInvalidSyntaxMessage(String errorMessage, String query, Object... values) throws Throwable + protected void assertInvalidSyntaxMessage(String errorMessage, String query, Object... values) { try { diff --git a/test/unit/org/apache/cassandra/cql3/KnownIssue.java b/test/unit/org/apache/cassandra/cql3/KnownIssue.java index 19378ce496..35caf0f01b 100644 --- a/test/unit/org/apache/cassandra/cql3/KnownIssue.java +++ b/test/unit/org/apache/cassandra/cql3/KnownIssue.java @@ -27,8 +27,6 @@ import java.util.EnumSet; */ public enum KnownIssue { - BETWEEN_START_LARGER_THAN_END("https://issues.apache.org/jira/browse/CASSANDRA-20154", - "BETWEEN is matching values when start > end, which should never return anything"), SAI_INET_MIXED("https://issues.apache.org/jira/browse/CASSANDRA-19492", "SAI converts ipv4 to ipv6 to simplify the index, this causes issues with range search as it starts to mix the values, which isn't always desirable or intuative"), CUSTOM_INDEX_MAX_COLUMN_48("https://issues.apache.org/jira/browse/CASSANDRA-19897", diff --git a/test/unit/org/apache/cassandra/cql3/ast/Select.java b/test/unit/org/apache/cassandra/cql3/ast/Select.java index 8a451c849a..7e8eccf06c 100644 --- a/test/unit/org/apache/cassandra/cql3/ast/Select.java +++ b/test/unit/org/apache/cassandra/cql3/ast/Select.java @@ -417,7 +417,12 @@ FROM [keyspace_name.] table_name public T orderByColumn(String name, AbstractType type, OrderBy.Ordering ordering) { - orderBy.add(new Symbol(name, type), ordering); + return orderByColumn(new Symbol(name, type), ordering); + } + + public T orderByColumn(Symbol column, OrderBy.Ordering ordering) + { + orderBy.add(column, ordering); return (T) this; } diff --git a/test/unit/org/apache/cassandra/cql3/validation/operations/SelectMultiColumnRelationTest.java b/test/unit/org/apache/cassandra/cql3/validation/operations/SelectMultiColumnRelationTest.java index 783f4314f6..f6b2a693ce 100644 --- a/test/unit/org/apache/cassandra/cql3/validation/operations/SelectMultiColumnRelationTest.java +++ b/test/unit/org/apache/cassandra/cql3/validation/operations/SelectMultiColumnRelationTest.java @@ -31,7 +31,7 @@ public class SelectMultiColumnRelationTest extends CQLTester private static final ByteBuffer TOO_BIG = ByteBuffer.allocate(1024 * 65); @Test - public void testSingleClusteringInvalidQueries() throws Throwable + public void testSingleClusteringInvalidQueries() { createTable("CREATE TABLE %s (a int, b int, c int, PRIMARY KEY (a, b))"); @@ -53,7 +53,7 @@ public class SelectMultiColumnRelationTest extends CQLTester } @Test - public void testMultiClusteringInvalidQueries() throws Throwable + public void testMultiClusteringInvalidQueries() { createTable("CREATE TABLE %s (a int, b int, c int, d int, PRIMARY KEY (a, b, c, d))"); @@ -229,9 +229,8 @@ public class SelectMultiColumnRelationTest extends CQLTester row(0, 0, 0, 0), row(0, 0, 1, 0)); - assertRows(execute("SELECT * FROM %s WHERE a = ? AND (b, c, d) BETWEEN (?, ?, ?) AND (?, ?, ?)", 0, 0, 1, 0, 0, 0, 0), - row(0, 0, 0, 0), - row(0, 0, 1, 0)); + // BETWEEN with inverted bounds returns empty result (first bound > second bound) + assertEmpty(execute("SELECT * FROM %s WHERE a = ? AND (b, c, d) BETWEEN (?, ?, ?) AND (?, ?, ?)", 0, 0, 1, 0, 0, 0, 0)); } @Test @@ -317,7 +316,7 @@ public class SelectMultiColumnRelationTest extends CQLTester } @Test - public void testSinglePartitionInvalidQueries() throws Throwable + public void testSinglePartitionInvalidQueries() { createTable("CREATE TABLE %s (a int PRIMARY KEY, b int)"); assertInvalidMessage("Multi-column relations can only be applied to clustering columns but was applied to: a", @@ -329,7 +328,7 @@ public class SelectMultiColumnRelationTest extends CQLTester } @Test - public void testSingleClustering() throws Throwable + public void testSingleClustering() { createTable("CREATE TABLE %s (a int, b int, c int, PRIMARY KEY (a, b))"); @@ -481,7 +480,7 @@ public class SelectMultiColumnRelationTest extends CQLTester } @Test - public void testMultipleClustering() throws Throwable + public void testMultipleClustering() { createTable("CREATE TABLE %s (a int, b int, c int, d int, PRIMARY KEY (a, b, c, d))"); @@ -726,11 +725,8 @@ public class SelectMultiColumnRelationTest extends CQLTester row(0, 0, 0, 0) ); - assertRows(execute("SELECT * FROM %s WHERE a = ? AND (b, c) BETWEEN (?, ?) AND (?, ?) ORDER BY b DESC, c DESC, d DESC", 0, 0, 1, 0, 0), - row(0, 0, 1, 1), - row(0, 0, 1, 0), - row(0, 0, 0, 0) - ); + // BETWEEN with inverted bounds returns empty result (first bound > second bound) + assertEmpty(execute("SELECT * FROM %s WHERE a = ? AND (b, c) BETWEEN (?, ?) AND (?, ?) ORDER BY b DESC, c DESC, d DESC", 0, 0, 1, 0, 0)); assertRows(execute("SELECT * FROM %s WHERE a = ? AND (b, c, d) < (?, ?, ?) ORDER BY b DESC, c DESC, d DESC", 0, 0, 1, 1), row(0, 0, 1, 0), @@ -743,11 +739,8 @@ public class SelectMultiColumnRelationTest extends CQLTester row(0, 0, 0, 0) ); - assertRows(execute("SELECT * FROM %s WHERE a = ? AND (b, c, d) BETWEEN (?, ?, ?) AND (?, ?, ?) ORDER BY b DESC, c DESC, d DESC", 0, 0, 1, 1, 0, 0, 0), - row(0, 0, 1, 1), - row(0, 0, 1, 0), - row(0, 0, 0, 0) - ); + // BETWEEN with inverted bounds returns empty result (first bound > second bound) + assertEmpty(execute("SELECT * FROM %s WHERE a = ? AND (b, c, d) BETWEEN (?, ?, ?) AND (?, ?, ?) ORDER BY b DESC, c DESC, d DESC", 0, 0, 1, 1, 0, 0, 0)); assertRows(execute("SELECT * FROM %s WHERE a = ? AND (b, c, d) > (?, ?, ?) AND (b) < (?) ORDER BY b DESC, c DESC, d DESC", 0, 0, 1, 0, 1), row(0, 0, 1, 1) @@ -952,7 +945,7 @@ public class SelectMultiColumnRelationTest extends CQLTester } @Test - public void testMultipleClusteringWithIndex() throws Throwable + public void testMultipleClusteringWithIndex() { Util.assumeLegacySecondaryIndex(); createTable("CREATE TABLE %s (a int, b int, c int, d int, e int, PRIMARY KEY (a, b, c, d))"); @@ -1042,7 +1035,7 @@ public class SelectMultiColumnRelationTest extends CQLTester } @Test - public void testMultipleClusteringWithIndexAndValueOver64K() throws Throwable + public void testMultipleClusteringWithIndexAndValueOver64K() { Util.assumeLegacySecondaryIndex(); createTable("CREATE TABLE %s (a int, b blob, c int, d int, PRIMARY KEY (a, b, c))"); @@ -1056,7 +1049,7 @@ public class SelectMultiColumnRelationTest extends CQLTester } @Test - public void testMultiColumnRestrictionsWithIndex() throws Throwable + public void testMultiColumnRestrictionsWithIndex() { createTable("CREATE TABLE %s (a int, b int, c int, d int, e int, v int, PRIMARY KEY (a, b, c, d, e))"); createIndex("CREATE INDEX ON %s (v)"); @@ -1085,7 +1078,7 @@ public class SelectMultiColumnRelationTest extends CQLTester } @Test - public void testMultiplePartitionKeyAndMultiClusteringWithIndex() throws Throwable + public void testMultiplePartitionKeyAndMultiClusteringWithIndex() { Util.assumeLegacySecondaryIndex(); createTable("CREATE TABLE %s (a int, b int, c int, d int, e int, f int, PRIMARY KEY ((a, b), c, d, e))"); @@ -1229,7 +1222,7 @@ public class SelectMultiColumnRelationTest extends CQLTester } @Test - public void testWithUnsetValues() throws Throwable + public void testWithUnsetValues() { createTable("CREATE TABLE %s (k int, i int, j int, s text, PRIMARY KEY(k,i,j))"); createIndex("CREATE INDEX s_index ON %s (s)"); @@ -1330,32 +1323,12 @@ public class SelectMultiColumnRelationTest extends CQLTester row(0, -1, 0, -1, 0) ); - assertRows(execute( + // BETWEEN with inverted bounds returns empty result (first bound > second bound in logical order) + assertEmpty(execute( "SELECT * FROM %s" + " WHERE a = ? " + "AND (b,c,d,e) BETWEEN (?,?,?,?) " + - "AND (?,?,?,?)", 0, 2, 0, 1, 1, -1, -1, -1, -1), - - row(0, 2, 0, 1, 1), - row(0, 2, 0, -1, 0), - row(0, 2, 0, -1, 1), - row(0, 1, -1, 1, 0), - row(0, 1, -1, 1, 1), - row(0, 1, -1, 0, 0), - row(0, 1, 0, 1, -1), - row(0, 1, 0, 1, 1), - row(0, 1, 0, 0, -1), - row(0, 1, 0, 0, 0), - row(0, 1, 0, 0, 1), - row(0, 1, 0, -1, -1), - row(0, 1, 1, 0, -1), - row(0, 1, 1, 0, 0), - row(0, 1, 1, 0, 1), - row(0, 1, 1, -1, 0), - row(0, 0, 0, 0, 0), - row(0, -1, 0, 0, 0), - row(0, -1, 0, -1, 0) - ); + "AND (?,?,?,?)", 0, 2, 0, 1, 1, -1, -1, -1, -1)); assertRows(execute( "SELECT * FROM %s" + @@ -1665,17 +1638,8 @@ public class SelectMultiColumnRelationTest extends CQLTester row(0, -1, 0, -1, 0) ); - assertRows(execute("SELECT * FROM %s WHERE a = ? AND (b,c,d,e) BETWEEN (?,?,?,?) AND (?,?,?,?)", 0, 1, 0, 0, 0, -10, -1, -4, -1), - row(0, 1, -1, 1, 0), - row(0, 1, -1, 1, 1), - row(0, 1, -1, 0, 0), - row(0, 1, 0, 0, -1), - row(0, 1, 0, 0, 0), - row(0, 1, 0, -1, -1), - row(0, 0, 0, 0, 0), - row(0, -1, 0, 0, 0), - row(0, -1, 0, -1, 0) - ); + // BETWEEN with inverted bounds returns empty result (first bound > second bound in logical order) + assertEmpty(execute("SELECT * FROM %s WHERE a = ? AND (b,c,d,e) BETWEEN (?,?,?,?) AND (?,?,?,?)", 0, 1, 0, 0, 0, -10, -1, -4, -1)); assertRows(execute("SELECT * FROM %s WHERE a = ? AND (b,c,d,e) > (?,?,?,?)", 0, 1, 0, 0, 0), row(0, 2, 0, 1, 1), @@ -2032,34 +1996,12 @@ public class SelectMultiColumnRelationTest extends CQLTester row(0, 2, -3, 1, 1) ); - assertRows(execute( + // BETWEEN with inverted bounds returns empty result (first bound > second bound in logical order) + assertEmpty(execute( "SELECT * FROM %s" + " WHERE a = ? " + "AND (b,c,d,e) BETWEEN (?,?,?,?) " + - "AND (?,?,?,?)", 0, 2, 0, 1, 1, -1, -10, -10, -10), - - row(0, -1, 0, 0, 0), - row(0, -1, 0, -1, 0), - row(0, 0, 0, 0, 0), - row(0, 1, 1, 0, -1), - row(0, 1, 1, 0, 0), - row(0, 1, 1, 0, 1), - row(0, 1, 1, -1, 0), - row(0, 1, 0, 1, -1), - row(0, 1, 0, 1, 1), - row(0, 1, 0, 0, -1), - row(0, 1, 0, 0, 0), - row(0, 1, 0, 0, 1), - row(0, 1, 0, -1, -1), - row(0, 1, -1, 1, 0), - row(0, 1, -1, 1, 1), - row(0, 1, -1, 0, 0), - row(0, 2, 0, 1, 1), - row(0, 2, 0, -1, 0), - row(0, 2, 0, -1, 1), - row(0, 2, -1, 1, 1), - row(0, 2, -3, 1, 1) - ); + "AND (?,?,?,?)", 0, 2, 0, 1, 1, -1, -10, -10, -10)); assertRows(execute( "SELECT * FROM %s" + @@ -2257,7 +2199,7 @@ public class SelectMultiColumnRelationTest extends CQLTester } @Test - public void testInvalidColumnNames() throws Throwable + public void testInvalidColumnNames() { createTable("CREATE TABLE %s (a int, b int, c int, d int, PRIMARY KEY (a, b, c))"); assertInvalidMessage("Undefined column name e", "SELECT * FROM %s WHERE (b, e) = (0, 0)"); @@ -2269,7 +2211,7 @@ public class SelectMultiColumnRelationTest extends CQLTester } @Test - public void testInRestrictionsWithAllowFiltering() throws Throwable + public void testInRestrictionsWithAllowFiltering() { createTable("CREATE TABLE %s (pk int, c1 text, c2 int, c3 int, v int, primary key(pk, c1, c2, c3))"); execute("INSERT INTO %s (pk, c1, c2, c3, v) values (?, ?, ?, ?, ?)", 1, "0", 0, 1, 3); @@ -2291,7 +2233,7 @@ public class SelectMultiColumnRelationTest extends CQLTester } @Test - public void testInRestrictionsWithIndex() throws Throwable + public void testInRestrictionsWithIndex() { createTable("CREATE TABLE %s (pk int, c1 text, c2 int, c3 int, v int, primary key(pk, c1, c2, c3))"); createIndex("CREATE INDEX ON %s (c3)"); diff --git a/test/unit/org/apache/cassandra/cql3/validation/operations/SelectSingleColumnRelationTest.java b/test/unit/org/apache/cassandra/cql3/validation/operations/SelectSingleColumnRelationTest.java index 5024eeaf41..1ebeec495e 100644 --- a/test/unit/org/apache/cassandra/cql3/validation/operations/SelectSingleColumnRelationTest.java +++ b/test/unit/org/apache/cassandra/cql3/validation/operations/SelectSingleColumnRelationTest.java @@ -586,11 +586,9 @@ public class SelectSingleColumnRelationTest extends CQLTester assertInvalidMessage(StatementRestrictions.REQUIRES_ALLOW_FILTERING_MESSAGE, "SELECT * FROM %s WHERE a = ? AND c = ? AND d BETWEEN ? AND ? AND f = ?", 0, 1, 1, 5, 0); - assertRows(execute("SELECT * FROM %s WHERE a = ? AND b = ? AND c = ? AND d BETWEEN ? AND ? AND f = ?", 0, 0, 1, 1, 0, 5), - row(0, 0, 1, 1, 1, 5)); + assertEmpty(execute("SELECT * FROM %s WHERE a = ? AND b = ? AND c = ? AND d BETWEEN ? AND ? AND f = ?", 0, 0, 1, 1, 0, 5)); - assertRows(execute("SELECT * FROM %s WHERE a = ? AND c = ? AND d BETWEEN ? AND ? AND f = ? ALLOW FILTERING", 0, 1, 1, 0, 5), - row(0, 0, 1, 1, 1, 5)); + assertEmpty(execute("SELECT * FROM %s WHERE a = ? AND c = ? AND d BETWEEN ? AND ? AND f = ? ALLOW FILTERING", 0, 1, 1, 0, 5)); } @Test @@ -1352,9 +1350,7 @@ public class SelectSingleColumnRelationTest extends CQLTester row (0, 2, 2, "MA"), row (0, 3, 3, "MA")); - assertRows(execute("SELECT * FROM %s WHERE c2 BETWEEN 3 AND 2 ALLOW FILTERING"), - row (0, 2, 2, "MA"), - row (0, 3, 3, "MA")); + assertEmpty(execute("SELECT * FROM %s WHERE c2 BETWEEN 3 AND 2 ALLOW FILTERING")); }); createTable("CREATE TABLE %s(p int, c int, c2 int, abbreviation ascii, PRIMARY KEY (p, c, c2)) WITH CLUSTERING ORDER BY (c DESC, c2 DESC)"); @@ -1369,9 +1365,7 @@ public class SelectSingleColumnRelationTest extends CQLTester row(0, 3, 3, "MA"), row(0, 2, 2, "MA")); - assertRows(execute("SELECT * FROM %s WHERE c2 BETWEEN 3 AND 2 ALLOW FILTERING"), - row(0, 3, 3, "MA"), - row(0, 2, 2, "MA")); + assertEmpty(execute("SELECT * FROM %s WHERE c2 BETWEEN 3 AND 2 ALLOW FILTERING")); }); } } diff --git a/test/unit/org/apache/cassandra/utils/ImmutableUniqueList.java b/test/unit/org/apache/cassandra/utils/ImmutableUniqueList.java index 4d156b58be..dfa57dc5d5 100644 --- a/test/unit/org/apache/cassandra/utils/ImmutableUniqueList.java +++ b/test/unit/org/apache/cassandra/utils/ImmutableUniqueList.java @@ -95,6 +95,13 @@ public class ImmutableUniqueList extends AbstractList implements UniqueLis return values[index]; } + public T get(T value) + { + int idx = indexOf(value); + if (idx == -1) throw new IllegalArgumentException("Unable to find value " + value); + return get(idx); + } + @Override public int indexOf(Object o) {