Support null values in PreparedStatements parameters patch by slebresne; reviewed by iamaleksey for CASSANDRA-5081
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/ef574569 Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/ef574569 Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/ef574569 Branch: refs/heads/trunk Commit: ef574569a23d33e18c3d9893a323a891a96215db Parents: 3a3f1d9 Author: Sylvain Lebresne <[email protected]> Authored: Mon Mar 4 17:58:16 2013 +0100 Committer: Sylvain Lebresne <[email protected]> Committed: Mon Mar 4 17:58:16 2013 +0100 ---------------------------------------------------------------------- CHANGES.txt | 1 + src/java/org/apache/cassandra/cql3/Constants.java | 20 ++++++-- src/java/org/apache/cassandra/cql3/Lists.java | 18 ++++++- src/java/org/apache/cassandra/cql3/Maps.java | 15 ++++- src/java/org/apache/cassandra/cql3/Sets.java | 7 ++- .../cassandra/cql3/functions/FunctionCall.java | 9 +++- .../cassandra/cql3/statements/SelectStatement.java | 40 ++++++++++++-- .../cassandra/cql3/statements/UpdateStatement.java | 15 ++++- 8 files changed, 103 insertions(+), 22 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/ef574569/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index a6b2cff..c1ae2b9 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -7,6 +7,7 @@ * Fix insufficient validation of UPDATE queries against counter cfs (CASSANDRA-5300) * Fix PropertyFileSnitch default DC/Rack behavior (CASSANDRA-5285) + * Handle null values when executing prepared statement (CASSANDRA-5081) Merged from 1.1: * nodetool: ability to repair specific range (CASSANDRA-5280) * Fix possible assertion triggered in SliceFromReadCommand (CASSANDRA-5284) http://git-wip-us.apache.org/repos/asf/cassandra/blob/ef574569/src/java/org/apache/cassandra/cql3/Constants.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql3/Constants.java b/src/java/org/apache/cassandra/cql3/Constants.java index 60098be..f630869 100644 --- a/src/java/org/apache/cassandra/cql3/Constants.java +++ b/src/java/org/apache/cassandra/cql3/Constants.java @@ -254,7 +254,8 @@ public abstract class Constants try { ByteBuffer value = values.get(bindIndex); - receiver.type.validate(value); + if (value != null) + receiver.type.validate(value); return value; } catch (MarshalException e) @@ -265,7 +266,8 @@ public abstract class Constants public Value bind(List<ByteBuffer> values) throws InvalidRequestException { - return new Constants.Value(bindAndGet(values)); + ByteBuffer bytes = bindAndGet(values); + return bytes == null ? null : new Constants.Value(bytes); } } @@ -279,7 +281,8 @@ public abstract class Constants public void execute(ByteBuffer rowKey, ColumnFamily cf, ColumnNameBuilder prefix, UpdateParameters params) throws InvalidRequestException { ByteBuffer cname = columnName == null ? prefix.build() : prefix.add(columnName.key).build(); - cf.addColumn(params.makeColumn(cname, t.bindAndGet(params.variables))); + ByteBuffer value = t.bindAndGet(params.variables); + cf.addColumn(value == null ? params.makeTombstone(cname) : params.makeColumn(cname, value)); } } @@ -292,7 +295,10 @@ public abstract class Constants public void execute(ByteBuffer rowKey, ColumnFamily cf, ColumnNameBuilder prefix, UpdateParameters params) throws InvalidRequestException { - long increment = ByteBufferUtil.toLong(t.bindAndGet(params.variables)); + ByteBuffer bytes = t.bindAndGet(params.variables); + if (bytes == null) + throw new InvalidRequestException("Invalid null value for counter increment"); + long increment = ByteBufferUtil.toLong(bytes); ByteBuffer cname = columnName == null ? prefix.build() : prefix.add(columnName.key).build(); cf.addCounter(new QueryPath(cf.metadata().cfName, null, cname), increment); } @@ -307,7 +313,11 @@ public abstract class Constants public void execute(ByteBuffer rowKey, ColumnFamily cf, ColumnNameBuilder prefix, UpdateParameters params) throws InvalidRequestException { - long increment = ByteBufferUtil.toLong(t.bindAndGet(params.variables)); + ByteBuffer bytes = t.bindAndGet(params.variables); + if (bytes == null) + throw new InvalidRequestException("Invalid null value for counter increment"); + + long increment = ByteBufferUtil.toLong(bytes); if (increment == Long.MIN_VALUE) throw new InvalidRequestException("The negation of " + increment + " overflows supported counter precision (signed 8 bytes integer)"); http://git-wip-us.apache.org/repos/asf/cassandra/blob/ef574569/src/java/org/apache/cassandra/cql3/Lists.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql3/Lists.java b/src/java/org/apache/cassandra/cql3/Lists.java index 0da8410..76f947c 100644 --- a/src/java/org/apache/cassandra/cql3/Lists.java +++ b/src/java/org/apache/cassandra/cql3/Lists.java @@ -161,7 +161,7 @@ public abstract class Lists public Value bind(List<ByteBuffer> values) throws InvalidRequestException { ByteBuffer value = values.get(bindIndex); - return Value.fromSerialized(value, (ListType)receiver.type); + return value == null ? null : Value.fromSerialized(value, (ListType)receiver.type); } } @@ -276,8 +276,12 @@ public abstract class Lists static void doAppend(Term t, ColumnFamily cf, ColumnNameBuilder columnName, UpdateParameters params) throws InvalidRequestException { Term.Terminal value = t.bind(params.variables); - assert value instanceof Lists.Value; + // If we append null, do nothing. Note that for Setter, we've + // already removed the previous value so we're good here too + if (value == null) + return; + assert value instanceof Lists.Value; List<ByteBuffer> toAdd = ((Lists.Value)value).elements; for (int i = 0; i < toAdd.size(); i++) { @@ -299,8 +303,10 @@ public abstract class Lists public void execute(ByteBuffer rowKey, ColumnFamily cf, ColumnNameBuilder prefix, UpdateParameters params) throws InvalidRequestException { Term.Terminal value = t.bind(params.variables); - assert value instanceof Lists.Value; + if (value == null) + return; + assert value instanceof Lists.Value; long time = PrecisionTime.REFERENCE_TIME - (System.currentTimeMillis() - PrecisionTime.REFERENCE_TIME); List<ByteBuffer> toAdd = ((Lists.Value)value).elements; @@ -336,6 +342,9 @@ public abstract class Lists return; Term.Terminal value = t.bind(params.variables); + if (value == null) + return; + assert value instanceof Lists.Value; // Note: below, we will call 'contains' on this toDiscard list for each element of existingList. @@ -368,6 +377,9 @@ public abstract class Lists public void execute(ByteBuffer rowKey, ColumnFamily cf, ColumnNameBuilder prefix, UpdateParameters params) throws InvalidRequestException { Term.Terminal index = t.bind(params.variables); + if (index == null) + throw new InvalidRequestException("Invalid null value for list index"); + assert index instanceof Constants.Value; List<Pair<ByteBuffer, IColumn>> existingList = params.getPrefetchedList(rowKey, columnName.key); http://git-wip-us.apache.org/repos/asf/cassandra/blob/ef574569/src/java/org/apache/cassandra/cql3/Maps.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql3/Maps.java b/src/java/org/apache/cassandra/cql3/Maps.java index e62c101..b49edc9 100644 --- a/src/java/org/apache/cassandra/cql3/Maps.java +++ b/src/java/org/apache/cassandra/cql3/Maps.java @@ -178,7 +178,7 @@ public abstract class Maps public Value bind(List<ByteBuffer> values) throws InvalidRequestException { ByteBuffer value = values.get(bindIndex); - return Value.fromSerialized(value, (MapType)receiver.type); + return value == null ? null : Value.fromSerialized(value, (MapType)receiver.type); } } @@ -219,10 +219,15 @@ public abstract class Maps { Term.Terminal key = k.bind(params.variables); Term.Terminal value = t.bind(params.variables); - assert key instanceof Constants.Value && value instanceof Constants.Value; + if (key == null) + throw new InvalidRequestException("Invalid null map key"); + assert key instanceof Constants.Value; + assert value == null || value instanceof Constants.Value; ByteBuffer cellName = prefix.add(columnName.key).add(((Constants.Value)key).bytes).build(); - cf.addColumn(params.makeColumn(cellName, ((Constants.Value)value).bytes)); + cf.addColumn(value == null + ? params.makeTombstone(cellName) + : params.makeColumn(cellName, ((Constants.Value)value).bytes)); } } @@ -241,6 +246,8 @@ public abstract class Maps static void doPut(Term t, ColumnFamily cf, ColumnNameBuilder columnName, UpdateParameters params) throws InvalidRequestException { Term.Terminal value = t.bind(params.variables); + if (value == null) + return; assert value instanceof Maps.Value; Map<ByteBuffer, ByteBuffer> toAdd = ((Maps.Value)value).map; @@ -262,6 +269,8 @@ public abstract class Maps public void execute(ByteBuffer rowKey, ColumnFamily cf, ColumnNameBuilder prefix, UpdateParameters params) throws InvalidRequestException { Term.Terminal key = t.bind(params.variables); + if (key == null) + throw new InvalidRequestException("Invalid null map key"); assert key instanceof Constants.Value; ByteBuffer cellName = prefix.add(columnName.key).add(((Constants.Value)key).bytes).build(); http://git-wip-us.apache.org/repos/asf/cassandra/blob/ef574569/src/java/org/apache/cassandra/cql3/Sets.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql3/Sets.java b/src/java/org/apache/cassandra/cql3/Sets.java index 1537507..aa68714 100644 --- a/src/java/org/apache/cassandra/cql3/Sets.java +++ b/src/java/org/apache/cassandra/cql3/Sets.java @@ -169,7 +169,7 @@ public abstract class Sets public Value bind(List<ByteBuffer> values) throws InvalidRequestException { ByteBuffer value = values.get(bindIndex); - return Value.fromSerialized(value, (SetType)receiver.type); + return value == null ? null : Value.fromSerialized(value, (SetType)receiver.type); } } @@ -204,6 +204,9 @@ public abstract class Sets static void doAdd(Term t, ColumnFamily cf, ColumnNameBuilder columnName, UpdateParameters params) throws InvalidRequestException { Term.Terminal value = t.bind(params.variables); + if (value == null) + return; + assert value instanceof Sets.Value; Set<ByteBuffer> toAdd = ((Sets.Value)value).elements; @@ -225,6 +228,8 @@ public abstract class Sets public void execute(ByteBuffer rowKey, ColumnFamily cf, ColumnNameBuilder prefix, UpdateParameters params) throws InvalidRequestException { Term.Terminal value = t.bind(params.variables); + if (value == null) + return; // This can be either a set or a single element Set<ByteBuffer> toDiscard = value instanceof Constants.Value http://git-wip-us.apache.org/repos/asf/cassandra/blob/ef574569/src/java/org/apache/cassandra/cql3/functions/FunctionCall.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql3/functions/FunctionCall.java b/src/java/org/apache/cassandra/cql3/functions/FunctionCall.java index daaaad9..36603d6 100644 --- a/src/java/org/apache/cassandra/cql3/functions/FunctionCall.java +++ b/src/java/org/apache/cassandra/cql3/functions/FunctionCall.java @@ -61,7 +61,14 @@ public class FunctionCall extends Term.NonTerminal { List<ByteBuffer> buffers = new ArrayList<ByteBuffer>(terms.size()); for (Term t : terms) - buffers.add(t.bindAndGet(values)); + { + // For now, we don't allow nulls as argument as no existing function needs it and it + // simplify things. + ByteBuffer val = t.bindAndGet(values); + if (val == null) + throw new InvalidRequestException(String.format("Invalid null value for argument to %s", fun)); + buffers.add(val); + } return fun.execute(buffers); } http://git-wip-us.apache.org/repos/asf/cassandra/blob/ef574569/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java index 825844a..d5a7425 100644 --- a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java @@ -343,13 +343,21 @@ public class SelectStatement implements CQLStatement if (builder.remainingCount() == 1) { for (Term t : r.eqValues) - keys.add(builder.copy().add(t.bindAndGet(variables)).build()); + { + ByteBuffer val = t.bindAndGet(variables); + if (val == null) + throw new InvalidRequestException(String.format("Invalid null value for partition key part %s", name)); + keys.add(builder.copy().add(val).build()); + } } else { if (r.eqValues.size() > 1) throw new InvalidRequestException("IN is only supported on the last column of the partition key"); - builder.add(r.eqValues.get(0).bindAndGet(variables)); + ByteBuffer val = r.eqValues.get(0).bindAndGet(variables); + if (val == null) + throw new InvalidRequestException(String.format("Invalid null value for partition key part %s", name)); + builder.add(val); } } return keys; @@ -373,6 +381,8 @@ public class SelectStatement implements CQLStatement return p.getMinimumToken(); ByteBuffer value = t.bindAndGet(variables); + if (value == null) + throw new InvalidRequestException("Invalid null token value"); return p.getTokenFactory().fromByteArray(value); } @@ -414,8 +424,10 @@ public class SelectStatement implements CQLStatement assert !isColumnRange(); ColumnNameBuilder builder = cfDef.getColumnNameBuilder(); + Iterator<ColumnIdentifier> idIter = cfDef.columns.keySet().iterator(); for (Restriction r : columnRestrictions) { + ColumnIdentifier id = idIter.next(); assert r != null && r.isEquality(); if (r.eqValues.size() > 1) { @@ -428,7 +440,10 @@ public class SelectStatement implements CQLStatement { Term v = iter.next(); ColumnNameBuilder b = iter.hasNext() ? builder.copy() : builder; - b.add(v.bindAndGet(variables)); + ByteBuffer val = v.bindAndGet(variables); + if (val == null) + throw new InvalidRequestException(String.format("Invalid null value for clustering key part %s", id)); + b.add(val); if (cfDef.isCompact) columns.add(b.build()); else @@ -438,7 +453,10 @@ public class SelectStatement implements CQLStatement } else { - builder.add(r.eqValues.get(0).bindAndGet(variables)); + ByteBuffer val = r.eqValues.get(0).bindAndGet(variables); + if (val == null) + throw new InvalidRequestException(String.format("Invalid null value for clustering key part %s", id)); + builder.add(val); } } @@ -520,13 +538,19 @@ public class SelectStatement implements CQLStatement if (r.isEquality()) { assert r.eqValues.size() == 1; - builder.add(r.eqValues.get(0).bindAndGet(variables)); + ByteBuffer val = r.eqValues.get(0).bindAndGet(variables); + if (val == null) + throw new InvalidRequestException(String.format("Invalid null clustering key part %s", name)); + builder.add(val); } else { Term t = r.bound(b); assert t != null; - return builder.add(t.bindAndGet(variables), r.getRelation(eocBound, b)).build(); + ByteBuffer val = t.bindAndGet(variables); + if (val == null) + throw new InvalidRequestException(String.format("Invalid null clustering key part %s", name)); + return builder.add(val, r.getRelation(eocBound, b)).build(); } } // Means no relation at all or everything was an equal @@ -559,6 +583,8 @@ public class SelectStatement implements CQLStatement for (Term t : restriction.eqValues) { ByteBuffer value = t.bindAndGet(variables); + if (value == null) + throw new InvalidRequestException(String.format("Unsupported null value for indexed column %s", name)); if (value.remaining() > 0xFFFF) throw new InvalidRequestException("Index expression values may not be larger than 64K"); expressions.add(new IndexExpression(name.name.key, IndexOperator.EQ, value)); @@ -571,6 +597,8 @@ public class SelectStatement implements CQLStatement if (restriction.bound(b) != null) { ByteBuffer value = restriction.bound(b).bindAndGet(variables); + if (value == null) + throw new InvalidRequestException(String.format("Unsupported null value for indexed column %s", name)); if (value.remaining() > 0xFFFF) throw new InvalidRequestException("Index expression values may not be larger than 64K"); expressions.add(new IndexExpression(name.name.key, restriction.getIndexOperator(b), value)); http://git-wip-us.apache.org/repos/asf/cassandra/blob/ef574569/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 683d24b..7dbd14a 100644 --- a/src/java/org/apache/cassandra/cql3/statements/UpdateStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/UpdateStatement.java @@ -156,7 +156,10 @@ public class UpdateStatement extends ModificationStatement else { assert values.size() == 1; // We only allow IN for row keys so far - builder.add(values.get(0).bindAndGet(variables)); + ByteBuffer val = values.get(0).bindAndGet(variables); + if (val == null) + throw new InvalidRequestException(String.format("Invalid null value for clustering key part %s", name)); + builder.add(val); } } return firstEmpty; @@ -177,14 +180,20 @@ public class UpdateStatement extends ModificationStatement { for (Term t : values) { - keys.add(keyBuilder.copy().add(t.bindAndGet(variables)).build()); + ByteBuffer val = values.get(0).bindAndGet(variables); + if (val == null) + throw new InvalidRequestException(String.format("Invalid null value for partition key part %s", name)); + keys.add(keyBuilder.copy().add(val).build()); } } else { if (values.size() > 1) throw new InvalidRequestException("IN is only supported on the last column of the partition key"); - keyBuilder.add(values.get(0).bindAndGet(variables)); + ByteBuffer val = values.get(0).bindAndGet(variables); + if (val == null) + throw new InvalidRequestException(String.format("Invalid null value for partition key part %s", name)); + keyBuilder.add(val); } } return keys;
