You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by sl...@apache.org on 2012/12/05 09:02:59 UTC
[2/3] git commit: Fix preparing queries with counter columns
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 <sy...@datastax.com>
Authored: Wed Dec 5 09:01:32 2012 +0100
Committer: Sylvain Lebresne <sy...@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;
}
}