Return-Path: X-Original-To: apmail-cassandra-commits-archive@www.apache.org Delivered-To: apmail-cassandra-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id E3B64D1CD for ; Wed, 5 Dec 2012 08:03:01 +0000 (UTC) Received: (qmail 57135 invoked by uid 500); 5 Dec 2012 08:03:01 -0000 Delivered-To: apmail-cassandra-commits-archive@cassandra.apache.org Received: (qmail 56898 invoked by uid 500); 5 Dec 2012 08:03:00 -0000 Mailing-List: contact commits-help@cassandra.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@cassandra.apache.org Delivered-To: mailing list commits@cassandra.apache.org Received: (qmail 56799 invoked by uid 99); 5 Dec 2012 08:03:00 -0000 Received: from tyr.zones.apache.org (HELO tyr.zones.apache.org) (140.211.11.114) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 05 Dec 2012 08:03:00 +0000 Received: by tyr.zones.apache.org (Postfix, from userid 65534) id CB72D818C3F; Wed, 5 Dec 2012 08:02:59 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: slebresne@apache.org To: commits@cassandra.apache.org X-Mailer: ASF-Git Admin Mailer Subject: [2/3] git commit: Fix preparing queries with counter columns Message-Id: <20121205080259.CB72D818C3F@tyr.zones.apache.org> Date: Wed, 5 Dec 2012 08:02:59 +0000 (UTC) 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 Authored: Wed Dec 5 09:01:32 2012 +0100 Committer: Sylvain Lebresne 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> 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 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; } }