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/06 08:42:02 UTC
[3/3] git commit: Fix preparing updates with CQL3 and collections
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 <sy...@datastax.com>
Authored: Tue Dec 4 13:30:48 2012 +0100
Committer: Sylvain Lebresne <sy...@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)
{