diff --git a/CHANGES.txt b/CHANGES.txt index fc37d79170..faf4e311c1 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) diff --git a/src/java/org/apache/cassandra/cql3/statements/DeleteStatement.java b/src/java/org/apache/cassandra/cql3/statements/DeleteStatement.java index c7d32b1284..1015a9034a 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 toRead = null; for (Pair 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(); + toRead.add(name.name.key); break; } } - Map rows = needsReading ? readRows(keys, builder, (CompositeType)cfDef.cfm.comparator, local, cl) : null; + Map rows = toRead != null ? readRows(keys, builder, toRead, (CompositeType)cfDef.cfm.comparator, local, cl) : null; Collection rowMutations = new ArrayList(keys.size()); UpdateParameters params = new UpdateParameters(variables, getTimestamp(clientState), -1); diff --git a/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java b/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java index 6e92ec2340..c207245e4d 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 readRows(List keys, ColumnNameBuilder builder, CompositeType composite, boolean local, ConsistencyLevel cl) + protected Map readRows(List keys, ColumnNameBuilder builder, List 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 commands = new ArrayList(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 { diff --git a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java index 226d00438a..e9ec7e07a0 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; } } diff --git a/src/java/org/apache/cassandra/cql3/statements/UpdateStatement.java b/src/java/org/apache/cassandra/cql3/statements/UpdateStatement.java index 2065f3243f..2a99b3f85b 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 toRead = null; for (Map.Entry 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(); + toRead.add(name.name.key); break; } } - Map rows = needsReading ? readRows(keys, builder, (CompositeType)cfDef.cfm.comparator, local, cl) : null; + Map rows = toRead != null ? readRows(keys, builder, toRead, (CompositeType)cfDef.cfm.comparator, local, cl) : null; Collection mutations = new LinkedList(); UpdateParameters params = new UpdateParameters(variables, getTimestamp(clientState), getTimeToLive()); @@ -255,7 +255,8 @@ public class UpdateStatement extends ModificationStatement for (Map.Entry 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)); } }