Repository: cassandra Updated Branches: refs/heads/trunk 6f2b855c6 -> 737a3385c
Fix sorting for queries with an IN condition on partition key columns patch by Benjamin Lerer; reviewed by Sam Tunnicliffe for CASSANDRA-10363 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/f587397c Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/f587397c Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/f587397c Branch: refs/heads/trunk Commit: f587397c9c41c1a68b4e46fc16bad8d48c975e4d Parents: 806378c Author: blerer <benjamin.le...@datastax.com> Authored: Fri Oct 16 14:41:54 2015 +0200 Committer: blerer <benjamin.le...@datastax.com> Committed: Fri Oct 16 14:41:54 2015 +0200 ---------------------------------------------------------------------- .../cassandra/cql3/ColumnSpecification.java | 10 ++++ .../org/apache/cassandra/cql3/ResultSet.java | 5 ++ .../apache/cassandra/cql3/UntypedResultSet.java | 6 +-- .../cql3/statements/SelectStatement.java | 18 +------ .../cassandra/cql3/statements/Selection.java | 45 +++++++++++++++- .../org/apache/cassandra/cql3/CQLTester.java | 3 +- .../operations/SelectOrderByTest.java | 54 +++++++++++++++++++- 7 files changed, 116 insertions(+), 25 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/f587397c/src/java/org/apache/cassandra/cql3/ColumnSpecification.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql3/ColumnSpecification.java b/src/java/org/apache/cassandra/cql3/ColumnSpecification.java index f5f921d..836c6b9 100644 --- a/src/java/org/apache/cassandra/cql3/ColumnSpecification.java +++ b/src/java/org/apache/cassandra/cql3/ColumnSpecification.java @@ -55,4 +55,14 @@ public class ColumnSpecification { return Objects.hashCode(ksName, cfName, name, type); } + + @Override + public String toString() + { + return Objects.toStringHelper(this) + .add("name", name) + .add("type", type) + .toString(); + } + } http://git-wip-us.apache.org/repos/asf/cassandra/blob/f587397c/src/java/org/apache/cassandra/cql3/ResultSet.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql3/ResultSet.java b/src/java/org/apache/cassandra/cql3/ResultSet.java index 813fd48..85cba57 100644 --- a/src/java/org/apache/cassandra/cql3/ResultSet.java +++ b/src/java/org/apache/cassandra/cql3/ResultSet.java @@ -284,6 +284,11 @@ public class ResultSet return names == null ? columnCount : names.size(); } + /** + * Adds the specified column which will not be serialized. + * + * @param name the column + */ public void addNonSerializedColumn(ColumnSpecification name) { // See comment above. Because columnCount doesn't account the newly added name, it http://git-wip-us.apache.org/repos/asf/cassandra/blob/f587397c/src/java/org/apache/cassandra/cql3/UntypedResultSet.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql3/UntypedResultSet.java b/src/java/org/apache/cassandra/cql3/UntypedResultSet.java index 81482ef..a0b6ae7 100644 --- a/src/java/org/apache/cassandra/cql3/UntypedResultSet.java +++ b/src/java/org/apache/cassandra/cql3/UntypedResultSet.java @@ -76,7 +76,7 @@ public abstract class UntypedResultSet implements Iterable<UntypedResultSet.Row> { if (cqlRows.rows.size() != 1) throw new IllegalStateException("One row required, " + cqlRows.rows.size() + " found"); - return new Row(cqlRows.metadata.names, cqlRows.rows.get(0)); + return new Row(cqlRows.metadata.requestNames(), cqlRows.rows.get(0)); } public Iterator<Row> iterator() @@ -89,7 +89,7 @@ public abstract class UntypedResultSet implements Iterable<UntypedResultSet.Row> { if (!iter.hasNext()) return endOfData(); - return new Row(cqlRows.metadata.names, iter.next()); + return new Row(cqlRows.metadata.requestNames(), iter.next()); } }; } @@ -154,7 +154,7 @@ public abstract class UntypedResultSet implements Iterable<UntypedResultSet.Row> this.select = select; this.pager = pager; this.pageSize = pageSize; - this.metadata = select.getResultMetadata().names; + this.metadata = select.getResultMetadata().requestNames(); } public int size() http://git-wip-us.apache.org/repos/asf/cassandra/blob/f587397c/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 44b780e..8a6d704 100644 --- a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java @@ -2207,7 +2207,7 @@ public class SelectStatement implements CQLStatement if (def == null) handleUnrecognizedOrderingColumn(column); - int index = indexOf(def, stmt.selection); + int index = stmt.selection.getResultSetIndex(def); if (index < 0) index = stmt.selection.addColumnForOrdering(def); stmt.orderingIndexes.put(def.name, index); @@ -2326,22 +2326,6 @@ public class SelectStatement implements CQLStatement return needFiltering; } - private int indexOf(ColumnDefinition def, Selection selection) - { - return indexOf(def, selection.getColumns().iterator()); - } - - private int indexOf(final ColumnDefinition def, Iterator<ColumnDefinition> defs) - { - return Iterators.indexOf(defs, new Predicate<ColumnDefinition>() - { - public boolean apply(ColumnDefinition n) - { - return def.name.equals(n.name); - } - }); - } - private SingleColumnRelation findInclusiveClusteringRelationForCompact(CFMetaData cfm) { for (Relation r : whereClause) http://git-wip-us.apache.org/repos/asf/cassandra/blob/f587397c/src/java/org/apache/cassandra/cql3/statements/Selection.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql3/statements/Selection.java b/src/java/org/apache/cassandra/cql3/statements/Selection.java index d29b917..f50ec1b 100644 --- a/src/java/org/apache/cassandra/cql3/statements/Selection.java +++ b/src/java/org/apache/cassandra/cql3/statements/Selection.java @@ -20,7 +20,6 @@ package org.apache.cassandra.cql3.statements; import java.nio.ByteBuffer; import java.util.*; -import com.google.common.collect.Iterators; import com.google.common.collect.Lists; import org.apache.cassandra.cql3.ColumnSpecification; @@ -105,6 +104,29 @@ public abstract class Selection return columns.size() - 1; } + /** + * Returns the index of the specified column within the resultset + * @param c the column + * @return the index of the specified column within the resultset or -1 + */ + public int getResultSetIndex(ColumnDefinition c) + { + return getColumnIndex(c); + } + + /** + * Returns the index of the specified column + * @param c the column + * @return the index of the specified column or -1 + */ + protected final int getColumnIndex(ColumnDefinition c) + { + for (int i = 0, m = columns.size(); i < m; i++) + if (columns.get(i).name.equals(c.name)) + return i; + return -1; + } + private static boolean requiresProcessing(List<RawSelector> rawSelectors) { for (RawSelector rawSelector : rawSelectors) @@ -507,11 +529,30 @@ public abstract class Selection } @Override + public int getResultSetIndex(ColumnDefinition c) + { + int index = getColumnIndex(c); + + if (index < 0) + return -1; + + for (int i = 0, m = selectors.size(); i < m; i++) + { + Selector selector = selectors.get(i); + if (selector instanceof SimpleSelector && ((SimpleSelector) selector).idx == index) + { + return i; + } + } + return -1; + } + + @Override public int addColumnForOrdering(ColumnDefinition c) { int index = super.addColumnForOrdering(c); selectors.add(new SimpleSelector(c.name.toString(), index, c.type)); - return index; + return selectors.size() - 1; } } http://git-wip-us.apache.org/repos/asf/cassandra/blob/f587397c/test/unit/org/apache/cassandra/cql3/CQLTester.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/cql3/CQLTester.java b/test/unit/org/apache/cassandra/cql3/CQLTester.java index 8e620af..5288c2f 100644 --- a/test/unit/org/apache/cassandra/cql3/CQLTester.java +++ b/test/unit/org/apache/cassandra/cql3/CQLTester.java @@ -416,8 +416,7 @@ public abstract class CQLTester for (int j = 0; j < meta.size(); j++) { ColumnSpecification column = meta.get(j); - Object expectedValue = expected[j]; - ByteBuffer expectedByteValue = makeByteBuffer(expected[j], (AbstractType)column.type); + ByteBuffer expectedByteValue = makeByteBuffer(expected[j], column.type); ByteBuffer actualValue = actual.getBytes(column.name.toString()); if (!Objects.equal(expectedByteValue, actualValue)) http://git-wip-us.apache.org/repos/asf/cassandra/blob/f587397c/test/unit/org/apache/cassandra/cql3/validation/operations/SelectOrderByTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/cql3/validation/operations/SelectOrderByTest.java b/test/unit/org/apache/cassandra/cql3/validation/operations/SelectOrderByTest.java index 9d2e594..cf923bc 100644 --- a/test/unit/org/apache/cassandra/cql3/validation/operations/SelectOrderByTest.java +++ b/test/unit/org/apache/cassandra/cql3/validation/operations/SelectOrderByTest.java @@ -316,7 +316,7 @@ public class SelectOrderByTest extends CQLTester } /** - * Check that order-by works with IN (#4327) + * Check that order-by works with IN (#4327, #10363) * migrated from cql_tests.py:TestCQL.order_by_with_in_test() */ @Test @@ -337,6 +337,58 @@ public class SelectOrderByTest extends CQLTester assertRows(execute("SELECT my_id, col1 FROM %s WHERE my_id in('key1', 'key2', 'key3') ORDER BY col1"), row("key1", 1), row("key3", 2), row("key2", 3)); + + createTable("CREATE TABLE %s (pk1 int, pk2 int, c int, v text, PRIMARY KEY ((pk1, pk2), c) )"); + execute("INSERT INTO %s (pk1, pk2, c, v) VALUES (?, ?, ?, ?)", 1, 1, 2, "A"); + execute("INSERT INTO %s (pk1, pk2, c, v) VALUES (?, ?, ?, ?)", 1, 2, 1, "B"); + execute("INSERT INTO %s (pk1, pk2, c, v) VALUES (?, ?, ?, ?)", 1, 3, 3, "C"); + execute("INSERT INTO %s (pk1, pk2, c, v) VALUES (?, ?, ?, ?)", 1, 1, 4, "D"); + + assertRows(execute("SELECT v, ttl(v), c FROM %s where pk1 = ? AND pk2 IN (?, ?) ORDER BY c; ", 1, 1, 2), + row("B", null, 1), + row("A", null, 2), + row("D", null, 4)); + + assertRows(execute("SELECT v, ttl(v), c as name_1 FROM %s where pk1 = ? AND pk2 IN (?, ?) ORDER BY c; ", 1, 1, 2), + row("B", null, 1), + row("A", null, 2), + row("D", null, 4)); + + assertRows(execute("SELECT v FROM %s where pk1 = ? AND pk2 IN (?, ?) ORDER BY c; ", 1, 1, 2), + row("B"), + row("A"), + row("D")); + + assertRows(execute("SELECT v as c FROM %s where pk1 = ? AND pk2 IN (?, ?) ORDER BY c; ", 1, 1, 2), + row("B"), + row("A"), + row("D")); + + createTable("CREATE TABLE %s (pk1 int, pk2 int, c1 int, c2 int, v text, PRIMARY KEY ((pk1, pk2), c1, c2) )"); + execute("INSERT INTO %s (pk1, pk2, c1, c2, v) VALUES (?, ?, ?, ?, ?)", 1, 1, 4, 4, "A"); + execute("INSERT INTO %s (pk1, pk2, c1, c2, v) VALUES (?, ?, ?, ?, ?)", 1, 2, 1, 2, "B"); + execute("INSERT INTO %s (pk1, pk2, c1, c2, v) VALUES (?, ?, ?, ?, ?)", 1, 3, 3, 3, "C"); + execute("INSERT INTO %s (pk1, pk2, c1, c2, v) VALUES (?, ?, ?, ?, ?)", 1, 1, 4, 1, "D"); + + assertRows(execute("SELECT v, ttl(v), c1, c2 FROM %s where pk1 = ? AND pk2 IN (?, ?) ORDER BY c1, c2; ", 1, 1, 2), + row("B", null, 1, 2), + row("D", null, 4, 1), + row("A", null, 4, 4)); + + assertRows(execute("SELECT v, ttl(v), c1 as name_1, c2 as name_2 FROM %s where pk1 = ? AND pk2 IN (?, ?) ORDER BY c1, c2; ", 1, 1, 2), + row("B", null, 1, 2), + row("D", null, 4, 1), + row("A", null, 4, 4)); + + assertRows(execute("SELECT v FROM %s where pk1 = ? AND pk2 IN (?, ?) ORDER BY c1, c2; ", 1, 1, 2), + row("B"), + row("D"), + row("A")); + + assertRows(execute("SELECT v as c2 FROM %s where pk1 = ? AND pk2 IN (?, ?) ORDER BY c1, c2; ", 1, 1, 2), + row("B"), + row("D"), + row("A")); } /**
