diff --git a/CHANGES.txt b/CHANGES.txt index afaf114fa0..8a41b2e2c9 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -16,6 +16,8 @@ Merged from 3.0: * MV should use the maximum timestamp of the primary key (CASSANDRA-10910) * Fix potential assertion error during compaction (CASSANDRA-10944) Merged from 2.2: +2.2.5 + * Fix potential NPE on ORDER BY queries with IN (CASSANDRA-10955) * Start L0 STCS-compactions even if there is a L0 -> L1 compaction going (CASSANDRA-10979) * Make UUID LSB unique per process (CASSANDRA-7925) diff --git a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java index c196e5eb92..4c9c875abc 100644 --- a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java @@ -1054,10 +1054,21 @@ public class SelectStatement implements CQLStatement } } + private static abstract class ColumnComparator implements Comparator + { + protected final int compare(Comparator 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> + private static class SingleColumnComparator extends ColumnComparator> { private final int index; private final Comparator comparator; @@ -1070,14 +1081,14 @@ public class SelectStatement implements CQLStatement public int compare(List a, List 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> + private static class CompositeComparator extends ColumnComparator> { private final List> orderTypes; private final List positions; @@ -1095,10 +1106,7 @@ public class SelectStatement implements CQLStatement Comparator 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; 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 ae6f772f90..6e06419158 100644 --- a/test/unit/org/apache/cassandra/cql3/validation/operations/SelectOrderByTest.java +++ b/test/unit/org/apache/cassandra/cql3/validation/operations/SelectOrderByTest.java @@ -433,6 +433,49 @@ public class SelectOrderByTest extends CQLTester "SELECT v as c2 FROM %s where pk1 = ? AND pk2 IN (?, ?) ORDER BY c1 DESC , c2 DESC LIMIT 0; ", 1, 1, 2); } + @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()