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/trunk Commit: deafdbe373df3717ec21f8e52d93f3d02bb5094a Parents: 0b479a7 Author: Benjamin Lerer <[email protected]> Authored: Wed Jan 13 15:41:51 2016 +0100 Committer: Sylvain Lebresne <[email protected]> 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()
