now() is being rejected in INSERTs when inside collections patch by slebresne; reviewed by iamaleksey for CASSANDRA-5795
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/ce4e4b9b Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/ce4e4b9b Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/ce4e4b9b Branch: refs/heads/cassandra-1.2 Commit: ce4e4b9b5b29b0e0518aafe02544de3765ea9dd7 Parents: d38446a Author: Sylvain Lebresne <[email protected]> Authored: Wed Jul 24 17:46:52 2013 +0200 Committer: Sylvain Lebresne <[email protected]> Committed: Wed Jul 24 17:46:52 2013 +0200 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../apache/cassandra/cql3/AbstractMarker.java | 5 ++ src/java/org/apache/cassandra/cql3/Lists.java | 76 ++++++++++++++---- src/java/org/apache/cassandra/cql3/Maps.java | 84 +++++++++++++++----- src/java/org/apache/cassandra/cql3/Sets.java | 73 +++++++++++++---- src/java/org/apache/cassandra/cql3/Term.java | 28 ++++++- .../cassandra/cql3/functions/FunctionCall.java | 10 +++ 7 files changed, 219 insertions(+), 58 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/ce4e4b9b/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index aaeb10b..71a956b 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -27,6 +27,7 @@ after TTL expires (CASSANDRA-5762) * Sort nodetool help output (CASSANDRA-5776) * Fix column expiring during 2 phases compaction (CASSANDRA-5799) + * now() is being rejected in INSERTs when inside collections (CASSANDRA-5795) 1.2.6 http://git-wip-us.apache.org/repos/asf/cassandra/blob/ce4e4b9b/src/java/org/apache/cassandra/cql3/AbstractMarker.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql3/AbstractMarker.java b/src/java/org/apache/cassandra/cql3/AbstractMarker.java index d8353a1..b4a4143 100644 --- a/src/java/org/apache/cassandra/cql3/AbstractMarker.java +++ b/src/java/org/apache/cassandra/cql3/AbstractMarker.java @@ -40,6 +40,11 @@ public abstract class AbstractMarker extends Term.NonTerminal boundNames[bindIndex] = receiver; } + public boolean containsBindMarker() + { + return true; + } + /** * A parsed, but non prepared, bind marker. */ http://git-wip-us.apache.org/repos/asf/cassandra/blob/ce4e4b9b/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 b579b8c..ae95dca 100644 --- a/src/java/org/apache/cassandra/cql3/Lists.java +++ b/src/java/org/apache/cassandra/cql3/Lists.java @@ -19,6 +19,7 @@ package org.apache.cassandra.cql3; import java.nio.ByteBuffer; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.concurrent.atomic.AtomicReference; @@ -60,34 +61,27 @@ public abstract class Lists this.elements = elements; } - public Value prepare(ColumnSpecification receiver) throws InvalidRequestException + public Term prepare(ColumnSpecification receiver) throws InvalidRequestException { validateAssignableTo(receiver); ColumnSpecification valueSpec = Lists.valueSpecOf(receiver); - List<ByteBuffer> values = new ArrayList<ByteBuffer>(elements.size()); + List<Term> values = new ArrayList<Term>(elements.size()); + boolean allTerminal = true; for (Term.Raw rt : elements) { Term t = rt.prepare(valueSpec); - if (t instanceof Term.NonTerminal) + if (t.containsBindMarker()) throw new InvalidRequestException(String.format("Invalid list literal for %s: bind variables are not supported inside collection literals", receiver)); - // We don't allow prepared marker in collections, nor nested collections (for the later, prepare will throw an exception) - assert t instanceof Constants.Value; - ByteBuffer bytes = ((Constants.Value)t).bytes; - if (bytes == null) - throw new InvalidRequestException("null is not supported inside collections"); - - // We don't support value > 64K because the serialization format encode the length as an unsigned short. - if (bytes.remaining() > FBUtilities.MAX_UNSIGNED_SHORT) - throw new InvalidRequestException(String.format("List value is too long. List values are limited to %d bytes but %d bytes value provided", - FBUtilities.MAX_UNSIGNED_SHORT, - bytes.remaining())); + if (t instanceof Term.NonTerminal) + allTerminal = false; - values.add(bytes); + values.add(t); } - return new Value(values); + DelayedValue value = new DelayedValue(values); + return allTerminal ? value.bind(Collections.<ByteBuffer>emptyList()) : value; } private void validateAssignableTo(ColumnSpecification receiver) throws InvalidRequestException @@ -156,6 +150,56 @@ public abstract class Lists } } + /* + * Basically similar to a Value, but with some non-pure function (that need + * to be evaluated at execution time) in it. + * + * Note: this would also work for a list with bind markers, but we don't support + * that because 1) it's not excessively useful and 2) we wouldn't have a good + * column name to return in the ColumnSpecification for those markers (not a + * blocker per-se but we don't bother due to 1)). + */ + public static class DelayedValue extends Term.NonTerminal + { + private final List<Term> elements; + + public DelayedValue(List<Term> elements) + { + this.elements = elements; + } + + public boolean containsBindMarker() + { + // False since we don't support them in collection + return false; + } + + public void collectMarkerSpecification(ColumnSpecification[] boundNames) + { + } + + public Value bind(List<ByteBuffer> values) throws InvalidRequestException + { + List<ByteBuffer> buffers = new ArrayList<ByteBuffer>(elements.size()); + for (Term t : elements) + { + ByteBuffer bytes = t.bindAndGet(values); + + if (bytes == null) + throw new InvalidRequestException("null is not supported inside collections"); + + // We don't support value > 64K because the serialization format encode the length as an unsigned short. + if (bytes.remaining() > FBUtilities.MAX_UNSIGNED_SHORT) + throw new InvalidRequestException(String.format("List value is too long. List values are limited to %d bytes but %d bytes value provided", + FBUtilities.MAX_UNSIGNED_SHORT, + bytes.remaining())); + + buffers.add(bytes); + } + return new Value(buffers); + } + } + public static class Marker extends AbstractMarker { protected Marker(int bindIndex, ColumnSpecification receiver) http://git-wip-us.apache.org/repos/asf/cassandra/blob/ce4e4b9b/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 e6d7c7e..e34e076 100644 --- a/src/java/org/apache/cassandra/cql3/Maps.java +++ b/src/java/org/apache/cassandra/cql3/Maps.java @@ -19,6 +19,9 @@ package org.apache.cassandra.cql3; import java.nio.ByteBuffer; import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; +import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -58,42 +61,29 @@ public abstract class Maps this.entries = entries; } - public Value prepare(ColumnSpecification receiver) throws InvalidRequestException + public Term prepare(ColumnSpecification receiver) throws InvalidRequestException { validateAssignableTo(receiver); ColumnSpecification keySpec = Maps.keySpecOf(receiver); ColumnSpecification valueSpec = Maps.valueSpecOf(receiver); - Map<ByteBuffer, ByteBuffer> values = new TreeMap<ByteBuffer, ByteBuffer>(((MapType)receiver.type).keys); + Map<Term, Term> values = new HashMap<Term, Term>(entries.size()); + boolean allTerminal = true; for (Pair<Term.Raw, Term.Raw> entry : entries) { Term k = entry.left.prepare(keySpec); Term v = entry.right.prepare(valueSpec); - if (k instanceof Term.NonTerminal || v instanceof Term.NonTerminal) + if (k.containsBindMarker() || v.containsBindMarker()) throw new InvalidRequestException(String.format("Invalid map literal for %s: bind variables are not supported inside collection literals", receiver)); - // We don't support values > 64K because the serialization format encode the length as an unsigned short. - ByteBuffer keyBytes = ((Constants.Value)k).bytes; - if (keyBytes == null) - throw new InvalidRequestException("null is not supported inside collections"); - if (keyBytes.remaining() > FBUtilities.MAX_UNSIGNED_SHORT) - throw new InvalidRequestException(String.format("Map key is too long. Map keys are limited to %d bytes but %d bytes keys provided", - FBUtilities.MAX_UNSIGNED_SHORT, - keyBytes.remaining())); - - ByteBuffer valueBytes = ((Constants.Value)v).bytes; - if (valueBytes == null) - throw new InvalidRequestException("null is not supported inside collections"); - if (valueBytes.remaining() > FBUtilities.MAX_UNSIGNED_SHORT) - throw new InvalidRequestException(String.format("Map value is too long. Map values are limited to %d bytes but %d bytes value provided", - FBUtilities.MAX_UNSIGNED_SHORT, - valueBytes.remaining())); + if (k instanceof Term.NonTerminal || v instanceof Term.NonTerminal) + allTerminal = false; - if (values.put(keyBytes, valueBytes) != null) - throw new InvalidRequestException(String.format("Invalid map literal: duplicate entry for key %s", entry.left)); + values.put(k, v); } - return new Value(values); + DelayedValue value = new DelayedValue(((MapType)receiver.type).keys, values); + return allTerminal ? value.bind(Collections.<ByteBuffer>emptyList()) : value; } private void validateAssignableTo(ColumnSpecification receiver) throws InvalidRequestException @@ -179,6 +169,56 @@ public abstract class Maps } } + // See Lists.DelayedValue + public static class DelayedValue extends Term.NonTerminal + { + private final Comparator<ByteBuffer> comparator; + private final Map<Term, Term> elements; + + public DelayedValue(Comparator<ByteBuffer> comparator, Map<Term, Term> elements) + { + this.comparator = comparator; + this.elements = elements; + } + + public boolean containsBindMarker() + { + // False since we don't support them in collection + return false; + } + + public void collectMarkerSpecification(ColumnSpecification[] boundNames) + { + } + + public Value bind(List<ByteBuffer> values) throws InvalidRequestException + { + Map<ByteBuffer, ByteBuffer> buffers = new TreeMap<ByteBuffer, ByteBuffer>(comparator); + for (Map.Entry<Term, Term> entry : elements.entrySet()) + { + // We don't support values > 64K because the serialization format encode the length as an unsigned short. + ByteBuffer keyBytes = entry.getKey().bindAndGet(values); + if (keyBytes == null) + throw new InvalidRequestException("null is not supported inside collections"); + if (keyBytes.remaining() > FBUtilities.MAX_UNSIGNED_SHORT) + throw new InvalidRequestException(String.format("Map key is too long. Map keys are limited to %d bytes but %d bytes keys provided", + FBUtilities.MAX_UNSIGNED_SHORT, + keyBytes.remaining())); + + ByteBuffer valueBytes = entry.getValue().bindAndGet(values); + if (valueBytes == null) + throw new InvalidRequestException("null is not supported inside collections"); + if (valueBytes.remaining() > FBUtilities.MAX_UNSIGNED_SHORT) + throw new InvalidRequestException(String.format("Map value is too long. Map values are limited to %d bytes but %d bytes value provided", + FBUtilities.MAX_UNSIGNED_SHORT, + valueBytes.remaining())); + + buffers.put(keyBytes, valueBytes); + } + return new Value(buffers); + } + } + public static class Marker extends AbstractMarker { protected Marker(int bindIndex, ColumnSpecification receiver) http://git-wip-us.apache.org/repos/asf/cassandra/blob/ce4e4b9b/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 fed6625..748f269 100644 --- a/src/java/org/apache/cassandra/cql3/Sets.java +++ b/src/java/org/apache/cassandra/cql3/Sets.java @@ -20,6 +20,8 @@ package org.apache.cassandra.cql3; import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.Collections; +import java.util.Comparator; +import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; import java.util.Set; @@ -57,7 +59,7 @@ public abstract class Sets this.elements = elements; } - public Term.Terminal prepare(ColumnSpecification receiver) throws InvalidRequestException + public Term prepare(ColumnSpecification receiver) throws InvalidRequestException { validateAssignableTo(receiver); @@ -66,31 +68,24 @@ public abstract class Sets if (receiver.type instanceof MapType && elements.isEmpty()) return new Maps.Value(Collections.<ByteBuffer, ByteBuffer>emptyMap()); + ColumnSpecification valueSpec = Sets.valueSpecOf(receiver); - Set<ByteBuffer> values = new TreeSet<ByteBuffer>(((SetType)receiver.type).elements); + Set<Term> values = new HashSet<Term>(elements.size()); + boolean allTerminal = true; for (Term.Raw rt : elements) { Term t = rt.prepare(valueSpec); - if (t instanceof Term.NonTerminal) + if (t.containsBindMarker()) throw new InvalidRequestException(String.format("Invalid set literal for %s: bind variables are not supported inside collection literals", receiver)); - // We don't allow prepared marker in collections, nor nested collections (for the later, prepare will throw an exception) - assert t instanceof Constants.Value; - ByteBuffer bytes = ((Constants.Value)t).bytes; - if (bytes == null) - throw new InvalidRequestException("null is not supported inside collections"); - - // We don't support value > 64K because the serialization format encode the length as an unsigned short. - if (bytes.remaining() > FBUtilities.MAX_UNSIGNED_SHORT) - throw new InvalidRequestException(String.format("Set value is too long. Set values are limited to %d bytes but %d bytes value provided", - FBUtilities.MAX_UNSIGNED_SHORT, - bytes.remaining())); + if (t instanceof Term.NonTerminal) + allTerminal = false; - if (!values.add(bytes)) - throw new InvalidRequestException(String.format("Invalid set literal: duplicate value %s", rt)); + values.add(t); } - return new Value(values); + DelayedValue value = new DelayedValue(((SetType)receiver.type).elements, values); + return allTerminal ? value.bind(Collections.<ByteBuffer>emptyList()) : value; } private void validateAssignableTo(ColumnSpecification receiver) throws InvalidRequestException @@ -166,6 +161,50 @@ public abstract class Sets } } + // See Lists.DelayedValue + public static class DelayedValue extends Term.NonTerminal + { + private final Comparator<ByteBuffer> comparator; + private final Set<Term> elements; + + public DelayedValue(Comparator<ByteBuffer> comparator, Set<Term> elements) + { + this.comparator = comparator; + this.elements = elements; + } + + public boolean containsBindMarker() + { + // False since we don't support them in collection + return false; + } + + public void collectMarkerSpecification(ColumnSpecification[] boundNames) + { + } + + public Value bind(List<ByteBuffer> values) throws InvalidRequestException + { + Set<ByteBuffer> buffers = new TreeSet<ByteBuffer>(comparator); + for (Term t : elements) + { + ByteBuffer bytes = t.bindAndGet(values); + + if (bytes == null) + throw new InvalidRequestException("null is not supported inside collections"); + + // We don't support value > 64K because the serialization format encode the length as an unsigned short. + if (bytes.remaining() > FBUtilities.MAX_UNSIGNED_SHORT) + throw new InvalidRequestException(String.format("Set value is too long. Set values are limited to %d bytes but %d bytes value provided", + FBUtilities.MAX_UNSIGNED_SHORT, + bytes.remaining())); + + buffers.add(bytes); + } + return new Value(buffers); + } + } + public static class Marker extends AbstractMarker { protected Marker(int bindIndex, ColumnSpecification receiver) http://git-wip-us.apache.org/repos/asf/cassandra/blob/ce4e4b9b/src/java/org/apache/cassandra/cql3/Term.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql3/Term.java b/src/java/org/apache/cassandra/cql3/Term.java index 347a459..f2fd74e 100644 --- a/src/java/org/apache/cassandra/cql3/Term.java +++ b/src/java/org/apache/cassandra/cql3/Term.java @@ -59,6 +59,15 @@ public interface Term public ByteBuffer bindAndGet(List<ByteBuffer> values) throws InvalidRequestException; /** + * Whether or not that term contains at least one bind marker. + * + * Note that this is slightly different from being or not a NonTerminal, + * because calls to non pure functions will be NonTerminal (see #5616) + * even if they don't have bind markers. + */ + public abstract boolean containsBindMarker(); + + /** * A parsed, non prepared (thus untyped) term. * * This can be one of: @@ -83,7 +92,11 @@ public interface Term } /** - * A terminal term, i.e. one without any bind marker. + * A terminal term, one that can be reduced to a byte buffer directly. + * + * This includes most terms that don't have a bind marker (an exception + * being delayed call for non pure function that are NonTerminal even + * if they don't have bind markers). * * This can be only one of: * - a constant value @@ -97,6 +110,13 @@ public interface Term public void collectMarkerSpecification(ColumnSpecification[] boundNames) {} public Terminal bind(List<ByteBuffer> values) { return this; } + // While some NonTerminal may not have bind markers, no Term can be Terminal + // with a bind marker + public boolean containsBindMarker() + { + return false; + } + /** * @return the serialized value of this terminal. */ @@ -109,12 +129,14 @@ public interface Term } /** - * A non terminal term, i.e. one that contains at least one bind marker. + * A non terminal term, i.e. a term that can only be reduce to a byte buffer + * at execution time. * - * We distinguish between the following type of NonTerminal: + * We have the following type of NonTerminal: * - marker for a constant value * - marker for a collection value (list, set, map) * - a function having bind marker + * - a non pure function (even if it doesn't have bind marker - see #5616) */ public abstract class NonTerminal implements Term { http://git-wip-us.apache.org/repos/asf/cassandra/blob/ce4e4b9b/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 dcc3d8b..a6b86a2 100644 --- a/src/java/org/apache/cassandra/cql3/functions/FunctionCall.java +++ b/src/java/org/apache/cassandra/cql3/functions/FunctionCall.java @@ -72,6 +72,16 @@ public class FunctionCall extends Term.NonTerminal return fun.execute(buffers); } + public boolean containsBindMarker() + { + for (Term t : terms) + { + if (t.containsBindMarker()) + return true; + } + return false; + } + private static Term.Terminal makeTerminal(Function fun, ByteBuffer result) throws InvalidRequestException { if (!(fun.returnType() instanceof CollectionType))
