cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From slebre...@apache.org
Subject git commit: Fix unintended update with conditional statement
Date Wed, 02 Apr 2014 17:51:12 GMT
Repository: cassandra
Updated Branches:
  refs/heads/cassandra-2.0 b218536db -> 167380fb0


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.0
Commit: 167380fb0d7fa3fc6dc9879c839419babaea2a16
Parents: b218536
Author: Sylvain Lebresne <sylvain@datastax.com>
Authored: Wed Apr 2 19:49:16 2014 +0200
Committer: Sylvain Lebresne <sylvain@datastax.com>
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;


Mime
View raw message