diff --git a/CHANGES.txt b/CHANGES.txt index ff255d8581..380d5583f0 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -41,6 +41,7 @@ * Have paxos reuse the timestamp generation of normal queries (CASSANDRA-7801) * Fix incremental repair not remove parent session on remote (CASSANDRA-8291) Merged from 2.0: + * Fix InvalidRequestException with ORDER BY (CASSANDRA-8286) * Disable SSLv3 for POODLE (CASSANDRA-8265) * Fix millisecond timestamps in Tracing (CASSANDRA-8297) * Include keyspace name in error message when there are insufficient diff --git a/src/java/org/apache/cassandra/cql3/ColumnIdentifier.java b/src/java/org/apache/cassandra/cql3/ColumnIdentifier.java index e1454b1c36..467b214ff6 100644 --- a/src/java/org/apache/cassandra/cql3/ColumnIdentifier.java +++ b/src/java/org/apache/cassandra/cql3/ColumnIdentifier.java @@ -153,6 +153,11 @@ public class ColumnIdentifier extends org.apache.cassandra.cql3.selection.Select return new ColumnIdentifier(comparator.fromString(rawText), text); } + public boolean processesSelection() + { + return false; + } + @Override public final int hashCode() { diff --git a/src/java/org/apache/cassandra/cql3/selection/RawSelector.java b/src/java/org/apache/cassandra/cql3/selection/RawSelector.java index c7e26580f4..7d5543f87b 100644 --- a/src/java/org/apache/cassandra/cql3/selection/RawSelector.java +++ b/src/java/org/apache/cassandra/cql3/selection/RawSelector.java @@ -53,4 +53,9 @@ public class RawSelector } }); } + + public boolean processesSelection() + { + return selectable.processesSelection(); + } } diff --git a/src/java/org/apache/cassandra/cql3/selection/Selectable.java b/src/java/org/apache/cassandra/cql3/selection/Selectable.java index 48ce11a552..c5ef857202 100644 --- a/src/java/org/apache/cassandra/cql3/selection/Selectable.java +++ b/src/java/org/apache/cassandra/cql3/selection/Selectable.java @@ -51,6 +51,11 @@ public abstract class Selectable public static interface Raw { public Selectable prepare(CFMetaData cfm); + + /** + * Returns true if any processing is performed on the selected column. + **/ + public boolean processesSelection(); } public static class WritetimeOrTTL extends Selectable @@ -103,6 +108,11 @@ public abstract class Selectable { return new WritetimeOrTTL(id.prepare(cfm), isWritetime); } + + public boolean processesSelection() + { + return true; + } } } @@ -162,6 +172,11 @@ public abstract class Selectable preparedArgs.add(arg.prepare(cfm)); return new WithFunction(functionName, preparedArgs); } + + public boolean processesSelection() + { + return true; + } } } @@ -221,6 +236,11 @@ public abstract class Selectable { return new WithFieldSelection(selected.prepare(cfm), field.prepare(cfm)); } + + public boolean processesSelection() + { + return true; + } } } } diff --git a/src/java/org/apache/cassandra/cql3/selection/Selection.java b/src/java/org/apache/cassandra/cql3/selection/Selection.java index 7c7dab714a..888d96d506 100644 --- a/src/java/org/apache/cassandra/cql3/selection/Selection.java +++ b/src/java/org/apache/cassandra/cql3/selection/Selection.java @@ -87,11 +87,11 @@ public abstract class Selection return false; } - private static boolean isUsingFunction(List rawSelectors) + private static boolean processesSelection(List rawSelectors) { for (RawSelector rawSelector : rawSelectors) { - if (!(rawSelector.selectable instanceof ColumnIdentifier)) + if (rawSelector.processesSelection()) return true; } return false; @@ -105,8 +105,8 @@ public abstract class Selection SelectorFactories.createFactoriesAndCollectColumnDefinitions(RawSelector.toSelectables(rawSelectors, cfm), cfm, defs); List metadata = collectMetadata(cfm, rawSelectors, factories); - return isUsingFunction(rawSelectors) ? new SelectionWithFunctions(defs, metadata, factories) - : new SimpleSelection(defs, metadata, false); + return processesSelection(rawSelectors) ? new SelectionWithProcessing(defs, metadata, factories) + : new SimpleSelection(defs, metadata, false); } private static List collectMetadata(CFMetaData cfm, @@ -336,13 +336,13 @@ public abstract class Selection } } - private static class SelectionWithFunctions extends Selection + private static class SelectionWithProcessing extends Selection { private final SelectorFactories factories; - public SelectionWithFunctions(Collection columns, - List metadata, - SelectorFactories factories) throws InvalidRequestException + public SelectionWithProcessing(Collection columns, + List metadata, + SelectorFactories factories) throws InvalidRequestException { super(columns, metadata, factories.containsWritetimeSelectorFactory(), factories.containsTTLSelectorFactory()); this.factories = factories; @@ -356,6 +356,14 @@ public abstract class Selection return factories.usesFunction(ksName, functionName); } + @Override + public int addColumnForOrdering(ColumnDefinition c) + { + int index = super.addColumnForOrdering(c); + factories.addSelectorForOrdering(c, index); + return index; + } + public boolean isAggregate() { return factories.containsOnlyAggregateFunctions(); diff --git a/src/java/org/apache/cassandra/cql3/selection/SelectorFactories.java b/src/java/org/apache/cassandra/cql3/selection/SelectorFactories.java index 4d3e9748f8..9f6025c56f 100644 --- a/src/java/org/apache/cassandra/cql3/selection/SelectorFactories.java +++ b/src/java/org/apache/cassandra/cql3/selection/SelectorFactories.java @@ -97,6 +97,16 @@ final class SelectorFactories implements Iterable return false; } + /** + * Adds a new Selector.Factory for a column that is needed only for ORDER BY purposes. + * @param def the column that is needed for ordering + * @param index the index of the column definition in the Selection's list of columns + */ + public void addSelectorForOrdering(ColumnDefinition def, int index) + { + factories.add(SimpleSelector.newFactory(def.name.toString(), index, def.type)); + } + /** * Checks if this SelectorFactories contains only factories for aggregates. * diff --git a/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java b/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java index 7dc9c66b20..ade97a03fd 100644 --- a/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java @@ -634,7 +634,7 @@ public abstract class ModificationStatement implements CQLStatement, MeasurableF } for (ColumnDefinition def : columnsWithConditions) defs.add(def); - selection = Selection.forColumns(defs); + selection = Selection.forColumns(new ArrayList<>(defs)); } long now = System.currentTimeMillis(); diff --git a/test/unit/org/apache/cassandra/cql3/CQLTester.java b/test/unit/org/apache/cassandra/cql3/CQLTester.java index ca022377e4..fb93caef05 100644 --- a/test/unit/org/apache/cassandra/cql3/CQLTester.java +++ b/test/unit/org/apache/cassandra/cql3/CQLTester.java @@ -235,6 +235,13 @@ public abstract class CQLTester } } + protected void dropTable(String query) + { + String fullQuery = String.format(query, KEYSPACE + "." + currentTable); + logger.info(fullQuery); + schemaChange(fullQuery); + } + protected void createIndex(String query) { String fullQuery = String.format(query, KEYSPACE + "." + currentTable); diff --git a/test/unit/org/apache/cassandra/cql3/SelectionOrderingTest.java b/test/unit/org/apache/cassandra/cql3/SelectionOrderingTest.java new file mode 100644 index 0000000000..301aaf46a9 --- /dev/null +++ b/test/unit/org/apache/cassandra/cql3/SelectionOrderingTest.java @@ -0,0 +1,233 @@ +/* + * 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; + +import org.junit.Test; + +public class SelectionOrderingTest extends CQLTester +{ + + @Test + public void testNormalSelectionOrderSingleClustering() throws Throwable + { + for (String descOption : new String[]{"", " WITH CLUSTERING ORDER BY (b DESC)"}) + { + createTable("CREATE TABLE %s (a int, b int, c int, PRIMARY KEY (a, b))" + descOption); + execute("INSERT INTO %s (a, b, c) VALUES (?, ?, ?)", 0, 0, 0); + execute("INSERT INTO %s (a, b, c) VALUES (?, ?, ?)", 0, 1, 1); + execute("INSERT INTO %s (a, b, c) VALUES (?, ?, ?)", 0, 2, 2); + + assertRows(execute("SELECT * FROM %s WHERE a=? ORDER BY b ASC", 0), + row(0, 0, 0), + row(0, 1, 1), + row(0, 2, 2) + ); + + assertRows(execute("SELECT * FROM %s WHERE a=? ORDER BY b DESC", 0), + row(0, 2, 2), + row(0, 1, 1), + row(0, 0, 0) + ); + + // order by the only column in the selection + assertRows(execute("SELECT b FROM %s WHERE a=? ORDER BY b ASC", 0), + row(0), row(1), row(2)); + + assertRows(execute("SELECT b FROM %s WHERE a=? ORDER BY b DESC", 0), + row(2), row(1), row(0)); + + // order by a column not in the selection + assertRows(execute("SELECT c FROM %s WHERE a=? ORDER BY b ASC", 0), + row(0), row(1), row(2)); + + assertRows(execute("SELECT c FROM %s WHERE a=? ORDER BY b DESC", 0), + row(2), row(1), row(0)); + } + } + + @Test + public void testFunctionSelectionOrderSingleClustering() throws Throwable + { + for (String descOption : new String[]{"", " WITH CLUSTERING ORDER BY (b DESC)"}) + { + createTable("CREATE TABLE %s (a int, b int, c int, PRIMARY KEY (a, b))" + descOption); + execute("INSERT INTO %s (a, b, c) VALUES (?, ?, ?)", 0, 0, 0); + execute("INSERT INTO %s (a, b, c) VALUES (?, ?, ?)", 0, 1, 1); + execute("INSERT INTO %s (a, b, c) VALUES (?, ?, ?)", 0, 2, 2); + + // order by the only column in the selection + assertRows(execute("SELECT blobAsInt(intAsBlob(b)) FROM %s WHERE a=? ORDER BY b ASC", 0), + row(0), row(1), row(2)); + + assertRows(execute("SELECT blobAsInt(intAsBlob(b)) FROM %s WHERE a=? ORDER BY b DESC", 0), + row(2), row(1), row(0)); + + // order by a column not in the selection + assertRows(execute("SELECT blobAsInt(intAsBlob(c)) FROM %s WHERE a=? ORDER BY b ASC", 0), + row(0), row(1), row(2)); + + assertRows(execute("SELECT blobAsInt(intAsBlob(c)) FROM %s WHERE a=? ORDER BY b DESC", 0), + row(2), row(1), row(0)); + + assertInvalid("SELECT * FROM %s WHERE a=? ORDER BY c ASC", 0); + assertInvalid("SELECT * FROM %s WHERE a=? ORDER BY c DESC", 0); + } + } + + @Test + public void testFieldSelectionOrderSingleClustering() throws Throwable + { + String type = createType("CREATE TYPE %s (a int)"); + + for (String descOption : new String[]{"", " WITH CLUSTERING ORDER BY (b DESC)"}) + { + createTable("CREATE TABLE %s (a int, b int, c frozen<" + type + " >, PRIMARY KEY (a, b))" + descOption); + execute("INSERT INTO %s (a, b, c) VALUES (?, ?, {a: ?})", 0, 0, 0); + execute("INSERT INTO %s (a, b, c) VALUES (?, ?, {a: ?})", 0, 1, 1); + execute("INSERT INTO %s (a, b, c) VALUES (?, ?, {a: ?})", 0, 2, 2); + + // order by a column not in the selection + assertRows(execute("SELECT c.a FROM %s WHERE a=? ORDER BY b ASC", 0), + row(0), row(1), row(2)); + + assertRows(execute("SELECT c.a FROM %s WHERE a=? ORDER BY b DESC", 0), + row(2), row(1), row(0)); + + assertRows(execute("SELECT blobAsInt(intAsBlob(c.a)) FROM %s WHERE a=? ORDER BY b DESC", 0), + row(2), row(1), row(0)); + dropTable("DROP TABLE %s"); + } + } + + @Test + public void testNormalSelectionOrderMultipleClustering() throws Throwable + { + createTable("CREATE TABLE %s (a int, b int, c int, d int, PRIMARY KEY (a, b, c))"); + execute("INSERT INTO %s (a, b, c, d) VALUES (?, ?, ?, ?)", 0, 0, 0, 0); + execute("INSERT INTO %s (a, b, c, d) VALUES (?, ?, ?, ?)", 0, 0, 1, 1); + execute("INSERT INTO %s (a, b, c, d) VALUES (?, ?, ?, ?)", 0, 0, 2, 2); + execute("INSERT INTO %s (a, b, c, d) VALUES (?, ?, ?, ?)", 0, 1, 0, 3); + execute("INSERT INTO %s (a, b, c, d) VALUES (?, ?, ?, ?)", 0, 1, 1, 4); + execute("INSERT INTO %s (a, b, c, d) VALUES (?, ?, ?, ?)", 0, 1, 2, 5); + + assertRows(execute("SELECT * FROM %s WHERE a=? ORDER BY b ASC", 0), + row(0, 0, 0, 0), + row(0, 0, 1, 1), + row(0, 0, 2, 2), + row(0, 1, 0, 3), + row(0, 1, 1, 4), + row(0, 1, 2, 5) + ); + + assertRows(execute("SELECT * FROM %s WHERE a=? ORDER BY b DESC", 0), + row(0, 1, 2, 5), + row(0, 1, 1, 4), + row(0, 1, 0, 3), + row(0, 0, 2, 2), + row(0, 0, 1, 1), + row(0, 0, 0, 0) + ); + + assertRows(execute("SELECT * FROM %s WHERE a=? ORDER BY b DESC, c DESC", 0), + row(0, 1, 2, 5), + row(0, 1, 1, 4), + row(0, 1, 0, 3), + row(0, 0, 2, 2), + row(0, 0, 1, 1), + row(0, 0, 0, 0) + ); + + assertInvalid("SELECT * FROM %s WHERE a=? ORDER BY c ASC", 0); + assertInvalid("SELECT * FROM %s WHERE a=? ORDER BY c DESC", 0); + assertInvalid("SELECT * FROM %s WHERE a=? ORDER BY b ASC, c DESC", 0); + assertInvalid("SELECT * FROM %s WHERE a=? ORDER BY b DESC, c ASC", 0); + assertInvalid("SELECT * FROM %s WHERE a=? ORDER BY d ASC", 0); + + // select and order by b + assertRows(execute("SELECT b FROM %s WHERE a=? ORDER BY b ASC", 0), + row(0), row(0), row(0), row(1), row(1), row(1)); + assertRows(execute("SELECT b FROM %s WHERE a=? ORDER BY b DESC", 0), + row(1), row(1), row(1), row(0), row(0), row(0)); + + // select c, order by b + assertRows(execute("SELECT c FROM %s WHERE a=? ORDER BY b ASC", 0), + row(0), row(1), row(2), row(0), row(1), row(2)); + assertRows(execute("SELECT c FROM %s WHERE a=? ORDER BY b DESC", 0), + row(2), row(1), row(0), row(2), row(1), row(0)); + + // select c, order by b, c + assertRows(execute("SELECT c FROM %s WHERE a=? ORDER BY b ASC, c ASC", 0), + row(0), row(1), row(2), row(0), row(1), row(2)); + assertRows(execute("SELECT c FROM %s WHERE a=? ORDER BY b DESC, c DESC", 0), + row(2), row(1), row(0), row(2), row(1), row(0)); + + // select d, order by b, c + assertRows(execute("SELECT d FROM %s WHERE a=? ORDER BY b ASC, c ASC", 0), + row(0), row(1), row(2), row(3), row(4), row(5)); + assertRows(execute("SELECT d FROM %s WHERE a=? ORDER BY b DESC, c DESC", 0), + row(5), row(4), row(3), row(2), row(1), row(0)); + } + + @Test + public void testFunctionSelectionOrderMultipleClustering() throws Throwable + { + createTable("CREATE TABLE %s (a int, b int, c int, d int, PRIMARY KEY (a, b, c))"); + execute("INSERT INTO %s (a, b, c, d) VALUES (?, ?, ?, ?)", 0, 0, 0, 0); + execute("INSERT INTO %s (a, b, c, d) VALUES (?, ?, ?, ?)", 0, 0, 1, 1); + execute("INSERT INTO %s (a, b, c, d) VALUES (?, ?, ?, ?)", 0, 0, 2, 2); + execute("INSERT INTO %s (a, b, c, d) VALUES (?, ?, ?, ?)", 0, 1, 0, 3); + execute("INSERT INTO %s (a, b, c, d) VALUES (?, ?, ?, ?)", 0, 1, 1, 4); + execute("INSERT INTO %s (a, b, c, d) VALUES (?, ?, ?, ?)", 0, 1, 2, 5); + + assertInvalid("SELECT blobAsInt(intAsBlob(b)) FROM %s WHERE a=? ORDER BY c ASC", 0); + assertInvalid("SELECT blobAsInt(intAsBlob(b)) FROM %s WHERE a=? ORDER BY c DESC", 0); + assertInvalid("SELECT blobAsInt(intAsBlob(b)) FROM %s WHERE a=? ORDER BY b ASC, c DESC", 0); + assertInvalid("SELECT blobAsInt(intAsBlob(b)) FROM %s WHERE a=? ORDER BY b DESC, c ASC", 0); + assertInvalid("SELECT blobAsInt(intAsBlob(b)) FROM %s WHERE a=? ORDER BY d ASC", 0); + + // select and order by b + assertRows(execute("SELECT blobAsInt(intAsBlob(b)) FROM %s WHERE a=? ORDER BY b ASC", 0), + row(0), row(0), row(0), row(1), row(1), row(1)); + assertRows(execute("SELECT blobAsInt(intAsBlob(b)) FROM %s WHERE a=? ORDER BY b DESC", 0), + row(1), row(1), row(1), row(0), row(0), row(0)); + + assertRows(execute("SELECT b, blobAsInt(intAsBlob(b)) FROM %s WHERE a=? ORDER BY b ASC", 0), + row(0, 0), row(0, 0), row(0, 0), row(1, 1), row(1, 1), row(1, 1)); + assertRows(execute("SELECT b, blobAsInt(intAsBlob(b)) FROM %s WHERE a=? ORDER BY b DESC", 0), + row(1, 1), row(1, 1), row(1, 1), row(0, 0), row(0, 0), row(0, 0)); + + // select c, order by b + assertRows(execute("SELECT blobAsInt(intAsBlob(c)) FROM %s WHERE a=? ORDER BY b ASC", 0), + row(0), row(1), row(2), row(0), row(1), row(2)); + assertRows(execute("SELECT blobAsInt(intAsBlob(c)) FROM %s WHERE a=? ORDER BY b DESC", 0), + row(2), row(1), row(0), row(2), row(1), row(0)); + + // select c, order by b, c + assertRows(execute("SELECT blobAsInt(intAsBlob(c)) FROM %s WHERE a=? ORDER BY b ASC, c ASC", 0), + row(0), row(1), row(2), row(0), row(1), row(2)); + assertRows(execute("SELECT blobAsInt(intAsBlob(c)) FROM %s WHERE a=? ORDER BY b DESC, c DESC", 0), + row(2), row(1), row(0), row(2), row(1), row(0)); + + // select d, order by b, c + assertRows(execute("SELECT blobAsInt(intAsBlob(d)) FROM %s WHERE a=? ORDER BY b ASC, c ASC", 0), + row(0), row(1), row(2), row(3), row(4), row(5)); + assertRows(execute("SELECT blobAsInt(intAsBlob(d)) FROM %s WHERE a=? ORDER BY b DESC, c DESC", 0), + row(5), row(4), row(3), row(2), row(1), row(0)); + + } +}