cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From slebre...@apache.org
Subject [2/3] git commit: Fix preparing queries with counter columns
Date Wed, 05 Dec 2012 08:02:59 GMT
Fix preparing queries with counter columns

patch by slebresne; reviewed by iamaleksey for CASSANDRA-5022


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

Branch: refs/heads/cassandra-1.2
Commit: ba8dfe300c6e52766ad7f98afdd6e2ff8e67b79b
Parents: 25d41ce
Author: Sylvain Lebresne <sylvain@datastax.com>
Authored: Wed Dec 5 09:01:32 2012 +0100
Committer: Sylvain Lebresne <sylvain@datastax.com>
Committed: Wed Dec 5 09:01:32 2012 +0100

----------------------------------------------------------------------
 CHANGES.txt                                        |    1 +
 .../cql3/operations/PreparedOperation.java         |    4 +
 .../cassandra/cql3/statements/UpdateStatement.java |   96 ++++++---------
 3 files changed, 42 insertions(+), 59 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/ba8dfe30/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index ad2a1a8..14fe46e 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -2,6 +2,7 @@
  * rename rpc_timeout settings to request_timeout (CASSANDRA-5027)
  * 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)
 
 
 1.2-beta3

http://git-wip-us.apache.org/repos/asf/cassandra/blob/ba8dfe30/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 a54b065..540fdde 100644
--- a/src/java/org/apache/cassandra/cql3/operations/PreparedOperation.java
+++ b/src/java/org/apache/cassandra/cql3/operations/PreparedOperation.java
@@ -136,6 +136,10 @@ public class PreparedOperation implements Operation
         return (validator instanceof ListType) && kind == Kind.MINUS_PREPARED;
     }
 
+    public boolean isPotentialCounterOperation() {
+        return kind == Kind.PLUS_PREPARED || kind == Kind.MINUS_PREPARED;
+    }
+
     public Type getType()
     {
         return Type.PREPARED;

http://git-wip-us.apache.org/repos/asf/cassandra/blob/ba8dfe30/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 c690530..48b74ac 100644
--- a/src/java/org/apache/cassandra/cql3/statements/UpdateStatement.java
+++ b/src/java/org/apache/cassandra/cql3/statements/UpdateStatement.java
@@ -26,6 +26,7 @@ import org.apache.cassandra.cql3.*;
 import org.apache.cassandra.config.CFMetaData;
 import org.apache.cassandra.cql3.operations.ColumnOperation;
 import org.apache.cassandra.cql3.operations.Operation;
+import org.apache.cassandra.cql3.operations.PreparedOperation;
 import org.apache.cassandra.db.*;
 import org.apache.cassandra.db.marshal.*;
 import org.apache.cassandra.exceptions.*;
@@ -205,8 +206,6 @@ public class UpdateStatement extends ModificationStatement
     throws InvalidRequestException
     {
         validateKey(key);
-        // if true we need to wrap RowMutation into CounterMutation
-        boolean hasCounterColumn = false;
 
         QueryProcessor.validateKey(key);
         RowMutation rm = new RowMutation(cfDef.cfm.ksName, key);
@@ -246,7 +245,7 @@ public class UpdateStatement extends ModificationStatement
                 assert operations.size() == 1;
                 operation = operations.get(0);
             }
-            hasCounterColumn = addToMutation(cf, builder, cfDef.value, operation, params,
null);
+            operation.execute(cf, builder.copy(), cfDef.value == null ? null : cfDef.value.type,
params, null);
         }
         else
         {
@@ -254,70 +253,26 @@ public class UpdateStatement extends ModificationStatement
             {
                 CFDefinition.Name name = entry.getKey();
                 Operation op = entry.getValue();
-                hasCounterColumn |= addToMutation(cf, builder.copy().add(name.name.key),
name, op, params, group == null || !op.requiresRead(name.type) ? null : group.getCollection(name.name.key));
+                op.execute(cf, builder.copy().add(name.name.key), name.type, params, group
== null || !op.requiresRead(name.type) ? null : group.getCollection(name.name.key));
             }
         }
 
