cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From slebre...@apache.org
Subject [3/3] git commit: Fix preparing updates with CQL3 and collections
Date Thu, 06 Dec 2012 07:42:02 GMT
Fix preparing updates with CQL3 and collections


Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/a90b310e
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/a90b310e
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/a90b310e

Branch: refs/heads/trunk
Commit: a90b310e61495d4d782c25a8debb44798cc00b79
Parents: a41b780
Author: Sylvain Lebresne <sylvain@datastax.com>
Authored: Tue Dec 4 13:30:48 2012 +0100
Committer: Sylvain Lebresne <sylvain@datastax.com>
Committed: Thu Dec 6 08:40:56 2012 +0100

----------------------------------------------------------------------
 CHANGES.txt                                        |    1 +
 src/java/org/apache/cassandra/cql3/Cql.g           |    9 +-
 .../cassandra/cql3/operations/ColumnOperation.java |    7 ++
 .../cassandra/cql3/operations/ListOperation.java   |   61 ++++++++++++++-
 .../cassandra/cql3/operations/MapOperation.java    |   29 +++++++
 .../cassandra/cql3/operations/Operation.java       |    3 +
 .../cql3/operations/PreparedOperation.java         |    6 ++
 .../cassandra/cql3/operations/SetOperation.java    |    8 ++
 .../cassandra/cql3/statements/BatchStatement.java  |    2 +-
 .../cassandra/cql3/statements/DeleteStatement.java |   23 +++++-
 .../cql3/statements/ModificationStatement.java     |    2 +-
 .../cassandra/cql3/statements/UpdateStatement.java |   14 +--
 12 files changed, 143 insertions(+), 22 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/a90b310e/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 14fe46e..6a3187d 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -3,6 +3,7 @@
  * add BF with 0.1 FP to LCS by default (CASSANDRA-5029)
  * Fix preparing insert queries (CASSANDRA-5016)
  * Fix preparing queries with counter increment (CASSANDRA-5022)
+ * Fix preparing updates with collections (CASSANDRA-5017)
 
 
 1.2-beta3

http://git-wip-us.apache.org/repos/asf/cassandra/blob/a90b310e/src/java/org/apache/cassandra/cql3/Cql.g
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/Cql.g b/src/java/org/apache/cassandra/cql3/Cql.g
index 493411b..acd30cc 100644
--- a/src/java/org/apache/cassandra/cql3/Cql.g
+++ b/src/java/org/apache/cassandra/cql3/Cql.g
@@ -644,11 +644,10 @@ termPairWithOperation[List<Pair<ColumnIdentifier, Operation>>
columns]
         )
     | key=cident '[' t=term ']' '=' vv=term
       {
-          Operation setOp = (t.getType() == Term.Type.INTEGER)
-                             ? ListOperation.SetIndex(Arrays.asList(t, vv))
-                             : MapOperation.Put(t, vv);
-
-          columns.add(Pair.<ColumnIdentifier, Operation>create(key, setOp));
+          // This is ambiguous, this can either set a list by index, or be a map put.
+          // So we always return a list setIndex and we'll check later and
+          // backtrack to a map operation if need be.
+          columns.add(Pair.<ColumnIdentifier, Operation>create(key, ListOperation.SetIndex(Arrays.asList(t,
vv))));
       }
     ;
 

http://git-wip-us.apache.org/repos/asf/cassandra/blob/a90b310e/src/java/org/apache/cassandra/cql3/operations/ColumnOperation.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/operations/ColumnOperation.java b/src/java/org/apache/cassandra/cql3/operations/ColumnOperation.java
index bfdec8c..224829f 100644
--- a/src/java/org/apache/cassandra/cql3/operations/ColumnOperation.java
+++ b/src/java/org/apache/cassandra/cql3/operations/ColumnOperation.java
@@ -112,11 +112,18 @@ public class ColumnOperation implements Operation
         cf.addCounter(new QueryPath(cf.metadata().cfName, null, builder.build()), val);
     }
 
+    public void addBoundNames(ColumnSpecification column, ColumnSpecification[] boundNames)
throws InvalidRequestException
+    {
+        if (value.isBindMarker())
+            boundNames[value.bindIndex] = column;
+    }
+
     public List<Term> getValues()
     {
         return Collections.singletonList(value);
     }
 
