You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by xe...@apache.org on 2012/09/11 00:13:31 UTC
[13/13] git commit: Fix cql error with ORDER BY when using IN patch
by Pavel Yaskevich; reviewed by Sylvain Lebresne for CASSANDRA-4612
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;