-        return (hasCounterColumn) ? new CounterMutation(rm, cl) : rm;
-    }
-
-    private boolean addToMutation(ColumnFamily cf,
-                                  ColumnNameBuilder builder,
-                                  CFDefinition.Name valueDef,
-                                  Operation valueOperation,
-                                  UpdateParameters params,
-                                  List<Pair<ByteBuffer, IColumn>> list) throws
InvalidRequestException
-    {
-        Operation.Type type = valueOperation.getType();
-
-        switch (type)
-        {
-            case COUNTER:
-                if (valueDef != null && valueDef.type.isCollection())
-                    throw new InvalidRequestException("Cannot assign collection value to
column with " + valueDef.type + " type.");
-                break;
-            case LIST:
-            case SET:
-            case MAP:
-                if (!valueDef.type.isCollection())
-                    throw new InvalidRequestException("Can't apply collection operation on
column with " + valueDef.type + " type.");
-                break;
-        }
-        valueOperation.execute(cf, builder.copy(), valueDef == null ? null : valueDef.type,
params, list);
-        return valueOperation.getType() == Operation.Type.COUNTER;
+        return type == Type.COUNTER ? new CounterMutation(rm, cl) : rm;
     }
 
     public ParsedStatement.Prepared prepare(CFDefinition.Name[] boundNames) throws InvalidRequestException
     {
-        if (columns != null)
-        {
-            for (Pair<ColumnIdentifier, Operation> column : columns)
-            {
-                if (column.right.getType() == Operation.Type.COUNTER)
-                {
-                    if (type == null)
-                        type = Type.COUNTER;
-                    else if (type != Type.COUNTER)
-                        throw new InvalidRequestException("Mix of counter and non-counter
operations is not allowed.");
-                }
-                else if (type == Type.COUNTER)
-                {
-                    throw new InvalidRequestException("Mix of counter and non-counter operations
is not allowed.");
-                }
-            }
-        }
-
-        if (type == null)
-            type = Type.LOGGED;
-
         // Deal here with the keyspace overwrite thingy to avoid mistake
-        CFMetaData metadata = validateColumnFamily(keyspace(), columnFamily(), type == Type.COUNTER);
+        CFMetaData metadata = validateColumnFamily(keyspace(), columnFamily());
         cfDef = metadata.getCfDef();
 
+        type = metadata.getDefaultValidator().isCommutative() ? Type.COUNTER : Type.LOGGED;
+
         if (columns == null)
         {
             // Created from an INSERT
-            // Don't hate, validate.
+            if (type == Type.COUNTER)
+                throw new InvalidRequestException("INSERT statement are not allowed on counter
tables, use UPDATE instead");
             if (columnNames.size() != columnOperations.size())
                 throw new InvalidRequestException("unmatched column names/values");
             if (columnNames.size() < 1)
@@ -363,6 +318,30 @@ public class UpdateStatement extends ModificationStatement
                 if (name == null)
                     throw new InvalidRequestException(String.format("Unknown identifier %s",
entry.left));
 
+                Operation operation = entry.right;
+
+                switch (operation.getType())
+                {
+                    case COUNTER:
+                        if (type != Type.COUNTER)
+                            throw new InvalidRequestException("Invalid counter operation
on non-counter table.");
+                        break;
+                    case LIST:
+                    case SET:
+                    case MAP:
+                        if (!name.type.isCollection())
+                            throw new InvalidRequestException("Cannot apply collection operation
on column " + name + " with " + name.type + " type.");
+                    // Fallthrough on purpose
+                    case COLUMN:
+                        if (type == Type.COUNTER)
+                            throw new InvalidRequestException("Invalid non-counter operation
on counter table.");
+                        break;
+                    case PREPARED:
+                        if (type == Type.COUNTER && !((PreparedOperation)operation).isPotentialCounterOperation())
+                            throw new InvalidRequestException("Invalid non-counter operation
on counter table.");
+                        break;
+                }
+
                 switch (name.kind)
                 {
                     case KEY_ALIAS:
@@ -370,15 +349,14 @@ public class UpdateStatement extends ModificationStatement
                         throw new InvalidRequestException(String.format("PRIMARY KEY part
%s found in SET part", entry.left));
                     case VALUE_ALIAS:
                     case COLUMN_METADATA:
-                        for (Operation op : processedColumns.get(name))
-                            if (op.getType() == Operation.Type.COLUMN)
+                        for (Operation otherOp : processedColumns.get(name))
+                            if (otherOp.getType() == Operation.Type.COLUMN)
                                 throw new InvalidRequestException(String.format("Multiple
definitions found for column %s", name));
 
-                        Operation op = entry.right;
-                        for (Term t : op.getValues())
+                        for (Term t : operation.getValues())
                             if (t.isBindMarker())
                                 boundNames[t.bindIndex] = name;
-                        processedColumns.put(name, op);
+                        processedColumns.put(name, operation);
                         break;
                 }
             }


Mime
View raw message