Repository: cassandra
Updated Branches:
  refs/heads/trunk d8036f936 -> 9a0eb9a31


Fix PER PARTITION LIMIT for queries requiring post-query ordering

patch by Alex Petrov; reviewed by Benjamin Lerer for CASSANDRA-11556


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

Branch: refs/heads/trunk
Commit: 9a0eb9a31e71cfc43def6497907ce2ab3d091aa1
Parents: d8036f9
Author: Alex Petrov <oleksandr.pet...@gmail.com>
Authored: Thu Apr 14 11:53:29 2016 +0200
Committer: Benjamin Lerer <b.le...@gmail.com>
Committed: Thu Apr 14 11:53:29 2016 +0200

----------------------------------------------------------------------
 CHANGES.txt                                     |   1 +
 .../cql3/statements/SelectStatement.java        |   9 +-
 .../validation/operations/SelectLimitTest.java  | 112 +++++++++++++++++++
 .../cql3/validation/operations/SelectTest.java  |  92 ---------------
 4 files changed, 119 insertions(+), 95 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/9a0eb9a3/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 3b5f1b7..443c8bc 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
 3.6
+ * Fix PER PARTITION LIMIT for queries requiring post-query ordering 
(CASSANDRA-11556)
  * Allow instantiation of UDTs and tuples in UDFs (CASSANDRA-10818)
  * Support UDT in CQLSSTableWriter (CASSANDRA-10624)
  * Support for non-frozen user-defined types, updating

http://git-wip-us.apache.org/repos/asf/cassandra/blob/9a0eb9a3/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 9745b05..2f64b25 100644
--- a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java
+++ b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java
@@ -596,12 +596,12 @@ public class SelectStatement implements CQLStatement
         // Whenever we support GROUP BY, we'll have to add a new DataLimits 
kind that knows how things are grouped and is thus
         // able to apply the user limit properly.
         // If we do post ordering we need to get all the results sorted before 
we can trim them.
-        if (!selection.isAggregate() && !needsPostQueryOrdering())
+        if (!selection.isAggregate())
         {
-            cqlRowLimit = userLimit;
+            if (!needsPostQueryOrdering())
+                cqlRowLimit = userLimit;
             cqlPerPartitionLimit = perPartitionLimit;
         }
-
         if (parameters.isDistinct)
             return cqlRowLimit == DataLimits.NO_LIMIT ? 
DataLimits.DISTINCT_NONE : DataLimits.distinctLimits(cqlRowLimit);
 
@@ -853,6 +853,9 @@ public class SelectStatement implements CQLStatement
                 validateDistinctSelection(cfm, selection, restrictions);
             }
 
+            checkFalse(selection.isAggregate() && perPartitionLimit != null,
+                       "PER PARTITION LIMIT is not allowed with aggregate 
queries.");
+
             Comparator<List<ByteBuffer>> orderingComparator = null;
             boolean isReversed = false;
 

http://git-wip-us.apache.org/repos/asf/cassandra/blob/9a0eb9a3/test/unit/org/apache/cassandra/cql3/validation/operations/SelectLimitTest.java
----------------------------------------------------------------------
diff --git 
a/test/unit/org/apache/cassandra/cql3/validation/operations/SelectLimitTest.java
 
b/test/unit/org/apache/cassandra/cql3/validation/operations/SelectLimitTest.java
index cf028a1..1dffb0c 100644
--- 
a/test/unit/org/apache/cassandra/cql3/validation/operations/SelectLimitTest.java
+++ 
b/test/unit/org/apache/cassandra/cql3/validation/operations/SelectLimitTest.java
@@ -113,4 +113,116 @@ public class SelectLimitTest extends CQLTester
                    row(2, 2),
                    row(2, 3));
     }