+
     public boolean requiresRead(AbstractType<?> validator)
     {
         return false;

http://git-wip-us.apache.org/repos/asf/cassandra/blob/a90b310e/src/java/org/apache/cassandra/cql3/operations/ListOperation.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/operations/ListOperation.java b/src/java/org/apache/cassandra/cql3/operations/ListOperation.java
index 25c616d..1e09195 100644
--- a/src/java/org/apache/cassandra/cql3/operations/ListOperation.java
+++ b/src/java/org/apache/cassandra/cql3/operations/ListOperation.java
@@ -24,14 +24,18 @@ import java.util.List;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicReference;
 
+import org.apache.cassandra.cql3.ColumnIdentifier;
 import org.apache.cassandra.cql3.ColumnNameBuilder;
+import org.apache.cassandra.cql3.ColumnSpecification;
 import org.apache.cassandra.cql3.Term;
 import org.apache.cassandra.cql3.UpdateParameters;
 import org.apache.cassandra.db.ColumnFamily;
 import org.apache.cassandra.db.IColumn;
 import org.apache.cassandra.db.marshal.AbstractType;
 import org.apache.cassandra.db.marshal.CollectionType;
+import org.apache.cassandra.db.marshal.Int32Type;
 import org.apache.cassandra.db.marshal.ListType;
+import org.apache.cassandra.db.marshal.MapType;
 import org.apache.cassandra.db.marshal.MarshalException;
 import org.apache.cassandra.exceptions.InvalidRequestException;
 import org.apache.cassandra.utils.Pair;
@@ -97,7 +101,7 @@ public class ListOperation implements Operation
                         UpdateParameters params,
                         List<Pair<ByteBuffer, IColumn>> list) throws InvalidRequestException
     {
-        if (!(validator instanceof ListType))
+        if (!(validator instanceof ListType || (kind == Kind.SET_IDX && validator
instanceof MapType)))
             throw new InvalidRequestException("List operations are only supported on List
typed columns, but " + validator + " given.");
 
         switch (kind)
@@ -107,7 +111,17 @@ public class ListOperation implements Operation
                 doAppend(cf, builder, (CollectionType)validator, params);
                 break;
             case SET_IDX:
-                doSet(cf, builder, params, (CollectionType)validator, list);
+                // Since the parser couldn't disambiguate between a 'list set by idx'
+                // and a 'map put by key', we have to do it now.
+                if (validator instanceof MapType)
+                {
+                    assert values.size() == 2;
+                    MapOperation.Put(values.get(0), values.get(1)).execute(cf, builder, validator,
params, null);
+                }
+                else
+                {
+                    doSet(cf, builder, params, (CollectionType)validator, list);
+                }
                 break;
             case APPEND:
                 doAppend(cf, builder, (CollectionType)validator, params);
@@ -271,6 +285,49 @@ public class ListOperation implements Operation
         cf.addColumn(params.makeTombstone(list.get(idx).right.name()));
     }
 
+    public void addBoundNames(ColumnSpecification column, ColumnSpecification[] boundNames)
throws InvalidRequestException
+    {
+        // Since the parser couldn't disambiguate between a 'list set by idx'
+        // and a 'map put by key', we have to do it now.
+        if (kind == Kind.SET_IDX && (column.type instanceof MapType))
+        {
+            assert values.size() == 2;
+            MapOperation.Put(values.get(0), values.get(1)).addBoundNames(column, boundNames);
+            return;
+        }
+
+        if (!(column.type instanceof ListType))
+            throw new InvalidRequestException(String.format("Invalid operation, %s is not
of list type", column.name));
+
+        ListType lt = (ListType)column.type;
+        if (kind == Kind.SET_IDX)
+        {
+            assert values.size() == 2;
+            Term idx = values.get(0);
+            Term value = values.get(1);
+            if (idx.isBindMarker())
+                boundNames[idx.bindIndex] = indexSpecOf(column);
+            if (value.isBindMarker())
+                boundNames[value.bindIndex] = valueSpecOf(column, lt);
+        }
+        else
+        {
+            for (Term t : values)
+                if (t.isBindMarker())
+                    boundNames[t.bindIndex] = column;
+        }
+    }
+
+    public static ColumnSpecification indexSpecOf(ColumnSpecification column)
+    {
+        return new ColumnSpecification(column.ksName, column.cfName, new ColumnIdentifier("idx("
+ column.name + ")", true), Int32Type.instance);
+    }
+
+    public static ColumnSpecification valueSpecOf(ColumnSpecification column, ListType type)
+    {
+        return new ColumnSpecification(column.ksName, column.cfName, new ColumnIdentifier("value("
+ column.name + ")", true), type.elements);
+    }
+
     public List<Term> getValues()
     {
         return values;

http://git-wip-us.apache.org/repos/asf/cassandra/blob/a90b310e/src/java/org/apache/cassandra/cql3/operations/MapOperation.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/operations/MapOperation.java b/src/java/org/apache/cassandra/cql3/operations/MapOperation.java
index 5f844a7..ddded47 100644
--- a/src/java/org/apache/cassandra/cql3/operations/MapOperation.java
+++ b/src/java/org/apache/cassandra/cql3/operations/MapOperation.java
@@ -23,7 +23,9 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
+import org.apache.cassandra.cql3.ColumnIdentifier;
 import org.apache.cassandra.cql3.ColumnNameBuilder;
+import org.apache.cassandra.cql3.ColumnSpecification;
 import org.apache.cassandra.cql3.Term;
 import org.apache.cassandra.cql3.UpdateParameters;
 import org.apache.cassandra.db.ColumnFamily;
@@ -127,6 +129,33 @@ public class MapOperation implements Operation
         cf.addColumn(params.makeTombstone(name));
     }
 
+    public void addBoundNames(ColumnSpecification column, ColumnSpecification[] boundNames)
throws InvalidRequestException
+    {
+        if (!(column.type instanceof MapType))
+            throw new InvalidRequestException(String.format("Invalid operation, %s is not
of map type", column.name));
+
+        MapType mt = (MapType)column.type;
+        for (Map.Entry<Term, Term> entry : values.entrySet())
+        {
+            Term key = entry.getKey();
+            Term value = entry.getValue();
+            if (key.isBindMarker())
+                boundNames[key.bindIndex] = keySpecOf(column, mt);
+            if (value.isBindMarker())
+                boundNames[value.bindIndex] = valueSpecOf(column, mt);
+        }
+    }
+
+    public static ColumnSpecification keySpecOf(ColumnSpecification column, MapType type)
+    {
+        return new ColumnSpecification(column.ksName, column.cfName, new ColumnIdentifier("key("
+ column.name + ")", true), type.keys);
+    }
+
+    public static ColumnSpecification valueSpecOf(ColumnSpecification column, MapType type)
+    {
+        return new ColumnSpecification(column.ksName, column.cfName, new ColumnIdentifier("value("
+ column.name + ")", true), type.values);
+    }
+
     public List<Term> getValues()
     {
         List<Term> l = new ArrayList<Term>(2 * values.size());

http://git-wip-us.apache.org/repos/asf/cassandra/blob/a90b310e/src/java/org/apache/cassandra/cql3/operations/Operation.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/operations/Operation.java b/src/java/org/apache/cassandra/cql3/operations/Operation.java
index 28caea7..6c30f7c 100644
--- a/src/java/org/apache/cassandra/cql3/operations/Operation.java
+++ b/src/java/org/apache/cassandra/cql3/operations/Operation.java
@@ -21,6 +21,7 @@ import java.nio.ByteBuffer;
 import java.util.List;
 
 import org.apache.cassandra.cql3.ColumnNameBuilder;
+import org.apache.cassandra.cql3.ColumnSpecification;
 import org.apache.cassandra.cql3.Term;
 import org.apache.cassandra.cql3.UpdateParameters;
 import org.apache.cassandra.db.ColumnFamily;
@@ -40,6 +41,8 @@ public interface Operation
                         UpdateParameters params,
                         List<Pair<ByteBuffer, IColumn>> list) throws InvalidRequestException;
 
+    public void addBoundNames(ColumnSpecification column, ColumnSpecification[] boundNames)
throws InvalidRequestException;
+
     public List<Term> getValues();
 
     public boolean requiresRead(AbstractType<?> validator);

http://git-wip-us.apache.org/repos/asf/cassandra/blob/a90b310e/src/java/org/apache/cassandra/cql3/operations/PreparedOperation.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/operations/PreparedOperation.java b/src/java/org/apache/cassandra/cql3/operations/PreparedOperation.java
index 540fdde..969e63c 100644
--- a/src/java/org/apache/cassandra/cql3/operations/PreparedOperation.java
+++ b/src/java/org/apache/cassandra/cql3/operations/PreparedOperation.java
@@ -125,6 +125,12 @@ public class PreparedOperation implements Operation
         }
     }
 
+    public void addBoundNames(ColumnSpecification column, ColumnSpecification[] boundNames)
throws InvalidRequestException
+    {
+        if (preparedValue.isBindMarker())
+            boundNames[preparedValue.bindIndex] = column;
+    }
+
     public List<Term> getValues()
     {
         return Collections.singletonList(preparedValue);

http://git-wip-us.apache.org/repos/asf/cassandra/blob/a90b310e/src/java/org/apache/cassandra/cql3/operations/SetOperation.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/operations/SetOperation.java b/src/java/org/apache/cassandra/cql3/operations/SetOperation.java
index 5d63bd9..e7f01c6 100644
--- a/src/java/org/apache/cassandra/cql3/operations/SetOperation.java
+++ b/src/java/org/apache/cassandra/cql3/operations/SetOperation.java
@@ -23,6 +23,7 @@ import java.util.List;
 import java.util.Set;
 
 import org.apache.cassandra.cql3.ColumnNameBuilder;
+import org.apache.cassandra.cql3.ColumnSpecification;
 import org.apache.cassandra.cql3.Term;
 import org.apache.cassandra.cql3.UpdateParameters;
 import org.apache.cassandra.db.ColumnFamily;
@@ -145,6 +146,13 @@ public class SetOperation implements Operation
         }
     }
 
+    public void addBoundNames(ColumnSpecification column, ColumnSpecification[] boundNames)
throws InvalidRequestException
+    {
+        for (Term t : values)
+            if (t.isBindMarker())
+                boundNames[t.bindIndex] = column;
+    }
+
     public List<Term> getValues()
     {
         return values;

http://git-wip-us.apache.org/repos/asf/cassandra/blob/a90b310e/src/java/org/apache/cassandra/cql3/statements/BatchStatement.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/statements/BatchStatement.java b/src/java/org/apache/cassandra/cql3/statements/BatchStatement.java
index 3909159..6d97376 100644
--- a/src/java/org/apache/cassandra/cql3/statements/BatchStatement.java
+++ b/src/java/org/apache/cassandra/cql3/statements/BatchStatement.java
@@ -128,7 +128,7 @@ public class BatchStatement extends ModificationStatement
         return mutations.values();
     }
 
-    public ParsedStatement.Prepared prepare(CFDefinition.Name[] boundNames) throws InvalidRequestException
+    public ParsedStatement.Prepared prepare(ColumnSpecification[] boundNames) throws InvalidRequestException
     {
         // XXX: we use our knowledge that Modification don't create new statement upon call
to prepare()
         for (ModificationStatement statement : statements)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/a90b310e/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 7d543dc..d8369b7 100644
--- a/src/java/org/apache/cassandra/cql3/statements/DeleteStatement.java
+++ b/src/java/org/apache/cassandra/cql3/statements/DeleteStatement.java
@@ -198,7 +198,7 @@ public class DeleteStatement extends ModificationStatement
         return rm;
     }
 
-    public ParsedStatement.Prepared prepare(CFDefinition.Name[] boundNames) throws InvalidRequestException
+    public ParsedStatement.Prepared prepare(ColumnSpecification[] boundNames) throws InvalidRequestException
     {
         CFMetaData metadata = ThriftValidation.validateColumnFamily(keyspace(), columnFamily());
         type = metadata.getDefaultValidator().isCommutative() ? Type.COUNTER : Type.LOGGED;
@@ -217,8 +217,23 @@ public class DeleteStatement extends ModificationStatement
             if (name.kind != CFDefinition.Name.Kind.COLUMN_METADATA && name.kind
!= CFDefinition.Name.Kind.VALUE_ALIAS)
                 throw new InvalidRequestException(String.format("Invalid identifier %s for
deletion (should not be a PRIMARY KEY part)", column));
 
-            if (column.key() != null && !(name.type instanceof ListType || name.type
instanceof MapType))
-                throw new InvalidRequestException(String.format("Invalid selection %s since
%s is neither a list or a map", column, column.id()));
+            if (column.key() != null)
+            {
+                if (name.type instanceof ListType)
+                {
+                    if (column.key().isBindMarker())
+                        boundNames[column.key().bindIndex] = ListOperation.indexSpecOf(name);
+                }
+                else if (name.type instanceof MapType)
+                {
+                    if (column.key().isBindMarker())
+                        boundNames[column.key().bindIndex] = MapOperation.keySpecOf(name,
(MapType)name.type);
+                }
+                else
+                {
+                    throw new InvalidRequestException(String.format("Invalid selection %s
since %s is neither a list or a map", column, column.id()));
+                }
+            }
 
             toRemove.add(Pair.create(name, column.key()));
         }
@@ -228,7 +243,7 @@ public class DeleteStatement extends ModificationStatement
 
     public ParsedStatement.Prepared prepare() throws InvalidRequestException
     {
-        CFDefinition.Name[] boundNames = new CFDefinition.Name[getBoundsTerms()];
+        ColumnSpecification[] boundNames = new ColumnSpecification[getBoundsTerms()];
         return prepare(boundNames);
     }
 

http://git-wip-us.apache.org/repos/asf/cassandra/blob/a90b310e/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 77bf83d..4af27ba 100644
--- a/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java
+++ b/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java
@@ -211,5 +211,5 @@ public abstract class ModificationStatement extends CFStatement implements
CQLSt
     protected abstract Collection<? extends IMutation> getMutations(List<ByteBuffer>
variables, boolean local, ConsistencyLevel cl, long now)
     throws RequestExecutionException, RequestValidationException;
 
-    public abstract ParsedStatement.Prepared prepare(CFDefinition.Name[] boundNames) throws
InvalidRequestException;
+    public abstract ParsedStatement.Prepared prepare(ColumnSpecification[] boundNames) throws
InvalidRequestException;
 }

http://git-wip-us.apache.org/repos/asf/cassandra/blob/a90b310e/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 48b74ac..46b1b18 100644
--- a/src/java/org/apache/cassandra/cql3/statements/UpdateStatement.java
+++ b/src/java/org/apache/cassandra/cql3/statements/UpdateStatement.java
@@ -260,7 +260,7 @@ public class UpdateStatement extends ModificationStatement
         return type == Type.COUNTER ? new CounterMutation(rm, cl) : rm;
     }
 
-    public ParsedStatement.Prepared prepare(CFDefinition.Name[] boundNames) throws InvalidRequestException
+    public ParsedStatement.Prepared prepare(ColumnSpecification[] boundNames) throws InvalidRequestException
     {
         // Deal here with the keyspace overwrite thingy to avoid mistake
         CFMetaData metadata = validateColumnFamily(keyspace(), columnFamily());
@@ -285,9 +285,7 @@ public class UpdateStatement extends ModificationStatement
                     throw new InvalidRequestException(String.format("Unknown identifier %s",
columnNames.get(i)));
 
                 Operation operation = columnOperations.get(i);
-                for (Term t : operation.getValues())
-                    if (t.isBindMarker())
-                        boundNames[t.bindIndex] = name;
+                operation.addBoundNames(name, boundNames);
 
                 switch (name.kind)
                 {
@@ -353,9 +351,7 @@ public class UpdateStatement extends ModificationStatement
                             if (otherOp.getType() == Operation.Type.COLUMN)
                                 throw new InvalidRequestException(String.format("Multiple
definitions found for column %s", name));
 
-                        for (Term t : operation.getValues())
-                            if (t.isBindMarker())
-                                boundNames[t.bindIndex] = name;
+                        operation.addBoundNames(name, boundNames);
                         processedColumns.put(name, operation);
                         break;
                 }
@@ -368,12 +364,12 @@ public class UpdateStatement extends ModificationStatement
 
     public ParsedStatement.Prepared prepare() throws InvalidRequestException
     {
-        CFDefinition.Name[] names = new CFDefinition.Name[getBoundsTerms()];
+        ColumnSpecification[] names = new ColumnSpecification[getBoundsTerms()];
         return prepare(names);
     }
 
     // Reused by DeleteStatement
-    static void processKeys(CFDefinition cfDef, List<Relation> keys, Map<ColumnIdentifier,
List<Term>> processed, CFDefinition.Name[] names) throws InvalidRequestException
+    static void processKeys(CFDefinition cfDef, List<Relation> keys, Map<ColumnIdentifier,
List<Term>> processed, ColumnSpecification[] names) throws InvalidRequestException
     {
         for (Relation rel : keys)
         {


Mime
View raw message