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 D0001DF57 for ; Thu, 6 Dec 2012 07:41:10 +0000 (UTC) Received: (qmail 99047 invoked by uid 500); 6 Dec 2012 07:41:10 -0000 Delivered-To: apmail-cassandra-commits-archive@cassandra.apache.org Received: (qmail 98853 invoked by uid 500); 6 Dec 2012 07:41:10 -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 98814 invoked by uid 99); 6 Dec 2012 07:41:08 -0000 Received: from tyr.zones.apache.org (HELO tyr.zones.apache.org) (140.211.11.114) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 06 Dec 2012 07:41:08 +0000 Received: by tyr.zones.apache.org (Postfix, from userid 65534) id 4791B81943D; Thu, 6 Dec 2012 07:41:08 +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: git commit: Fix preparing updates with CQL3 and collections Message-Id: <20121206074108.4791B81943D@tyr.zones.apache.org> Date: Thu, 6 Dec 2012 07:41:08 +0000 (UTC) Updated Branches: refs/heads/cassandra-1.2.0 a41b7806a -> a90b310e6 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/cassandra-1.2.0 Commit: a90b310e61495d4d782c25a8debb44798cc00b79 Parents: a41b780 Author: Sylvain Lebresne Authored: Tue Dec 4 13:30:48 2012 +0100 Committer: Sylvain Lebresne 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> 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.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.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 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> 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 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 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 getValues() { List l = new ArrayList(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> list) throws InvalidRequestException; + public void addBoundNames(ColumnSpecification column, ColumnSpecification[] boundNames) throws InvalidRequestException; + public List 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 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 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 getMutations(List 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 keys, Map> processed, CFDefinition.Name[] names) throws InvalidRequestException + static void processKeys(CFDefinition cfDef, List keys, Map> processed, ColumnSpecification[] names) throws InvalidRequestException { for (Relation rel : keys) {