You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by sl...@apache.org on 2014/03/21 19:09:30 UTC
[1/2] git commit: Fix paging with SELECT DISTINCT
Repository: cassandra
Updated Branches:
refs/heads/cassandra-2.1 a7c4659db -> 7c4a88949
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/cassandra-2.1
Commit: c843b6b85cf828fde16d8d8a04411cba515f715e
Parents: 3b708f9
Author: Sylvain Lebresne <sy...@datastax.com>
Authored: Fri Mar 21 19:03:12 2014 +0100
Committer: Sylvain Lebresne <sy...@datastax.com>
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();
[2/2] git commit: Merge branch 'cassandra-2.0' into cassandra-2.1
Posted by sl...@apache.org.
Merge branch 'cassandra-2.0' into cassandra-2.1
Conflicts:
test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/7c4a8894
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/7c4a8894
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/7c4a8894
Branch: refs/heads/cassandra-2.1
Commit: 7c4a889491ee4d3774f2e95c24ed7dbcbb62dcb1
Parents: a7c4659 c843b6b
Author: Sylvain Lebresne <sy...@datastax.com>
Authored: Fri Mar 21 19:09:21 2014 +0100
Committer: Sylvain Lebresne <sy...@datastax.com>
Committed: Fri Mar 21 19:09:21 2014 +0100
----------------------------------------------------------------------
CHANGES.txt | 1 +
.../apache/cassandra/db/ColumnFamilyStore.java | 3 ++-
.../apache/cassandra/db/PagedRangeCommand.java | 24 +++++++++++++++-----
.../cassandra/service/pager/QueryPagers.java | 10 ++++++--
.../service/pager/RangeSliceQueryPager.java | 3 ++-
.../cassandra/db/ColumnFamilyStoreTest.java | 4 ++--
6 files changed, 33 insertions(+), 12 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cassandra/blob/7c4a8894/CHANGES.txt
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cassandra/blob/7c4a8894/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
----------------------------------------------------------------------
diff --cc src/java/org/apache/cassandra/db/ColumnFamilyStore.java
index 845352d,b58329e..1041812
--- a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
+++ b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
@@@ -1961,10 -1667,11 +1961,11 @@@ public class ColumnFamilyStore implemen
*/
public ExtendedFilter makeExtendedFilter(AbstractBounds<RowPosition> keyRange,
SliceQueryFilter columnRange,
- ByteBuffer columnStart,
- ByteBuffer columnStop,
+ Composite columnStart,
+ Composite columnStop,
List<IndexExpression> rowFilter,
int maxResults,
+ boolean countCQL3Rows,
long now)
{
DataRange dataRange = new DataRange.Paging(keyRange, columnRange, columnStart, columnStop, metadata.comparator);
http://git-wip-us.apache.org/repos/asf/cassandra/blob/7c4a8894/src/java/org/apache/cassandra/db/PagedRangeCommand.java
----------------------------------------------------------------------
diff --cc src/java/org/apache/cassandra/db/PagedRangeCommand.java
index 5c8f3ba,d6f3ca1..f2d81b9
--- a/src/java/org/apache/cassandra/db/PagedRangeCommand.java
+++ b/src/java/org/apache/cassandra/db/PagedRangeCommand.java
@@@ -37,24 -37,24 +37,27 @@@ public class PagedRangeCommand extends
{
public static final IVersionedSerializer<PagedRangeCommand> serializer = new Serializer();
- public final ByteBuffer start;
- public final ByteBuffer stop;
+ public final Composite start;
+ public final Composite stop;
public final int limit;
++ private final boolean countCQL3Rows;
public PagedRangeCommand(String keyspace,
String columnFamily,
long timestamp,
AbstractBounds<RowPosition> keyRange,
SliceQueryFilter predicate,
- ByteBuffer start,
- ByteBuffer stop,
+ Composite start,
+ Composite stop,
List<IndexExpression> rowFilter,
-- int limit)
++ int limit,
++ boolean countCQL3Rows)
{
super(keyspace, columnFamily, timestamp, keyRange, predicate, rowFilter);
this.start = start;
this.stop = stop;
this.limit = limit;
++ this.countCQL3Rows = countCQL3Rows;
}
public MessageOut<PagedRangeCommand> createMessage()
@@@ -74,7 -74,7 +77,8 @@@
newStart,
newStop,
rowFilter,
-- limit);
++ limit,
++ countCQL3Rows);
}
public AbstractRangeCommand withUpdatedLimit(int newLimit)
@@@ -87,7 -87,7 +91,8 @@@
start,
stop,
rowFilter,
-- newLimit);
++ newLimit,
++ countCQL3Rows);
}
public int limit()
@@@ -97,7 -97,15 +102,7 @@@
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;
++ return countCQL3Rows;
}
public List<Row> executeLocally()
@@@ -146,6 -152,6 +151,8 @@@
}
out.writeInt(cmd.limit);
++ if (version >= MessagingService.VERSION_21)
++ out.writeBoolean(cmd.countCQL3Rows);
}
public PagedRangeCommand deserialize(DataInput in, int version) throws IOException
@@@ -174,7 -178,7 +181,10 @@@
}
int limit = in.readInt();
-- return new PagedRangeCommand(keyspace, columnFamily, timestamp, keyRange, predicate, start, stop, rowFilter, limit);
++ boolean countCQL3Rows = version >= MessagingService.VERSION_21
++ ? in.readBoolean()
++ : predicate.compositesToGroup >= 0 || predicate.count != 1; // See #6857
++ return new PagedRangeCommand(keyspace, columnFamily, timestamp, keyRange, predicate, start, stop, rowFilter, limit, countCQL3Rows);
}
public long serializedSize(PagedRangeCommand cmd, int version)
@@@ -203,6 -205,6 +213,8 @@@
}
size += TypeSizes.NATIVE.sizeof(cmd.limit);
++ if (version >= MessagingService.VERSION_21)
++ size += TypeSizes.NATIVE.sizeof(cmd.countCQL3Rows);
return size;
}
}
http://git-wip-us.apache.org/repos/asf/cassandra/blob/7c4a8894/src/java/org/apache/cassandra/service/pager/QueryPagers.java
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cassandra/blob/7c4a8894/src/java/org/apache/cassandra/service/pager/RangeSliceQueryPager.java
----------------------------------------------------------------------
diff --cc src/java/org/apache/cassandra/service/pager/RangeSliceQueryPager.java
index 7e79ffe,0df1d25..b03cc69
--- a/src/java/org/apache/cassandra/service/pager/RangeSliceQueryPager.java
+++ b/src/java/org/apache/cassandra/service/pager/RangeSliceQueryPager.java
@@@ -81,7 -80,7 +81,8 @@@ public class RangeSliceQueryPager exten
start,
sf.finish(),
command.rowFilter,
-- pageSize);
++ pageSize,
++ command.countCQL3Rows);
return localQuery
? pageCmd.executeLocally()
http://git-wip-us.apache.org/repos/asf/cassandra/blob/7c4a8894/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java
----------------------------------------------------------------------
diff --cc test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java
index 0fc82ff,bc39ad6..21f715c
--- a/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java
+++ b/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java
@@@ -1183,11 -1194,11 +1183,11 @@@ public class ColumnFamilyStoreTest exte
assertColumnNames(row2, "c0", "c1");
// Paging within bounds
- SliceQueryFilter sf = new SliceQueryFilter(ByteBufferUtil.bytes("c1"),
- ByteBufferUtil.bytes("c2"),
+ SliceQueryFilter sf = new SliceQueryFilter(cellname("c1"),
+ cellname("c2"),
false,
0);
- rows = cfs.getRangeSlice(cfs.makeExtendedFilter(new Bounds<RowPosition>(ka, kc), sf, cellname("c2"), cellname("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()));
++ rows = cfs.getRangeSlice(cfs.makeExtendedFilter(new Bounds<RowPosition>(ka, kc), sf, cellname("c2"), cellname("c1"), null, 2, true, System.currentTimeMillis()));
assert rows.size() == 2 : "Expected 2 rows, got " + toString(rows);
iter = rows.iterator();
row1 = iter.next();
@@@ -1195,7 -1206,7 +1195,7 @@@
assertColumnNames(row1, "c2");
assertColumnNames(row2, "c1");
- rows = cfs.getRangeSlice(cfs.makeExtendedFilter(new Bounds<RowPosition>(kb, kc), sf, cellname("c1"), cellname("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()));
++ rows = cfs.getRangeSlice(cfs.makeExtendedFilter(new Bounds<RowPosition>(kb, kc), sf, cellname("c1"), cellname("c1"), null, 10, true, System.currentTimeMillis()));
assert rows.size() == 2 : "Expected 2 rows, got " + toString(rows);
iter = rows.iterator();
row1 = iter.next();