You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by sl...@apache.org on 2016/01/22 15:51:59 UTC

[03/10] cassandra git commit: Avoid potential NPE for queries with ORDER BY and IN

Avoid potential NPE for queries with ORDER BY and IN

patch by blerer; reviewed by beobal for CASSANDRA-10955


Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/deafdbe3
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/deafdbe3
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/deafdbe3

Branch: refs/heads/cassandra-3.3
Commit: deafdbe373df3717ec21f8e52d93f3d02bb5094a
Parents: 0b479a7
Author: Benjamin Lerer <b....@gmail.com>
Authored: Wed Jan 13 15:41:51 2016 +0100
Committer: Sylvain Lebresne <sy...@datastax.com>
Committed: Fri Jan 22 15:49:36 2016 +0100

----------------------------------------------------------------------
 CHANGES.txt                                     |  1 +
 .../cql3/statements/SelectStatement.java        | 22 ++++++----
 .../operations/SelectOrderByTest.java           | 43 ++++++++++++++++++++
 3 files changed, 59 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/deafdbe3/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 6c01e22..1a92fd6 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
 2.2.5
+ * Fix potential NPE on ORDER BY queries with IN (CASSANDRA-10955)
  * Avoid over-fetching during the page of range queries (CASSANDRA-8521)
  * Start L0 STCS-compactions even if there is a L0 -> L1 compaction
    going (CASSANDRA-10979)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/deafdbe3/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 5cfa94b..848b3a6 100644
--- a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java
+++ b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java
@@ -1092,10 +1092,21 @@ public class SelectStatement implements CQLStatement
         }
     }
 
+    private static abstract class ColumnComparator<T> implements Comparator<T>
+    {
+        protected final int compare(Comparator<ByteBuffer> comparator, ByteBuffer aValue, ByteBuffer bValue)
+        {
+            if (aValue == null)
+                return bValue == null ? 0 : -1;
+
+            return bValue == null ? 1 : comparator.compare(aValue, bValue);
+        }
+    }
+
     /**
      * Used in orderResults(...) method when single 'ORDER BY' condition where given
      */
-    private static class SingleColumnComparator implements Comparator<List<ByteBuffer>>
+    private static class SingleColumnComparator extends ColumnComparator<List<ByteBuffer>>
     {
         private final int index;
         private final Comparator<ByteBuffer> comparator;
@@ -1108,14 +1119,14 @@ public class SelectStatement implements CQLStatement
 
         public int compare(List<ByteBuffer> a, List<ByteBuffer> b)
         {
-            return comparator.compare(a.get(index), b.get(index));
+            return compare(comparator, a.get(index), b.get(index));
         }
     }
 
     /**
      * Used in orderResults(...) method when multiple 'ORDER BY' conditions where given
      */
-    private static class CompositeComparator implements Comparator<List<ByteBuffer>>
+    private static class CompositeComparator extends ColumnComparator<List<ByteBuffer>>
     {
         private final List<Comparator<ByteBuffer>> orderTypes;
         private final List<Integer> positions;
@@ -1133,10 +1144,7 @@ public class SelectStatement implements CQLStatement
                 Comparator<ByteBuffer> type = orderTypes.get(i);
                 int columnPos = positions.get(i);
 
-                ByteBuffer aValue = a.get(columnPos);
-                ByteBuffer bValue = b.get(columnPos);
-
-                int comparison = type.compare(aValue, bValue);
+                int comparison = compare(type, a.get(columnPos), b.get(columnPos));
 
                 if (comparison != 0)
                     return comparison;

http://git-wip-us.apache.org/repos/asf/cassandra/blob/deafdbe3/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 05ac3b9..f8ec13c 100644
--- a/test/unit/org/apache/cassandra/cql3/validation/operations/SelectOrderByTest.java
+++ b/test/unit/org/apache/cassandra/cql3/validation/operations/SelectOrderByTest.java
@@ -392,6 +392,49 @@ public class SelectOrderByTest extends CQLTester
                    row("A"));
     }
 
+    @Test
+    public void testOrderByForInClauseWithNullValue() throws Throwable
+    {
+        createTable("CREATE TABLE %s (a int, b int, c int, s int static, d int, PRIMARY KEY (a, b, c))");
+
+        execute("INSERT INTO %s (a, b, c, d) VALUES (1, 1, 1, 1)");
+        execute("INSERT INTO %s (a, b, c, d) VALUES (1, 1, 2, 1)");
+        execute("INSERT INTO %s (a, b, c, d) VALUES (2, 2, 1, 1)");
+        execute("INSERT INTO %s (a, b, c, d) VALUES (2, 2, 2, 1)");
+
+        execute("UPDATE %s SET s = 1 WHERE a = 1");
+        execute("UPDATE %s SET s = 2 WHERE a = 2");
+        execute("UPDATE %s SET s = 3 WHERE a = 3");
+
+        assertRows(execute("SELECT a, b, c, d, s FROM %s WHERE a IN (1, 2, 3) ORDER BY b DESC"),
+                   row(2, 2, 2, 1, 2),
+                   row(2, 2, 1, 1, 2),
+                   row(1, 1, 2, 1, 1),
+                   row(1, 1, 1, 1, 1),
+                   row(3, null, null, null, 3));
+
+        assertRows(execute("SELECT a, b, c, d, s FROM %s WHERE a IN (1, 2, 3) ORDER BY b ASC"),
+                   row(3, null, null, null, 3),
+                   row(1, 1, 1, 1, 1),
+                   row(1, 1, 2, 1, 1),
+                   row(2, 2, 1, 1, 2),
+                   row(2, 2, 2, 1, 2));
+
+        assertRows(execute("SELECT a, b, c, d, s FROM %s WHERE a IN (1, 2, 3) ORDER BY b DESC , c DESC"),
+                   row(2, 2, 2, 1, 2),
+                   row(2, 2, 1, 1, 2),
+                   row(1, 1, 2, 1, 1),
+                   row(1, 1, 1, 1, 1),
+                   row(3, null, null, null, 3));
+
+        assertRows(execute("SELECT a, b, c, d, s FROM %s WHERE a IN (1, 2, 3) ORDER BY b ASC, c ASC"),
+                   row(3, null, null, null, 3),
+                   row(1, 1, 1, 1, 1),
+                   row(1, 1, 2, 1, 1),
+                   row(2, 2, 1, 1, 2),
+                   row(2, 2, 2, 1, 2));
+    }
+
     /**
      * Test reversed comparators
      * migrated from cql_tests.py:TestCQL.reversed_comparator_test()