Repository: cassandra Updated Branches: refs/heads/cassandra-2.1 ab1a02cfa -> f7e690d7f
Fix unintended update with conditional statement patch by slebresne; reviewed by iamaleksey for CASSANDRA-6893 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/167380fb Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/167380fb Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/167380fb Branch: refs/heads/cassandra-2.1 Commit: 167380fb0d7fa3fc6dc9879c839419babaea2a16 Parents: b218536 Author: Sylvain Lebresne <[email protected]> Authored: Wed Apr 2 19:49:16 2014 +0200 Committer: Sylvain Lebresne <[email protected]> Committed: Wed Apr 2 19:49:16 2014 +0200 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../apache/cassandra/cql3/ColumnCondition.java | 198 ++++++++++--------- .../cql3/statements/CQL3CasConditions.java | 11 +- 3 files changed, 114 insertions(+), 96 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/167380fb/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 3cc9937..66196d0 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -41,6 +41,7 @@ * Fix clash with CQL column name in thrift validation (CASSANDRA-6892) * Fix error with super columns in mixed 1.2-2.0 clusters (CASSANDRA-6966) * Fix bad skip of sstables on slice query with composite start/finish (CASSANDRA-6825) + * Fix unintended update with conditional statement (CASSANDRA-6893) 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/167380fb/src/java/org/apache/cassandra/cql3/ColumnCondition.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql3/ColumnCondition.java b/src/java/org/apache/cassandra/cql3/ColumnCondition.java index ce44c3b..0ebd4af 100644 --- a/src/java/org/apache/cassandra/cql3/ColumnCondition.java +++ b/src/java/org/apache/cassandra/cql3/ColumnCondition.java @@ -39,8 +39,6 @@ public class ColumnCondition public final CFDefinition.Name column; private final Term value; - private List<ByteBuffer> variables; - private ColumnCondition(CFDefinition.Name column, Term value) { this.column = column; @@ -53,13 +51,6 @@ public class ColumnCondition return new ColumnCondition(column, value); } - // See CQL3CasConditions for why it's convenient to have this - public ColumnCondition attach(List<ByteBuffer> variables) - { - this.variables = variables; - return this; - } - /** * Collects the column specification for the bind variables of this operation. * @@ -71,109 +62,134 @@ public class ColumnCondition value.collectMarkerSpecification(boundNames); } - // Not overriding equals() because we need the variables to have been attached when this is - // called and so having a non standard method name might help avoid mistakes - public boolean equalsTo(ColumnCondition other) throws InvalidRequestException + public ColumnCondition.WithVariables with(List<ByteBuffer> variables) { - return column.equals(other.column) - && value.bindAndGet(variables).equals(other.value.bindAndGet(other.variables)); + return new WithVariables(variables); } - private ColumnNameBuilder copyOrUpdatePrefix(CFMetaData cfm, ColumnNameBuilder rowPrefix) + public class WithVariables { - return column.kind == CFDefinition.Name.Kind.STATIC ? cfm.getStaticColumnNameBuilder() : rowPrefix.copy(); - } + private final List<ByteBuffer> variables; - /** - * Validates whether this condition applies to {@code current}. - */ - public boolean appliesTo(ColumnNameBuilder rowPrefix, ColumnFamily current, long now) throws InvalidRequestException - { - if (column.type instanceof CollectionType) - return collectionAppliesTo((CollectionType)column.type, rowPrefix, current, now); - - ColumnNameBuilder prefix = copyOrUpdatePrefix(current.metadata(), rowPrefix); - ByteBuffer columnName = column.kind == CFDefinition.Name.Kind.VALUE_ALIAS - ? prefix.build() - : prefix.add(column.name.key).build(); - - Column c = current.getColumn(columnName); - ByteBuffer v = value.bindAndGet(variables); - return v == null - ? c == null || !c.isLive(now) - : c != null && c.isLive(now) && c.value().equals(v); - } + private WithVariables(List<ByteBuffer> variables) + { + this.variables = variables; + } - private boolean collectionAppliesTo(CollectionType type, ColumnNameBuilder rowPrefix, ColumnFamily current, final long now) throws InvalidRequestException - { - ColumnNameBuilder collectionPrefix = copyOrUpdatePrefix(current.metadata(), rowPrefix).add(column.name.key); - // We are testing for collection equality, so we need to have the expected values *and* only those. - ColumnSlice[] collectionSlice = new ColumnSlice[]{ new ColumnSlice(collectionPrefix.build(), collectionPrefix.buildAsEndOfRange()) }; - // Filter live columns, this makes things simpler afterwards - Iterator<Column> iter = Iterators.filter(current.iterator(collectionSlice), new Predicate<Column>() + // Not overriding equals() because we need the variables to have been attached when this is + // called and so having a non standard method name might help avoid mistakes + public boolean equalsTo(WithVariables other) throws InvalidRequestException { - public boolean apply(Column c) - { - // we only care about live columns - return c.isLive(now); - } - }); + return column.equals(other.column()) + && value.bindAndGet(variables).equals(other.value().bindAndGet(other.variables)); + } - Term.Terminal v = value.bind(variables); - if (v == null) - return !iter.hasNext(); + private CFDefinition.Name column() + { + return column; + } - switch (type.kind) + private Term value() { - case LIST: return listAppliesTo(current.metadata(), iter, ((Lists.Value)v).elements); - case SET: return setAppliesTo(current.metadata(), iter, ((Sets.Value)v).elements); - case MAP: return mapAppliesTo(current.metadata(), iter, ((Maps.Value)v).map); + return value; } - throw new AssertionError(); - } - private static ByteBuffer collectionKey(CFMetaData cfm, Column c) - { - ByteBuffer[] bbs = ((CompositeType)cfm.comparator).split(c.name()); - return bbs[bbs.length - 1]; - } + private ColumnNameBuilder copyOrUpdatePrefix(CFMetaData cfm, ColumnNameBuilder rowPrefix) + { + return column.kind == CFDefinition.Name.Kind.STATIC ? cfm.getStaticColumnNameBuilder() : rowPrefix.copy(); + } - private boolean listAppliesTo(CFMetaData cfm, Iterator<Column> iter, List<ByteBuffer> elements) - { - for (ByteBuffer e : elements) - if (!iter.hasNext() || iter.next().value().equals(e)) - return false; - // We must not have more elements than expected - return !iter.hasNext(); - } + /** + * Validates whether this condition applies to {@code current}. + */ + public boolean appliesTo(ColumnNameBuilder rowPrefix, ColumnFamily current, long now) throws InvalidRequestException + { + if (column.type instanceof CollectionType) + return collectionAppliesTo((CollectionType)column.type, rowPrefix, current, now); + + ColumnNameBuilder prefix = copyOrUpdatePrefix(current.metadata(), rowPrefix); + ByteBuffer columnName = column.kind == CFDefinition.Name.Kind.VALUE_ALIAS + ? prefix.build() + : prefix.add(column.name.key).build(); + + Column c = current.getColumn(columnName); + ByteBuffer v = value.bindAndGet(variables); + return v == null + ? c == null || !c.isLive(now) + : c != null && c.isLive(now) && column.type.compare(c.value(), v) == 0; + } - private boolean setAppliesTo(CFMetaData cfm, Iterator<Column> iter, Set<ByteBuffer> elements) - { - Set<ByteBuffer> remaining = new HashSet<>(elements); - while (iter.hasNext()) + private boolean collectionAppliesTo(CollectionType type, ColumnNameBuilder rowPrefix, ColumnFamily current, final long now) throws InvalidRequestException { - if (remaining.isEmpty()) - return false; + ColumnNameBuilder collectionPrefix = copyOrUpdatePrefix(current.metadata(), rowPrefix).add(column.name.key); + // We are testing for collection equality, so we need to have the expected values *and* only those. + ColumnSlice[] collectionSlice = new ColumnSlice[]{ new ColumnSlice(collectionPrefix.build(), collectionPrefix.buildAsEndOfRange()) }; + // Filter live columns, this makes things simpler afterwards + Iterator<Column> iter = Iterators.filter(current.iterator(collectionSlice), new Predicate<Column>() + { + public boolean apply(Column c) + { + // we only care about live columns + return c.isLive(now); + } + }); + + Term.Terminal v = value.bind(variables); + if (v == null) + return !iter.hasNext(); + + switch (type.kind) + { + case LIST: return listAppliesTo(current.metadata(), iter, ((Lists.Value)v).elements); + case SET: return setAppliesTo(current.metadata(), iter, ((Sets.Value)v).elements); + case MAP: return mapAppliesTo(current.metadata(), iter, ((Maps.Value)v).map); + } + throw new AssertionError(); + } - if (!remaining.remove(collectionKey(cfm, iter.next()))) - return false; + private ByteBuffer collectionKey(CFMetaData cfm, Column c) + { + ByteBuffer[] bbs = ((CompositeType)cfm.comparator).split(c.name()); + return bbs[bbs.length - 1]; } - return remaining.isEmpty(); - } - private boolean mapAppliesTo(CFMetaData cfm, Iterator<Column> iter, Map<ByteBuffer, ByteBuffer> elements) - { - Map<ByteBuffer, ByteBuffer> remaining = new HashMap<>(elements); - while (iter.hasNext()) + private boolean listAppliesTo(CFMetaData cfm, Iterator<Column> iter, List<ByteBuffer> elements) { - if (remaining.isEmpty()) - return false; + for (ByteBuffer e : elements) + if (!iter.hasNext() || !iter.next().value().equals(e)) + return false; + // We must not have more elements than expected + return !iter.hasNext(); + } + + private boolean setAppliesTo(CFMetaData cfm, Iterator<Column> iter, Set<ByteBuffer> elements) + { + Set<ByteBuffer> remaining = new HashSet<>(elements); + while (iter.hasNext()) + { + if (remaining.isEmpty()) + return false; - Column c = iter.next(); - if (!remaining.remove(collectionKey(cfm, c)).equals(c.value())) - return false; + if (!remaining.remove(collectionKey(cfm, iter.next()))) + return false; + } + return remaining.isEmpty(); + } + + private boolean mapAppliesTo(CFMetaData cfm, Iterator<Column> iter, Map<ByteBuffer, ByteBuffer> elements) + { + Map<ByteBuffer, ByteBuffer> remaining = new HashMap<>(elements); + while (iter.hasNext()) + { + if (remaining.isEmpty()) + return false; + + Column c = iter.next(); + if (!remaining.remove(collectionKey(cfm, c)).equals(c.value())) + return false; + } + return remaining.isEmpty(); } - return remaining.isEmpty(); } public static class Raw http://git-wip-us.apache.org/repos/asf/cassandra/blob/167380fb/src/java/org/apache/cassandra/cql3/statements/CQL3CasConditions.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql3/statements/CQL3CasConditions.java b/src/java/org/apache/cassandra/cql3/statements/CQL3CasConditions.java index a7ec8d4..7d3c0f7 100644 --- a/src/java/org/apache/cassandra/cql3/statements/CQL3CasConditions.java +++ b/src/java/org/apache/cassandra/cql3/statements/CQL3CasConditions.java @@ -166,7 +166,7 @@ public class CQL3CasConditions implements CASConditions private static class ColumnsConditions extends RowCondition { - private final Map<ColumnIdentifier, ColumnCondition> conditions = new HashMap<>(); + private final Map<ColumnIdentifier, ColumnCondition.WithVariables> conditions = new HashMap<>(); private ColumnsConditions(ColumnNameBuilder rowPrefix, long now) { @@ -178,10 +178,11 @@ public class CQL3CasConditions implements CASConditions for (ColumnCondition condition : conds) { // We will need the variables in appliesTo but with protocol batches, each condition in this object can have a - // different list of variables. So attach them to the condition directly, it's not particulary elegant but its simpler - ColumnCondition previous = conditions.put(condition.column.name, condition.attach(variables)); + // different list of variables. + ColumnCondition.WithVariables current = condition.with(variables); + ColumnCondition.WithVariables previous = conditions.put(condition.column.name, current); // If 2 conditions are actually equal, let it slide - if (previous != null && !previous.equalsTo(condition)) + if (previous != null && !previous.equalsTo(current)) throw new InvalidRequestException("Duplicate and incompatible conditions for column " + condition.column.name); } } @@ -191,7 +192,7 @@ public class CQL3CasConditions implements CASConditions if (current == null) return conditions.isEmpty(); - for (ColumnCondition condition : conditions.values()) + for (ColumnCondition.WithVariables condition : conditions.values()) if (!condition.appliesTo(rowPrefix, current, now)) return false; return true;