+
+    @Test
+    public void testPerPartitionLimit() throws Throwable
+    {
+        perPartitionLimitTest(false);
+    }
+
+    @Test
+    public void testPerPartitionLimitWithCompactStorage() throws Throwable
+    {
+        perPartitionLimitTest(true);
+    }
+
+    private void perPartitionLimitTest(boolean withCompactStorage) throws 
Throwable
+    {
+        String query = "CREATE TABLE %s (a int, b int, c int, PRIMARY KEY (a, 
b))";
+
+        if (withCompactStorage)
+            createTable(query + " WITH COMPACT STORAGE");
+        else
+            createTable(query);
+
+        for (int i = 0; i < 5; i++)
+        {
+            for (int j = 0; j < 5; j++)
+            {
+                execute("INSERT INTO %s (a, b, c) VALUES (?, ?, ?)", i, j, j);
+            }
+        }
+
+        assertInvalidMessage("LIMIT must be strictly positive",
+                             "SELECT * FROM %s PER PARTITION LIMIT ?", 0);
+        assertInvalidMessage("LIMIT must be strictly positive",
+                             "SELECT * FROM %s PER PARTITION LIMIT ?", -1);
+
+        assertRowsIgnoringOrder(execute("SELECT * FROM %s PER PARTITION LIMIT 
?", 2),
+                                row(0, 0, 0),
+                                row(0, 1, 1),
+                                row(1, 0, 0),
+                                row(1, 1, 1),
+                                row(2, 0, 0),
+                                row(2, 1, 1),
+                                row(3, 0, 0),
+                                row(3, 1, 1),
+                                row(4, 0, 0),
+                                row(4, 1, 1));
+
+        // Combined Per Partition and "global" limit
+        assertRowCount(execute("SELECT * FROM %s PER PARTITION LIMIT ? LIMIT 
?", 2, 6),
+                       6);
+
+        // odd amount of results
+        assertRowCount(execute("SELECT * FROM %s PER PARTITION LIMIT ? LIMIT 
?", 2, 5),
+                       5);
+
+        // IN query
+        assertRows(execute("SELECT * FROM %s WHERE a IN (2,3) PER PARTITION 
LIMIT ?", 2),
+                   row(2, 0, 0),
+                   row(2, 1, 1),
+                   row(3, 0, 0),
+                   row(3, 1, 1));
+
+        assertRows(execute("SELECT * FROM %s WHERE a IN (2,3) PER PARTITION 
LIMIT ? LIMIT 3", 2),
+                   row(2, 0, 0),
+                   row(2, 1, 1),
+                   row(3, 0, 0));
+
+        assertRows(execute("SELECT * FROM %s WHERE a IN (1,2,3) PER PARTITION 
LIMIT ? LIMIT 3", 2),
+                   row(1, 0, 0),
+                   row(1, 1, 1),
+                   row(2, 0, 0));
+
+        // with restricted partition key
+        assertRows(execute("SELECT * FROM %s WHERE a = ? PER PARTITION LIMIT 
?", 2, 3),
+                   row(2, 0, 0),
+                   row(2, 1, 1),
+                   row(2, 2, 2));
+
+        // with ordering
+        assertRows(execute("SELECT * FROM %s WHERE a IN (3, 2) ORDER BY b DESC 
PER PARTITION LIMIT ?", 2),
+                   row(2, 4, 4),
+                   row(3, 4, 4),
+                   row(2, 3, 3),
+                   row(3, 3, 3));
+
+        assertRows(execute("SELECT * FROM %s WHERE a IN (3, 2) ORDER BY b DESC 
PER PARTITION LIMIT ? LIMIT ?", 3, 4),
+                   row(2, 4, 4),
+                   row(3, 4, 4),
+                   row(2, 3, 3),
+                   row(3, 3, 3));
+
+        assertRows(execute("SELECT * FROM %s WHERE a = ? ORDER BY b DESC PER 
PARTITION LIMIT ?", 2, 3),
+                   row(2, 4, 4),
+                   row(2, 3, 3),
+                   row(2, 2, 2));
+
+        // with filtering
+        assertRows(execute("SELECT * FROM %s WHERE a = ? AND b > ? PER 
PARTITION LIMIT ? ALLOW FILTERING", 2, 0, 2),
+                   row(2, 1, 1),
+                   row(2, 2, 2));
+
+        assertRows(execute("SELECT * FROM %s WHERE a = ? AND b > ? ORDER BY b 
DESC PER PARTITION LIMIT ? ALLOW FILTERING", 2, 2, 2),
+                   row(2, 4, 4),
+                   row(2, 3, 3));
+
+        assertInvalidMessage("PER PARTITION LIMIT is not allowed with SELECT 
DISTINCT queries",
+                             "SELECT DISTINCT a FROM %s PER PARTITION LIMIT 
?", 3);
+        assertInvalidMessage("PER PARTITION LIMIT is not allowed with SELECT 
DISTINCT queries",
+                             "SELECT DISTINCT a FROM %s PER PARTITION LIMIT ? 
LIMIT ?", 3, 4);
+        assertInvalidMessage("PER PARTITION LIMIT is not allowed with 
aggregate queries.",
+                             "SELECT COUNT(*) FROM %s PER PARTITION LIMIT ?", 
3);
+    }
 }

http://git-wip-us.apache.org/repos/asf/cassandra/blob/9a0eb9a3/test/unit/org/apache/cassandra/cql3/validation/operations/SelectTest.java
----------------------------------------------------------------------
diff --git 
a/test/unit/org/apache/cassandra/cql3/validation/operations/SelectTest.java 
b/test/unit/org/apache/cassandra/cql3/validation/operations/SelectTest.java
index be62f6c..08c2732 100644
--- a/test/unit/org/apache/cassandra/cql3/validation/operations/SelectTest.java
+++ b/test/unit/org/apache/cassandra/cql3/validation/operations/SelectTest.java
@@ -2354,96 +2354,4 @@ public class SelectTest extends CQLTester
                              unset());
     }
 
-    @Test
-    public void testPerPartitionLimit() throws Throwable
-    {
-        perPartitionLimitTest(false);
-    }
-
-    @Test
-    public void testPerPartitionLimitWithCompactStorage() throws Throwable
-    {
-        perPartitionLimitTest(true);
-    }
-
-    private void perPartitionLimitTest(boolean withCompactStorage) throws 
Throwable
-    {
-        String query = "CREATE TABLE %s (a int, b int, c int, PRIMARY KEY (a, 
b))";
-
-        if (withCompactStorage)
-            createTable(query + " WITH COMPACT STORAGE");
-        else
-            createTable(query);
-
-        for (int i = 0; i < 5; i++)
-        {
-            for (int j = 0; j < 5; j++)
-            {
-                execute("INSERT INTO %s (a, b, c) VALUES (?, ?, ?)", i, j, j);
-            }
-        }
-
-        assertInvalidMessage("LIMIT must be strictly positive",
-                             "SELECT * FROM %s PER PARTITION LIMIT ?", 0);
-        assertInvalidMessage("LIMIT must be strictly positive",
-                             "SELECT * FROM %s PER PARTITION LIMIT ?", -1);
-
-        assertRowsIgnoringOrder(execute("SELECT * FROM %s PER PARTITION LIMIT 
?", 2),
-                                row(0, 0, 0),
-                                row(0, 1, 1),
-                                row(1, 0, 0),
-                                row(1, 1, 1),
-                                row(2, 0, 0),
-                                row(2, 1, 1),
-                                row(3, 0, 0),
-                                row(3, 1, 1),
-                                row(4, 0, 0),
-                                row(4, 1, 1));
-
-
-        // Combined Per Partition and "global" limit
-        assertRowCount(execute("SELECT * FROM %s PER PARTITION LIMIT ? LIMIT 
?", 2, 6),
-                       6);
-
-        // odd amount of results
-        assertRowCount(execute("SELECT * FROM %s PER PARTITION LIMIT ? LIMIT 
?", 2, 5),
-                       5);
-
-        // IN query
-        assertRowsIgnoringOrder(execute("SELECT * FROM %s WHERE a IN (2,3) PER 
PARTITION LIMIT ?", 2),
-                                row(2, 0, 0),
-                                row(2, 1, 1),
-                                row(3, 0, 0),
-                                row(3, 1, 1));
-
-        assertRowCount(execute("SELECT * FROM %s WHERE a IN (2,3) PER 
PARTITION LIMIT ? LIMIT 3", 2), 3);
-        assertRowCount(execute("SELECT * FROM %s WHERE a IN (1,2,3) PER 
PARTITION LIMIT ? LIMIT 3", 2), 3);
-
-
-        // with restricted partition key
-        assertRows(execute("SELECT * FROM %s WHERE a = ? PER PARTITION LIMIT 
?", 2, 3),
-                   row(2, 0, 0),
-                   row(2, 1, 1),
-                   row(2, 2, 2));
-
-        // with ordering
-        assertRows(execute("SELECT * FROM %s WHERE a = ? ORDER BY b DESC PER 
PARTITION LIMIT ?", 2, 3),
-                   row(2, 4, 4),
-                   row(2, 3, 3),
-                   row(2, 2, 2));
-
-        // with filtering
-        assertRows(execute("SELECT * FROM %s WHERE a = ? AND b > ? PER 
PARTITION LIMIT ? ALLOW FILTERING", 2, 0, 2),
-                   row(2, 1, 1),
-                   row(2, 2, 2));
-
-        assertRows(execute("SELECT * FROM %s WHERE a = ? AND b > ? ORDER BY b 
DESC PER PARTITION LIMIT ? ALLOW FILTERING", 2, 2, 2),
-                   row(2, 4, 4),
-                   row(2, 3, 3));
-
-        assertInvalidMessage("PER PARTITION LIMIT is not allowed with SELECT 
DISTINCT queries",
-                             "SELECT DISTINCT a FROM %s PER PARTITION LIMIT 
?", 3);
-        assertInvalidMessage("PER PARTITION LIMIT is not allowed with SELECT 
DISTINCT queries",
-                             "SELECT DISTINCT a FROM %s PER PARTITION LIMIT ? 
LIMIT ?", 3, 4);
-    }
 }
\ No newline at end of file

Reply via email to