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 <[email protected]> Authored: Wed Jan 16 17:58:42 2013 +0100 Committer: Sylvain Lebresne <[email protected]> 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);
