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 2013/01/16 17:59:55 UTC
git commit: Fix bogus sharing of SliceQueryFilter when using
superColumn
Updated Branches:
refs/heads/trunk edd5caa45 -> 449573622
Fix bogus sharing of SliceQueryFilter when using superColumn
patch by slebresne; reviewed by jbellis for CASSANDRA-5123
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/44957362
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/44957362
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/44957362
Branch: refs/heads/trunk
Commit: 4495736227a2f610262a26162ed93d64f321d0e1
Parents: edd5caa
Author: Sylvain Lebresne <sy...@datastax.com>
Authored: Wed Jan 16 17:58:42 2013 +0100
Committer: Sylvain Lebresne <sy...@datastax.com>
Committed: Wed Jan 16 17:58:42 2013 +0100
----------------------------------------------------------------------
CHANGES.txt | 2 +-
.../cassandra/cql3/statements/SelectStatement.java | 33 ++++-----------
.../cassandra/db/filter/IDiskAtomFilter.java | 2 +
.../cassandra/db/filter/NamesQueryFilter.java | 6 +++
.../cassandra/db/filter/SliceQueryFilter.java | 5 ++
.../apache/cassandra/thrift/CassandraServer.java | 4 +-
6 files changed, 26 insertions(+), 26 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cassandra/blob/44957362/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 5cd8fe9..f2c3efb 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -2,7 +2,7 @@
* make index_interval configurable per columnfamily (CASSANDRA-3961)
* add default_tim_to_live (CASSANDRA-3974)
* add memtable_flush_period_in_ms (CASSANDRA-4237)
- * replace supercolumns internally by composites (CASSANDRA-3237)
+ * replace supercolumns internally by composites (CASSANDRA-3237, 5123)
* upgrade thrift to 0.9.0 (CASSANDRA-3719)
1.2.1
http://git-wip-us.apache.org/repos/asf/cassandra/blob/44957362/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 179443c..4888c37 100644
--- a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java
+++ b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java
@@ -201,31 +201,16 @@ public class SelectStatement implements CQLStatement
Collection<ByteBuffer> keys = getKeys(variables);
List<ReadCommand> commands = new ArrayList<ReadCommand>(keys.size());
- // ...a range (slice) of column names
- if (isColumnRange())
- {
- // Note that we use the total limit for every key. This is
- // potentially inefficient, but then again, IN + LIMIT is not a
- // very sensible choice
- for (ByteBuffer key : keys)
- {
- QueryProcessor.validateKey(key);
- // Note that we should not share the slice filter amongst the command, due to SliceQueryFilter not
- // being immutable due to its columnCounter used by the lastCounted() method
- // (this is fairly ugly and we should change that but that's probably not a tiny refactor to do that cleanly)
- commands.add(new SliceFromReadCommand(keyspace(), key, columnFamily(), (SliceQueryFilter)makeFilter(variables)));
- }
- }
- // ...of a list of column names
- else
+ IDiskAtomFilter filter = makeFilter(variables);
+ // Note that we use the total limit for every key, which is potentially inefficient.
+ // However, IN + LIMIT is not a very sensible choice.
+ for (ByteBuffer key : keys)
{
- // ByNames commands can share the filter
- IDiskAtomFilter filter = makeFilter(variables);
- for (ByteBuffer key: keys)
- {
- QueryProcessor.validateKey(key);
- commands.add(new SliceByNamesReadCommand(keyspace(), key, columnFamily(), (NamesQueryFilter)filter));
- }
+ QueryProcessor.validateKey(key);
+ // We should not share the slice filter amongst the commands (hence the cloneShallow), due to
+ // SliceQueryFilter not being immutable due to its columnCounter used by the lastCounted() method
+ // (this is fairly ugly and we should change that but that's probably not a tiny refactor to do that cleanly)
+ commands.add(ReadCommand.create(keyspace(), key, columnFamily(), filter.cloneShallow()));
}
return commands;
}
http://git-wip-us.apache.org/repos/asf/cassandra/blob/44957362/src/java/org/apache/cassandra/db/filter/IDiskAtomFilter.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/filter/IDiskAtomFilter.java b/src/java/org/apache/cassandra/db/filter/IDiskAtomFilter.java
index 5115e95..8ef73c3 100644
--- a/src/java/org/apache/cassandra/db/filter/IDiskAtomFilter.java
+++ b/src/java/org/apache/cassandra/db/filter/IDiskAtomFilter.java
@@ -75,6 +75,8 @@ public interface IDiskAtomFilter
public int getLiveCount(ColumnFamily cf);
+ public IDiskAtomFilter cloneShallow();
+
public static class Serializer implements IVersionedSerializer<IDiskAtomFilter>
{
public static Serializer instance = new Serializer();
http://git-wip-us.apache.org/repos/asf/cassandra/blob/44957362/src/java/org/apache/cassandra/db/filter/NamesQueryFilter.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/filter/NamesQueryFilter.java b/src/java/org/apache/cassandra/db/filter/NamesQueryFilter.java
index 5d0bc49..acbe7f6 100644
--- a/src/java/org/apache/cassandra/db/filter/NamesQueryFilter.java
+++ b/src/java/org/apache/cassandra/db/filter/NamesQueryFilter.java
@@ -66,6 +66,12 @@ public class NamesQueryFilter implements IDiskAtomFilter
this(FBUtilities.singleton(column));
}
+ public NamesQueryFilter cloneShallow()
+ {
+ // NQF is immutable as far as shallow cloning is concerned, so save the allocation.
+ return this;
+ }
+
public NamesQueryFilter withUpdatedColumns(SortedSet<ByteBuffer> newColumns)
{
return new NamesQueryFilter(newColumns, countCQL3Rows);
http://git-wip-us.apache.org/repos/asf/cassandra/blob/44957362/src/java/org/apache/cassandra/db/filter/SliceQueryFilter.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/filter/SliceQueryFilter.java b/src/java/org/apache/cassandra/db/filter/SliceQueryFilter.java
index 25e7053..7213402 100644
--- a/src/java/org/apache/cassandra/db/filter/SliceQueryFilter.java
+++ b/src/java/org/apache/cassandra/db/filter/SliceQueryFilter.java
@@ -81,6 +81,11 @@ public class SliceQueryFilter implements IDiskAtomFilter
this.countMutliplierForCompatibility = countMutliplierForCompatibility;
}
+ public SliceQueryFilter cloneShallow()
+ {
+ return new SliceQueryFilter(slices, reversed, count, compositesToGroup, countMutliplierForCompatibility);
+ }
+
public SliceQueryFilter withUpdatedCount(int newCount)
{
return new SliceQueryFilter(slices, reversed, newCount, compositesToGroup, countMutliplierForCompatibility);
http://git-wip-us.apache.org/repos/asf/cassandra/blob/44957362/src/java/org/apache/cassandra/thrift/CassandraServer.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/thrift/CassandraServer.java b/src/java/org/apache/cassandra/thrift/CassandraServer.java
index e052a35..a00f862 100644
--- a/src/java/org/apache/cassandra/thrift/CassandraServer.java
+++ b/src/java/org/apache/cassandra/thrift/CassandraServer.java
@@ -434,7 +434,9 @@ public class CassandraServer implements Cassandra.Iface
for (ByteBuffer key: keys)
{
ThriftValidation.validateKey(metadata, key);
- commands.add(ReadCommand.create(keyspace, key, column_parent.getColumn_family(), filter));
+ // Note that we should not share a slice filter amongst the command, due to SliceQueryFilter not being immutable
+ // due to its columnCounter used by the lastCounted() method (also see SelectStatement.getSliceCommands)
+ commands.add(ReadCommand.create(keyspace, key, column_parent.getColumn_family(), filter.cloneShallow()));
}
return getSlice(commands, column_parent.isSetSuper_column(), consistencyLevel);