Updated Branches: refs/heads/trunk 8c99a3769 -> 3b8bff78d
Fix assertion error in cql3 select patch by slebresne; reviewed by jbellis for CASSANDRA-4783 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/3b8bff78 Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/3b8bff78 Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/3b8bff78 Branch: refs/heads/trunk Commit: 3b8bff78d40804b37fa0658b0a8e4342dd98366f Parents: 8c99a37 Author: Sylvain Lebresne <[email protected]> Authored: Mon Oct 15 10:17:49 2012 +0200 Committer: Sylvain Lebresne <[email protected]> Committed: Tue Oct 23 10:30:48 2012 +0200 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../cassandra/cql3/statements/DeleteStatement.java | 8 ++++-- .../cql3/statements/ModificationStatement.java | 19 +++++++++----- .../cassandra/cql3/statements/SelectStatement.java | 4 +- .../cassandra/cql3/statements/UpdateStatement.java | 15 ++++++----- 5 files changed, 28 insertions(+), 19 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/3b8bff78/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index fc37d79..faf4e31 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -34,6 +34,7 @@ * Composite indexes may miss results (CASSANDRA-4796) * Move consistency level to the protocol level (CASSANDRA-4734, 4824) * Fix Subcolumn slice ends not respected (CASSANDRA-4826) + * Fix Assertion error in cql3 select (CASSANDRA-4783) Merged from 1.1: * fix indexing empty column values (CASSANDRA-4832) * allow JdbcDate to compose null Date objects (CASSANDRA-4830) http://git-wip-us.apache.org/repos/asf/cassandra/blob/3b8bff78/src/java/org/apache/cassandra/cql3/statements/DeleteStatement.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql3/statements/DeleteStatement.java b/src/java/org/apache/cassandra/cql3/statements/DeleteStatement.java index c7d32b1..1015a90 100644 --- a/src/java/org/apache/cassandra/cql3/statements/DeleteStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/DeleteStatement.java @@ -86,7 +86,7 @@ public class DeleteStatement extends ModificationStatement throw new InvalidRequestException(String.format("Missing mandatory PRIMARY KEY part %s since %s specified", firstEmpty, toRemove.iterator().next().left)); // Lists DISCARD operation incurs a read. Do that now. - boolean needsReading = false; + List<ByteBuffer> toRead = null; for (Pair<CFDefinition.Name, Term> p : toRemove) { CFDefinition.Name name = p.left; @@ -94,12 +94,14 @@ public class DeleteStatement extends ModificationStatement if ((name.type instanceof ListType) && value != null) { - needsReading = true; + if (toRead == null) + toRead = new ArrayList<ByteBuffer>(); + toRead.add(name.name.key); break; } } - Map<ByteBuffer, ColumnGroupMap> rows = needsReading ? readRows(keys, builder, (CompositeType)cfDef.cfm.comparator, local, cl) : null; + Map<ByteBuffer, ColumnGroupMap> rows = toRead != null ? readRows(keys, builder, toRead, (CompositeType)cfDef.cfm.comparator, local, cl) : null; Collection<RowMutation> rowMutations = new ArrayList<RowMutation>(keys.size()); UpdateParameters params = new UpdateParameters(variables, getTimestamp(clientState), -1); http://git-wip-us.apache.org/repos/asf/cassandra/blob/3b8bff78/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java b/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java index 6e92ec2..c207245 100644 --- a/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java @@ -29,7 +29,9 @@ import org.apache.cassandra.cql3.*; import org.apache.cassandra.transport.messages.ResultMessage; import org.apache.cassandra.db.*; +import org.apache.cassandra.db.filter.ColumnSlice; import org.apache.cassandra.db.filter.QueryPath; +import org.apache.cassandra.db.filter.SliceQueryFilter; import org.apache.cassandra.db.marshal.CompositeType; import org.apache.cassandra.exceptions.*; import org.apache.cassandra.db.IMutation; @@ -140,7 +142,7 @@ public abstract class ModificationStatement extends CFStatement implements CQLSt return timeToLive; } - protected Map<ByteBuffer, ColumnGroupMap> readRows(List<ByteBuffer> keys, ColumnNameBuilder builder, CompositeType composite, boolean local, ConsistencyLevel cl) + protected Map<ByteBuffer, ColumnGroupMap> readRows(List<ByteBuffer> keys, ColumnNameBuilder builder, List<ByteBuffer> toRead, CompositeType composite, boolean local, ConsistencyLevel cl) throws RequestExecutionException, RequestValidationException { try @@ -152,17 +154,20 @@ public abstract class ModificationStatement extends CFStatement implements CQLSt throw new InvalidRequestException(String.format("Write operation require a read but consistency %s is not supported on reads", cl)); } + ColumnSlice[] slices = new ColumnSlice[toRead.size()]; + for (int i = 0; i < toRead.size(); i++) + { + ByteBuffer start = builder.copy().add(toRead.get(i)).build(); + ByteBuffer finish = builder.copy().add(toRead.get(i)).buildAsEndOfRange(); + slices[i] = new ColumnSlice(start, finish); + } + List<ReadCommand> commands = new ArrayList<ReadCommand>(keys.size()); for (ByteBuffer key : keys) - { commands.add(new SliceFromReadCommand(keyspace(), key, new QueryPath(columnFamily()), - builder.copy().build(), - builder.copy().buildAsEndOfRange(), - false, - Integer.MAX_VALUE)); - } + new SliceQueryFilter(slices, false, Integer.MAX_VALUE))); try { http://git-wip-us.apache.org/repos/asf/cassandra/blob/3b8bff78/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 226d004..e9ec7e0 100644 --- a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java @@ -1075,7 +1075,7 @@ public class SelectStatement implements CQLStatement } // We only support IN for the last name and for compact storage so far // TODO: #3885 allows us to extend to non compact as well, but that remains to be done - else if (cfDef.isCompact && restriction.eqValues.size() > 1 && i != stmt.columnRestrictions.length - 1) + else if (restriction.eqValues.size() > 1 && (!cfDef.isCompact || i != stmt.columnRestrictions.length - 1)) { throw new InvalidRequestException(String.format("PRIMARY KEY part %s cannot be restricted by IN relation", cname)); } @@ -1124,7 +1124,7 @@ public class SelectStatement implements CQLStatement { // We only support IN for the last name so far if (i != stmt.keyRestrictions.length - 1) - throw new InvalidRequestException(String.format("partition KEY part %s cannot be restricted by IN relation (only the last part of the partition key can)", cname)); + throw new InvalidRequestException(String.format("Partition KEY part %s cannot be restricted by IN relation (only the last part of the partition key can)", cname)); stmt.keyIsInRelation = true; } } http://git-wip-us.apache.org/repos/asf/cassandra/blob/3b8bff78/src/java/org/apache/cassandra/cql3/statements/UpdateStatement.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql3/statements/UpdateStatement.java b/src/java/org/apache/cassandra/cql3/statements/UpdateStatement.java index 2065f32..2a99b3f 100644 --- a/src/java/org/apache/cassandra/cql3/statements/UpdateStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/UpdateStatement.java @@ -113,10 +113,8 @@ public class UpdateStatement extends ModificationStatement ColumnNameBuilder builder = cfDef.getColumnNameBuilder(); buildColumnNames(cfDef, processedKeys, builder, variables, true); - // Lists SET operation incurs a read. Do that now. Note that currently, - // if there is at least one list, we just read the whole "row" (in the CQL sense of - // row) to simplify. Once #3885 is in, we can improve. - boolean needsReading = false; + // Lists SET operation incurs a read. + List<ByteBuffer> toRead = null; for (Map.Entry<CFDefinition.Name, Operation> entry : processedColumns.entries()) { CFDefinition.Name name = entry.getKey(); @@ -127,12 +125,14 @@ public class UpdateStatement extends ModificationStatement if (value.requiresRead()) { - needsReading = true; + if (toRead == null) + toRead = new ArrayList<ByteBuffer>(); + toRead.add(name.name.key); break; } } - Map<ByteBuffer, ColumnGroupMap> rows = needsReading ? readRows(keys, builder, (CompositeType)cfDef.cfm.comparator, local, cl) : null; + Map<ByteBuffer, ColumnGroupMap> rows = toRead != null ? readRows(keys, builder, toRead, (CompositeType)cfDef.cfm.comparator, local, cl) : null; Collection<IMutation> mutations = new LinkedList<IMutation>(); UpdateParameters params = new UpdateParameters(variables, getTimestamp(clientState), getTimeToLive()); @@ -255,7 +255,8 @@ public class UpdateStatement extends ModificationStatement for (Map.Entry<CFDefinition.Name, Operation> entry : processedColumns.entries()) { CFDefinition.Name name = entry.getKey(); - hasCounterColumn |= addToMutation(cf, builder.copy().add(name.name.key), name, entry.getValue(), params, group == null ? null : group.getCollection(name.name.key)); + Operation op = entry.getValue(); + hasCounterColumn |= addToMutation(cf, builder.copy().add(name.name.key), name, op, params, group == null || !op.requiresRead() ? null : group.getCollection(name.name.key)); } }
