Repository: cassandra Updated Branches: refs/heads/trunk 66b304e8c -> 02aad29e8
Fix paging with SELECT DISTINCT patch by slebresne; reviewed by thobbs for CASSANDRA-6857 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/c843b6b8 Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/c843b6b8 Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/c843b6b8 Branch: refs/heads/trunk Commit: c843b6b85cf828fde16d8d8a04411cba515f715e Parents: 3b708f9 Author: Sylvain Lebresne <[email protected]> Authored: Fri Mar 21 19:03:12 2014 +0100 Committer: Sylvain Lebresne <[email protected]> Committed: Fri Mar 21 19:03:12 2014 +0100 ---------------------------------------------------------------------- CHANGES.txt | 1 + src/java/org/apache/cassandra/db/ColumnFamilyStore.java | 3 ++- src/java/org/apache/cassandra/db/PagedRangeCommand.java | 12 ++++++++++-- .../org/apache/cassandra/service/pager/QueryPagers.java | 10 ++++++++-- .../org/apache/cassandra/db/ColumnFamilyStoreTest.java | 4 ++-- 5 files changed, 23 insertions(+), 7 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/c843b6b8/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 31fd319..c5f2666 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -25,6 +25,7 @@ * Extend triggers to support CAS updates (CASSANDRA-6882) * Static columns with IF NOT EXISTS don't always work as expected (CASSANDRA-6873) * Add CqlRecordReader to take advantage of native CQL pagination (CASSANDRA-6311) + * Fix paging with SELECT DISTINCT (CASSANDRA-6857) Merged from 1.2: * Add UNLOGGED, COUNTER options to BATCH documentation (CASSANDRA-6816) * add extra SSL cipher suites (CASSANDRA-6613) http://git-wip-us.apache.org/repos/asf/cassandra/blob/c843b6b8/src/java/org/apache/cassandra/db/ColumnFamilyStore.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java index 5c3eb19..b58329e 100644 --- a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java +++ b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java @@ -1671,10 +1671,11 @@ public class ColumnFamilyStore implements ColumnFamilyStoreMBean ByteBuffer columnStop, List<IndexExpression> rowFilter, int maxResults, + boolean countCQL3Rows, long now) { DataRange dataRange = new DataRange.Paging(keyRange, columnRange, columnStart, columnStop, metadata.comparator); - return ExtendedFilter.create(this, dataRange, rowFilter, maxResults, true, now); + return ExtendedFilter.create(this, dataRange, rowFilter, maxResults, countCQL3Rows, now); } public List<Row> getRangeSlice(AbstractBounds<RowPosition> range, http://git-wip-us.apache.org/repos/asf/cassandra/blob/c843b6b8/src/java/org/apache/cassandra/db/PagedRangeCommand.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/PagedRangeCommand.java b/src/java/org/apache/cassandra/db/PagedRangeCommand.java index e152f43..d6f3ca1 100644 --- a/src/java/org/apache/cassandra/db/PagedRangeCommand.java +++ b/src/java/org/apache/cassandra/db/PagedRangeCommand.java @@ -97,14 +97,22 @@ public class PagedRangeCommand extends AbstractRangeCommand public boolean countCQL3Rows() { - return true; + // We only use PagedRangeCommand for CQL3. However, for SELECT DISTINCT, we want to return false here, because + // we just want to pick the first cell of each partition and returning true here would throw off the logic in + // ColumnFamilyStore.filter(). + // What we do know is that for a SELECT DISTINCT the underlying SliceQueryFilter will have a compositesToGroup==-1 + // and a count==1. And while it would be possible for a normal SELECT on a COMPACT table to also have such + // parameters, it's fine returning false since if we do count one cell for each partition, then each partition + // will coincide with exactly one CQL3 row. + SliceQueryFilter filter = (SliceQueryFilter)predicate; + return filter.compositesToGroup >= 0 || filter.count != 1; } public List<Row> executeLocally() { ColumnFamilyStore cfs = Keyspace.open(keyspace).getColumnFamilyStore(columnFamily); - ExtendedFilter exFilter = cfs.makeExtendedFilter(keyRange, (SliceQueryFilter)predicate, start, stop, rowFilter, limit, timestamp); + ExtendedFilter exFilter = cfs.makeExtendedFilter(keyRange, (SliceQueryFilter)predicate, start, stop, rowFilter, limit, countCQL3Rows(), timestamp); if (cfs.indexManager.hasIndexFor(rowFilter)) return cfs.search(exFilter); else http://git-wip-us.apache.org/repos/asf/cassandra/blob/c843b6b8/src/java/org/apache/cassandra/service/pager/QueryPagers.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/service/pager/QueryPagers.java b/src/java/org/apache/cassandra/service/pager/QueryPagers.java index c353536..65112aa 100644 --- a/src/java/org/apache/cassandra/service/pager/QueryPagers.java +++ b/src/java/org/apache/cassandra/service/pager/QueryPagers.java @@ -71,8 +71,14 @@ public class QueryPagers else { assert command instanceof RangeSliceCommand; - // We can never be sure a range slice won't need paging - return true; + RangeSliceCommand rsc = (RangeSliceCommand)command; + // We don't support paging for thrift in general because the way thrift RangeSliceCommand count rows + // independently of cells makes things harder (see RangeSliceQueryPager). The one case where we do + // get a RangeSliceCommand from CQL3 without the countCQL3Rows flag set is for DISTINCT. In that case + // however, the underlying sliceQueryFilter count is 1, so that the RSC limit is still a limit on the + // number of CQL3 rows returned. + assert rsc.countCQL3Rows || (rsc.predicate instanceof SliceQueryFilter && ((SliceQueryFilter)rsc.predicate).count == 1); + return rsc.maxResults > pageSize; } } http://git-wip-us.apache.org/repos/asf/cassandra/blob/c843b6b8/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java b/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java index 5fc006b..bc39ad6 100644 --- a/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java +++ b/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java @@ -1198,7 +1198,7 @@ public class ColumnFamilyStoreTest extends SchemaLoader ByteBufferUtil.bytes("c2"), false, 0); - rows = cfs.getRangeSlice(cfs.makeExtendedFilter(new Bounds<RowPosition>(ka, kc), sf, ByteBufferUtil.bytes("c2"), ByteBufferUtil.bytes("c1"), null, 2, System.currentTimeMillis())); + rows = cfs.getRangeSlice(cfs.makeExtendedFilter(new Bounds<RowPosition>(ka, kc), sf, ByteBufferUtil.bytes("c2"), ByteBufferUtil.bytes("c1"), null, 2, true, System.currentTimeMillis())); assert rows.size() == 2 : "Expected 2 rows, got " + toString(rows); iter = rows.iterator(); row1 = iter.next(); @@ -1206,7 +1206,7 @@ public class ColumnFamilyStoreTest extends SchemaLoader assertColumnNames(row1, "c2"); assertColumnNames(row2, "c1"); - rows = cfs.getRangeSlice(cfs.makeExtendedFilter(new Bounds<RowPosition>(kb, kc), sf, ByteBufferUtil.bytes("c1"), ByteBufferUtil.bytes("c1"), null, 10, System.currentTimeMillis())); + rows = cfs.getRangeSlice(cfs.makeExtendedFilter(new Bounds<RowPosition>(kb, kc), sf, ByteBufferUtil.bytes("c1"), ByteBufferUtil.bytes("c1"), null, 10, true, System.currentTimeMillis())); assert rows.size() == 2 : "Expected 2 rows, got " + toString(rows); iter = rows.iterator(); row1 = iter.next();
