Fix cql error with ORDER BY when using IN patch by Pavel Yaskevich; reviewed by Sylvain Lebresne for CASSANDRA-4612
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/76e092b2 Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/76e092b2 Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/76e092b2 Branch: refs/heads/trunk Commit: 76e092b2d3785ac1549f1af16702839bee570c69 Parents: 7371e10 Author: Pavel Yaskevich <xe...@apache.org> Authored: Thu Sep 6 18:11:05 2012 +0300 Committer: Pavel Yaskevich <xe...@apache.org> Committed: Fri Sep 7 18:59:46 2012 +0300 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../cassandra/cql3/statements/SelectStatement.java | 75 ++++++++++----- 2 files changed, 53 insertions(+), 23 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/76e092b2/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index f192be2..ed4f026 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -2,6 +2,7 @@ * (cql3) fix potential NPE with both equal and unequal restriction (CASSANDRA-4532) * (cql3) improves ORDER BY validation (CASSANDRA-4624) * Fix potential deadlock during counter writes (CASSANDRA-4578) + * Fix cql error with ORDER BY when using IN (CASSANDRA-4612) 1.1.5 http://git-wip-us.apache.org/repos/asf/cassandra/blob/76e092b2/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 1d0918e..2f47f86 100644 --- a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java @@ -833,30 +833,38 @@ public class SelectStatement implements CQLStatement if (parameters.orderings.size() == 1) { CFDefinition.Name ordering = cfDef.get(parameters.orderings.keySet().iterator().next()); - Collections.sort(cqlRows, new SingleColumnComparator(ordering.position + 1, ordering.type)); + Collections.sort(cqlRows, new SingleColumnComparator(getColumnPositionInSelect(ordering), ordering.type)); return; } - // figures out where ordering would start in results (startPosition), - // builds a composite type for multi-column comparison from the comparators of the ordering components + // builds a 'composite' type for multi-column comparison from the comparators of the ordering components // and passes collected position information and built composite comparator to CompositeComparator to do // an actual comparison of the CQL rows. - int startPosition = -1; - List<AbstractType<?>> types = new ArrayList<AbstractType<?>>(); + List<AbstractType<?>> types = new ArrayList<AbstractType<?>>(parameters.orderings.size()); + int[] positions = new int[parameters.orderings.size()]; + int idx = 0; for (ColumnIdentifier identifier : parameters.orderings.keySet()) { CFDefinition.Name orderingColumn = cfDef.get(identifier); - - if (startPosition == -1) - startPosition = orderingColumn.position + 1; - types.add(orderingColumn.type); + positions[idx++] = getColumnPositionInSelect(orderingColumn); } - Collections.sort(cqlRows, new CompositeComparator(startPosition, types)); + Collections.sort(cqlRows, new CompositeComparator(types, positions)); } + // determine position of column in the select clause + private int getColumnPositionInSelect(CFDefinition.Name columnName) + { + for (int i = 0; i < selectedNames.size(); i++) + { + if (selectedNames.get(i).left.equals(columnName)) + return i; + } + + throw new IllegalArgumentException(String.format("Column %s wasn't found in select clause.", columnName)); + } /** * For sparse composite, returns wheter two columns belong to the same @@ -1073,6 +1081,28 @@ public class SelectStatement implements CQLStatement if (stmt.isKeyRange()) throw new InvalidRequestException("ORDER BY is only supported when the partition key is restricted by an EQ or an IN."); + // check if we are trying to order by column that wouldn't be included in the results + if (!stmt.selectedNames.isEmpty()) // empty means wildcard was used + { + for (ColumnIdentifier column : stmt.parameters.orderings.keySet()) + { + CFDefinition.Name name = cfDef.get(column); + + boolean hasColumn = false; + for (Pair<CFDefinition.Name, Selector> selectPair : stmt.selectedNames) + { + if (selectPair.left.equals(name)) + { + hasColumn = true; + break; + } + } + + if (!hasColumn) + throw new InvalidRequestException("ORDER BY could not be used on columns missing in select clause."); + } + } + Boolean[] reversedMap = new Boolean[cfDef.columns.size()]; int i = 0; for (Map.Entry<ColumnIdentifier, Boolean> entry : stmt.parameters.orderings.entrySet()) @@ -1346,30 +1376,29 @@ public class SelectStatement implements CQLStatement */ private static class CompositeComparator implements Comparator<CqlRow> { - private final int startColumnIndex; - private final List<AbstractType<?>> orderings; + private final List<AbstractType<?>> orderTypes; + private final int[] positions; - private CompositeComparator(int startIndex, List<AbstractType<?>> orderComparators) + private CompositeComparator(List<AbstractType<?>> orderTypes, int[] positions) { - startColumnIndex = startIndex; - orderings = orderComparators; + this.orderTypes = orderTypes; + this.positions = positions; } public int compare(CqlRow a, CqlRow b) { - int currentIndex = startColumnIndex; - - for (AbstractType<?> comparator : orderings) + for (int i = 0; i < positions.length; i++) { - ByteBuffer aValue = a.getColumns().get(currentIndex).bufferForValue(); - ByteBuffer bValue = b.getColumns().get(currentIndex).bufferForValue(); + AbstractType<?> type = orderTypes.get(i); + int columnPos = positions[i]; - int comparison = comparator.compare(aValue, bValue); + ByteBuffer aValue = a.getColumns().get(columnPos).bufferForValue(); + ByteBuffer bValue = b.getColumns().get(columnPos).bufferForValue(); + + int comparison = type.compare(aValue, bValue); if (comparison != 0) return comparison; - - currentIndex++; } return 0;