cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From xe...@apache.org
Subject git commit: Fix cql error with ORDER BY when using IN patch by Pavel Yaskevich; reviewed by Sylvain Lebresne for CASSANDRA-4612
Date Fri, 07 Sep 2012 16:00:55 GMT
Updated Branches:
  refs/heads/cassandra-1.1 7371e10be -> 76e092b2d


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/cassandra-1.1
Commit: 76e092b2d3785ac1549f1af16702839bee570c69
Parents: 7371e10
Author: Pavel Yaskevich <xedin@apache.org>
Authored: Thu Sep 6 18:11:05 2012 +0300
Committer: Pavel Yaskevich <xedin@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;


Mime
View raw message