Merge pull request #13 from apigee/DPS-1108
Fixes bug with too small page size when only intersecting 2 terms
diff --git a/stack/core/src/main/java/org/apache/usergrid/persistence/cassandra/QueryProcessor.java b/stack/core/src/main/java/org/apache/usergrid/persistence/cassandra/QueryProcessor.java
index 459926e..9ccf240 100644
--- a/stack/core/src/main/java/org/apache/usergrid/persistence/cassandra/QueryProcessor.java
+++ b/stack/core/src/main/java/org/apache/usergrid/persistence/cassandra/QueryProcessor.java
@@ -96,7 +96,7 @@
private int size;
private Query query;
- private int pageSizeHint;
+ private int sliceCount;
public QueryProcessor( Query query, CollectionInfo collectionInfo, EntityManager em,
@@ -131,7 +131,8 @@
private void process() throws PersistenceException {
- int opCount = 0;
+
+ sliceCount = 0;
// no operand. Check for sorts
if ( rootOperand != null ) {
@@ -143,16 +144,16 @@
rootNode = visitor.getRootNode();
- opCount = visitor.getSliceCount();
+ sliceCount = rootNode.getCount();
}
// see if we have sorts, if so, we can add them all as a single node at
// the root
if ( sorts.size() > 0 ) {
- OrderByNode order = generateSorts( opCount );
+ OrderByNode order = generateSorts( sliceCount );
- opCount += order.getFirstPredicate().getAllSlices().size();
+ sliceCount += order.getCount();
rootNode = order;
}
@@ -204,13 +205,6 @@
rootNode = allNode;
}
}
-
- if ( opCount > 1 ) {
- pageSizeHint = PAGE_SIZE;
- }
- else {
- pageSizeHint = Math.min( size, PAGE_SIZE );
- }
}
@@ -320,7 +314,7 @@
// stack for nodes that will be used to construct the tree and create
// objects
- private CountingStack<QueryNode> nodes = new CountingStack<QueryNode>();
+ private Stack<QueryNode> nodes = new Stack<QueryNode>();
private int contextCount = -1;
@@ -622,43 +616,9 @@
}
}
-
- public int getSliceCount() {
- return nodes.getSliceCount();
- }
}
- private static class CountingStack<T> extends Stack<T> {
-
- private int count = 0;
-
- /**
- *
- */
- private static final long serialVersionUID = 1L;
-
-
- @Override
- public T push( final T entry ) {
- if ( entry instanceof SliceNode ) {
- //by definition a slicenode has a size. Some have nothing to indicate full range scans, but they still count
- //as an operand
- count += Math.max(( ( SliceNode ) entry ).getAllSlices().size(), 1);
- }
- //not node creates a subraction, this needs to increase the count
- else if ( entry instanceof NotNode ) {
- count ++;
- }
-
- return super.push( entry ); //To change body of overridden methods use File | Settings | File Templates.
- }
-
-
- public int getSliceCount() {
- return count;
- }
- }
/**
@@ -670,11 +630,14 @@
* It is crucial that the root iterator only needs the result set size per page
* otherwise our cursor logic will fail when passing cursor data to the leaf nodes
*******/
- if ( node == rootNode ) {
+
+ //if it's a root node, and there's only 1 slice to check in the entire tree, then just select what we need
+ //so we short circuit on range scans faster. otherwise it's more efficient to make less trips with candidates we discard from cassandra
+ if ( node == rootNode && sliceCount == 1 ) {
return size;
}
- return pageSizeHint;
+ return PAGE_SIZE;
}
diff --git a/stack/core/src/main/java/org/apache/usergrid/persistence/query/ir/AllNode.java b/stack/core/src/main/java/org/apache/usergrid/persistence/query/ir/AllNode.java
index ffc7ef2..d870e49 100644
--- a/stack/core/src/main/java/org/apache/usergrid/persistence/query/ir/AllNode.java
+++ b/stack/core/src/main/java/org/apache/usergrid/persistence/query/ir/AllNode.java
@@ -52,6 +52,12 @@
@Override
+ public int getCount() {
+ return 1;
+ }
+
+
+ @Override
public String toString() {
return "AllNode";
}
diff --git a/stack/core/src/main/java/org/apache/usergrid/persistence/query/ir/BooleanNode.java b/stack/core/src/main/java/org/apache/usergrid/persistence/query/ir/BooleanNode.java
index a6be2e8..ac3e42d 100644
--- a/stack/core/src/main/java/org/apache/usergrid/persistence/query/ir/BooleanNode.java
+++ b/stack/core/src/main/java/org/apache/usergrid/persistence/query/ir/BooleanNode.java
@@ -43,6 +43,12 @@
@Override
+ public int getCount() {
+ return left.getCount()+ right.getCount();
+ }
+
+
+ @Override
public String toString() {
return "BooleanNode [left=" + left + ", right=" + right + "]";
}
diff --git a/stack/core/src/main/java/org/apache/usergrid/persistence/query/ir/EmailIdentifierNode.java b/stack/core/src/main/java/org/apache/usergrid/persistence/query/ir/EmailIdentifierNode.java
index 9266518..92dffee 100644
--- a/stack/core/src/main/java/org/apache/usergrid/persistence/query/ir/EmailIdentifierNode.java
+++ b/stack/core/src/main/java/org/apache/usergrid/persistence/query/ir/EmailIdentifierNode.java
@@ -41,6 +41,12 @@
}
+ @Override
+ public int getCount() {
+ return 1;
+ }
+
+
public Identifier getIdentifier() {
return identifier;
}
diff --git a/stack/core/src/main/java/org/apache/usergrid/persistence/query/ir/NameIdentifierNode.java b/stack/core/src/main/java/org/apache/usergrid/persistence/query/ir/NameIdentifierNode.java
index 626be8c..2bd1b05 100644
--- a/stack/core/src/main/java/org/apache/usergrid/persistence/query/ir/NameIdentifierNode.java
+++ b/stack/core/src/main/java/org/apache/usergrid/persistence/query/ir/NameIdentifierNode.java
@@ -38,6 +38,12 @@
}
+ @Override
+ public int getCount() {
+ return 1;
+ }
+
+
public String getName() {
return name;
}
diff --git a/stack/core/src/main/java/org/apache/usergrid/persistence/query/ir/NotNode.java b/stack/core/src/main/java/org/apache/usergrid/persistence/query/ir/NotNode.java
index aee8886..06b8208 100644
--- a/stack/core/src/main/java/org/apache/usergrid/persistence/query/ir/NotNode.java
+++ b/stack/core/src/main/java/org/apache/usergrid/persistence/query/ir/NotNode.java
@@ -57,6 +57,12 @@
@Override
+ public int getCount() {
+ return subtractNode.getCount() + keepNode.getCount();
+ }
+
+
+ @Override
public String toString() {
return "NotNode [child=" + subtractNode + "]";
}
diff --git a/stack/core/src/main/java/org/apache/usergrid/persistence/query/ir/OrderByNode.java b/stack/core/src/main/java/org/apache/usergrid/persistence/query/ir/OrderByNode.java
index 655131e..ae3eb9e 100644
--- a/stack/core/src/main/java/org/apache/usergrid/persistence/query/ir/OrderByNode.java
+++ b/stack/core/src/main/java/org/apache/usergrid/persistence/query/ir/OrderByNode.java
@@ -83,9 +83,15 @@
}
+ @Override
+ public int getCount() {
+ return firstPredicate.getCount() + secondarySorts.size();
+ }
+
+
/* (non-Javadoc)
- * @see java.lang.Object#toString()
- */
+ * @see java.lang.Object#toString()
+ */
@Override
public String toString() {
return "OrderByNode [sorts=" + secondarySorts + "]";
diff --git a/stack/core/src/main/java/org/apache/usergrid/persistence/query/ir/QueryNode.java b/stack/core/src/main/java/org/apache/usergrid/persistence/query/ir/QueryNode.java
index 478bef9..5d3f0aa 100644
--- a/stack/core/src/main/java/org/apache/usergrid/persistence/query/ir/QueryNode.java
+++ b/stack/core/src/main/java/org/apache/usergrid/persistence/query/ir/QueryNode.java
@@ -26,4 +26,10 @@
/** Visit this node */
public abstract void visit( NodeVisitor visitor ) throws Exception;
+
+
+ /**
+ * Get the count of the total number of slices in our tree from this node and it's children
+ */
+ public abstract int getCount();
}
diff --git a/stack/core/src/main/java/org/apache/usergrid/persistence/query/ir/SliceNode.java b/stack/core/src/main/java/org/apache/usergrid/persistence/query/ir/SliceNode.java
index da6d234..046b517 100644
--- a/stack/core/src/main/java/org/apache/usergrid/persistence/query/ir/SliceNode.java
+++ b/stack/core/src/main/java/org/apache/usergrid/persistence/query/ir/SliceNode.java
@@ -59,6 +59,12 @@
}
+ @Override
+ public int getCount() {
+ return pairs.size();
+ }
+
+
/**
* Set the start value. If the range pair doesn't exist, it's created
*
diff --git a/stack/core/src/main/java/org/apache/usergrid/persistence/query/ir/UuidIdentifierNode.java b/stack/core/src/main/java/org/apache/usergrid/persistence/query/ir/UuidIdentifierNode.java
index 56cc268..42e2c08 100644
--- a/stack/core/src/main/java/org/apache/usergrid/persistence/query/ir/UuidIdentifierNode.java
+++ b/stack/core/src/main/java/org/apache/usergrid/persistence/query/ir/UuidIdentifierNode.java
@@ -42,6 +42,12 @@
}
+ @Override
+ public int getCount() {
+ return 1;
+ }
+
+
public UUID getUuid() {
return uuid;
}
diff --git a/stack/core/src/main/java/org/apache/usergrid/persistence/query/ir/WithinNode.java b/stack/core/src/main/java/org/apache/usergrid/persistence/query/ir/WithinNode.java
index d207f82..6551aee 100644
--- a/stack/core/src/main/java/org/apache/usergrid/persistence/query/ir/WithinNode.java
+++ b/stack/core/src/main/java/org/apache/usergrid/persistence/query/ir/WithinNode.java
@@ -90,6 +90,12 @@
@Override
+ public int getCount() {
+ return 1;
+ }
+
+
+ @Override
public String toString() {
return "WithinNode [propertyName=" + propertyName + ", distance=" + distance + ", lattitude=" + lattitude
+ ", longitude=" + longitude + "]";
diff --git a/stack/core/src/test/java/org/apache/usergrid/persistence/cassandra/QueryProcessorTest.java b/stack/core/src/test/java/org/apache/usergrid/persistence/cassandra/QueryProcessorTest.java
index 683e1c0..c1fe7ee 100644
--- a/stack/core/src/test/java/org/apache/usergrid/persistence/cassandra/QueryProcessorTest.java
+++ b/stack/core/src/test/java/org/apache/usergrid/persistence/cassandra/QueryProcessorTest.java
@@ -24,12 +24,15 @@
import org.antlr.runtime.ANTLRStringStream;
import org.antlr.runtime.TokenRewriteStream;
import org.junit.Test;
+
import org.apache.usergrid.cassandra.Concurrent;
import org.apache.usergrid.persistence.Query;
import org.apache.usergrid.persistence.exceptions.PersistenceException;
import org.apache.usergrid.persistence.query.ir.AndNode;
import org.apache.usergrid.persistence.query.ir.NotNode;
import org.apache.usergrid.persistence.query.ir.OrNode;
+import org.apache.usergrid.persistence.query.ir.OrderByNode;
+import org.apache.usergrid.persistence.query.ir.QueryNode;
import org.apache.usergrid.persistence.query.ir.QuerySlice;
import org.apache.usergrid.persistence.query.ir.SliceNode;
import org.apache.usergrid.persistence.query.ir.WithinNode;
@@ -42,7 +45,9 @@
import static org.junit.Assert.assertTrue;
-/** @author tnine */
+/**
+ * @author tnine
+ */
@Concurrent()
public class QueryProcessorTest {
@@ -386,7 +391,9 @@
}
- /** Tests that when properties are not siblings, they are properly assigned to a SliceNode */
+ /**
+ * Tests that when properties are not siblings, they are properly assigned to a SliceNode
+ */
@Test
public void nestedCompression() throws Exception {
String queryString =
@@ -453,7 +460,9 @@
}
- /** Tests that when there are multiple or with and clauses, the tree is constructed correctly */
+ /**
+ * Tests that when there are multiple or with and clauses, the tree is constructed correctly
+ */
@Test
public void nestedOrCompression() throws Exception {
String queryString =
@@ -530,7 +539,9 @@
}
- /** Tests that when NOT is not the root operand the tree has a different root */
+ /**
+ * Tests that when NOT is not the root operand the tree has a different root
+ */
@Test
public void andNot() throws Exception {
String queryString = "select * where a > 1 and not b = 2";
@@ -575,7 +586,9 @@
}
- /** Tests that when NOT is the root operand, a full scan range is performed. */
+ /**
+ * Tests that when NOT is the root operand, a full scan range is performed.
+ */
@Test
public void notRootOperand() throws Exception {
String queryString = "select * where not b = 2";
@@ -724,4 +737,86 @@
assertEquals( value, slice.getFinish().getValue() );
assertTrue( slice.getFinish().isInclusive() );
}
+
+
+ @Test
+ public void validateHintSizeForOrder() throws Exception {
+ String queryString = "order by name desc";
+
+ ANTLRStringStream in = new ANTLRStringStream( queryString );
+ QueryFilterLexer lexer = new QueryFilterLexer( in );
+ TokenRewriteStream tokens = new TokenRewriteStream( lexer );
+ QueryFilterParser parser = new QueryFilterParser( tokens );
+
+ /**
+ * Test set limit
+ */
+
+ final int limit = 105;
+
+ Query query = parser.ql().query;
+ query.setLimit( limit );
+
+ QueryProcessor processor = new QueryProcessor( query, null, null, null );
+
+ OrderByNode node = ( OrderByNode ) processor.getFirstNode();
+
+ assertEquals( limit, processor.getPageSizeHint( node ) );
+ }
+
+
+ @Test
+ public void validateHintSizeForEquality() throws Exception {
+ String queryString = "select * where X = 'Foo'";
+
+ ANTLRStringStream in = new ANTLRStringStream( queryString );
+ QueryFilterLexer lexer = new QueryFilterLexer( in );
+ TokenRewriteStream tokens = new TokenRewriteStream( lexer );
+ QueryFilterParser parser = new QueryFilterParser( tokens );
+
+ /**
+ * Test set limit
+ */
+
+ final int limit = 105;
+
+ Query query = parser.ql().query;
+ query.setLimit( limit );
+
+ QueryProcessor processor = new QueryProcessor( query, null, null, null );
+
+ SliceNode node = ( SliceNode ) processor.getFirstNode();
+
+ assertEquals( limit, processor.getPageSizeHint( node ) );
+ }
+
+
+ @Test
+ public void validateHintSizeForComplexQueries() throws Exception {
+ // String queryString = "select * where y = 'Foo' AND z = 'Bar'";
+
+ String queryString = "select * where y = 'Foo' AND z = 'Bar'";
+
+ ANTLRStringStream in = new ANTLRStringStream( queryString );
+ QueryFilterLexer lexer = new QueryFilterLexer( in );
+ TokenRewriteStream tokens = new TokenRewriteStream( lexer );
+ QueryFilterParser parser = new QueryFilterParser( tokens );
+
+ /**
+ * Test set limit
+ */
+
+ final int limit = 105;
+
+ Query query = parser.ql().query;
+ query.setLimit( limit );
+
+ QueryProcessor processor = new QueryProcessor( query, null, null, null );
+
+ QueryNode slice = processor.getFirstNode();
+
+ assertEquals( 1000, processor.getPageSizeHint( slice ) );
+
+
+ }
}
