cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From slebre...@apache.org
Subject [2/2] git commit: now() is being rejected in INSERTs when inside collections
Date Wed, 24 Jul 2013 15:47:43 GMT
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 <sylvain@datastax.com>
Authored: Wed Jul 24 17:46:52 2013 +0200
Committer: Sylvain Lebresne <sylvain@datastax.com>
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))


Mime
View raw message