You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by bl...@apache.org on 2018/01/24 15:11:09 UTC

cassandra git commit: Make sub-range selection for non-frozen collections return null instead of empty

Repository: cassandra
Updated Branches:
  refs/heads/trunk 9635c6041 -> 4de7a65ed


Make sub-range selection for non-frozen collections return null instead of empty

patch by Benjamin Lerer; reviewed by Andrés de la Peña for CASSANDRA-14182


Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/4de7a65e
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/4de7a65e
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/4de7a65e

Branch: refs/heads/trunk
Commit: 4de7a65ed9f3c97658a80dd64032ad6e82e9d58b
Parents: 9635c60
Author: Benjamin Lerer <b....@gmail.com>
Authored: Mon Jan 22 13:31:02 2018 +0100
Committer: Benjamin Lerer <b....@gmail.com>
Committed: Wed Jan 24 16:08:18 2018 +0100

----------------------------------------------------------------------
 CHANGES.txt                                     |  1 +
 .../cql3/selection/ElementsSelector.java        |  2 +-
 .../serializers/CollectionSerializer.java       | 12 +++-
 .../cassandra/serializers/ListSerializer.java   |  8 ++-
 .../cassandra/serializers/MapSerializer.java    | 12 +++-
 .../cassandra/serializers/SetSerializer.java    | 12 +++-
 .../validation/entities/CollectionsTest.java    | 70 +++++++++++++++++++-
 7 files changed, 107 insertions(+), 10 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/4de7a65e/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index b34cb1b..b29ae78 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
 4.0
+ * Make sub-range selection for non-frozen collections return null instead of empty (CASSANDRA-14182)
  * BloomFilter serialization format should not change byte ordering (CASSANDRA-9067)
  * Remove unused on-heap BloomFilter implementation (CASSANDRA-14152)
  * Delete temp test files on exit (CASSANDRA-14153)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/4de7a65e/src/java/org/apache/cassandra/cql3/selection/ElementsSelector.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/selection/ElementsSelector.java b/src/java/org/apache/cassandra/cql3/selection/ElementsSelector.java
index f94b5c8..9427c51 100644
--- a/src/java/org/apache/cassandra/cql3/selection/ElementsSelector.java
+++ b/src/java/org/apache/cassandra/cql3/selection/ElementsSelector.java
@@ -304,7 +304,7 @@ abstract class ElementsSelector extends Selector
 
         protected ByteBuffer extractSelection(ByteBuffer collection)
         {
-            return type.getSerializer().getSliceFromSerialized(collection, from, to, type.nameComparator());
+            return type.getSerializer().getSliceFromSerialized(collection, from, to, type.nameComparator(), type.isFrozenCollection());
         }
 
         public AbstractType<?> getType()

http://git-wip-us.apache.org/repos/asf/cassandra/blob/4de7a65e/src/java/org/apache/cassandra/serializers/CollectionSerializer.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/serializers/CollectionSerializer.java b/src/java/org/apache/cassandra/serializers/CollectionSerializer.java
index 9d261c6..d988cc0 100644
--- a/src/java/org/apache/cassandra/serializers/CollectionSerializer.java
+++ b/src/java/org/apache/cassandra/serializers/CollectionSerializer.java
@@ -134,6 +134,8 @@ public abstract class CollectionSerializer<T> implements TypeSerializer<T>
 
     /**
      * Returns the slice of a collection directly from its serialized value.
+     * <p>If the slice contains no elements an empty collection will be returned for frozen collections, and a 
+     * {@code null} one for non-frozen collections.</p>
      *
      * @param collection the serialized collection. This cannot be {@code null}.
      * @param from the left bound of the slice to extract. This cannot be {@code null} but if this is
@@ -141,10 +143,14 @@ public abstract class CollectionSerializer<T> implements TypeSerializer<T>
      * of {@code collection}.
      * @param comparator the type to use to compare the {@code from} and {@code to} values to those
      * in the collection.
-     * @return a valid serialized collection (possibly empty) corresponding to slice {@code [from, to]}
-     * of {@code collection}.
+     * @param frozen {@code true} if the collection is a frozen one, {@code false} otherwise
+     * @return a serialized collection corresponding to slice {@code [from, to]} of {@code collection}.
      */
-    public abstract ByteBuffer getSliceFromSerialized(ByteBuffer collection, ByteBuffer from, ByteBuffer to, AbstractType<?> comparator);
+    public abstract ByteBuffer getSliceFromSerialized(ByteBuffer collection,
+                                                      ByteBuffer from,
+                                                      ByteBuffer to,
+                                                      AbstractType<?> comparator,
+                                                      boolean frozen);
 
     /**
      * Creates a new serialized map composed from the data from {@code input} between {@code startPos}

http://git-wip-us.apache.org/repos/asf/cassandra/blob/4de7a65e/src/java/org/apache/cassandra/serializers/ListSerializer.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/serializers/ListSerializer.java b/src/java/org/apache/cassandra/serializers/ListSerializer.java
index 98ebd48..dd0bc9e 100644
--- a/src/java/org/apache/cassandra/serializers/ListSerializer.java
+++ b/src/java/org/apache/cassandra/serializers/ListSerializer.java
@@ -169,13 +169,19 @@ public class ListSerializer<T> extends CollectionSerializer<List<T>>
         return (Class) List.class;
     }
 
+    @Override
     public ByteBuffer getSerializedValue(ByteBuffer collection, ByteBuffer key, AbstractType<?> comparator)
     {
         // We don't allow selecting an element of a list so we don't need this.
         throw new UnsupportedOperationException();
     }
 
-    public ByteBuffer getSliceFromSerialized(ByteBuffer collection, ByteBuffer from, ByteBuffer to, AbstractType<?> comparator)
+    @Override
+    public ByteBuffer getSliceFromSerialized(ByteBuffer collection,
+                                             ByteBuffer from,
+                                             ByteBuffer to,
+                                             AbstractType<?> comparator,
+                                             boolean frozen)
     {
         // We don't allow slicing of list so we don't need this.
         throw new UnsupportedOperationException();

http://git-wip-us.apache.org/repos/asf/cassandra/blob/4de7a65e/src/java/org/apache/cassandra/serializers/MapSerializer.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/serializers/MapSerializer.java b/src/java/org/apache/cassandra/serializers/MapSerializer.java
index f428502..c772a45 100644
--- a/src/java/org/apache/cassandra/serializers/MapSerializer.java
+++ b/src/java/org/apache/cassandra/serializers/MapSerializer.java
@@ -129,6 +129,7 @@ public class MapSerializer<K, V> extends CollectionSerializer<Map<K, V>>
         }
     }
 
+    @Override
     public ByteBuffer getSerializedValue(ByteBuffer collection, ByteBuffer key, AbstractType<?> comparator)
     {
         try
@@ -155,7 +156,12 @@ public class MapSerializer<K, V> extends CollectionSerializer<Map<K, V>>
         }
     }
 
-    public ByteBuffer getSliceFromSerialized(ByteBuffer collection, ByteBuffer from, ByteBuffer to, AbstractType<?> comparator)
+    @Override
+    public ByteBuffer getSliceFromSerialized(ByteBuffer collection,
+                                             ByteBuffer from,
+                                             ByteBuffer to,
+                                             AbstractType<?> comparator,
+                                             boolean frozen)
     {
         if (from == ByteBufferUtil.UNSET_BYTE_BUFFER && to == ByteBufferUtil.UNSET_BYTE_BUFFER)
             return collection;
@@ -208,6 +214,10 @@ public class MapSerializer<K, V> extends CollectionSerializer<Map<K, V>>
                 if (comparison == 0)
                     break;
             }
+
+            if (count == 0 && !frozen)
+                return null;
+
             return copyAsNewCollection(collection, count, startPos, input.position(), ProtocolVersion.V3);
         }
         catch (BufferUnderflowException e)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/4de7a65e/src/java/org/apache/cassandra/serializers/SetSerializer.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/serializers/SetSerializer.java b/src/java/org/apache/cassandra/serializers/SetSerializer.java
index e4e970c..2548219 100644
--- a/src/java/org/apache/cassandra/serializers/SetSerializer.java
+++ b/src/java/org/apache/cassandra/serializers/SetSerializer.java
@@ -140,6 +140,7 @@ public class SetSerializer<T> extends CollectionSerializer<Set<T>>
         return (Class) Set.class;
     }
 
+    @Override
     public ByteBuffer getSerializedValue(ByteBuffer collection, ByteBuffer key, AbstractType<?> comparator)
     {
         try
@@ -165,7 +166,12 @@ public class SetSerializer<T> extends CollectionSerializer<Set<T>>
         }
     }
 
-    public ByteBuffer getSliceFromSerialized(ByteBuffer collection, ByteBuffer from, ByteBuffer to, AbstractType<?> comparator)
+    @Override
+    public ByteBuffer getSliceFromSerialized(ByteBuffer collection,
+                                             ByteBuffer from,
+                                             ByteBuffer to,
+                                             AbstractType<?> comparator,
+                                             boolean frozen)
     {
         if (from == ByteBufferUtil.UNSET_BYTE_BUFFER && to == ByteBufferUtil.UNSET_BYTE_BUFFER)
             return collection;
@@ -216,6 +222,10 @@ public class SetSerializer<T> extends CollectionSerializer<Set<T>>
                 if (comparison == 0)
                     break;
             }
+
+            if (count == 0 && !frozen)
+                return null;
+
             return copyAsNewCollection(collection, count, startPos, input.position(), ProtocolVersion.V3);
         }
         catch (BufferUnderflowException e)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/4de7a65e/test/unit/org/apache/cassandra/cql3/validation/entities/CollectionsTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/cql3/validation/entities/CollectionsTest.java b/test/unit/org/apache/cassandra/cql3/validation/entities/CollectionsTest.java
index e26f207..9ad47c0 100644
--- a/test/unit/org/apache/cassandra/cql3/validation/entities/CollectionsTest.java
+++ b/test/unit/org/apache/cassandra/cql3/validation/entities/CollectionsTest.java
@@ -1198,7 +1198,7 @@ public class CollectionsTest extends CQLTester
 
         assertRows(execute("SELECT k, l, m['2'..'3'], o FROM %s WHERE k = 0"),
                    row(0, "foobar", map("22", "value22"), 42),
-                   row(0, "foobar", map(), 42)
+                   row(0, "foobar", null, 42)
         );
 
         assertRows(execute("SELECT k, l, m['22'..], o FROM %s WHERE k = 0"),
@@ -1711,7 +1711,7 @@ public class CollectionsTest extends CQLTester
 
         assertRows(result,
                    row(0,
-                       map("1", "one", "2", "two"), "two", map("2", "two"), map("1", "one", "2", "two"), map(),
+                       map("1", "one", "2", "two"), "two", map("2", "two"), map("1", "one", "2", "two"), null,
                        map("1", "one", "2", "two"), "two", map("2", "two"), map("1", "one", "2", "two"), map(),
                        set("1", "2", "3"), "2", set("2", "3"), set("1", "2"), set("3"),
                        set("1", "2", "3"), "2", set("2", "3"), set("1", "2"), set("3")));
@@ -1815,7 +1815,7 @@ public class CollectionsTest extends CQLTester
         flush();
 
         assertRows(execute("SELECT m[7..8] FROM %s WHERE k=?", 0),
-                   row(map()));
+                   row((Map<Integer, Integer>) null));
 
         assertRows(execute("SELECT m[0..3] FROM %s WHERE k=?", 0),
                    row(map(0, 0, 1, 1, 2, 2, 3, 3)));
@@ -1915,4 +1915,68 @@ public class CollectionsTest extends CQLTester
         assertInvalidMessage("Invalid map literal for m: value (1, '1', 1.0, 1) is not of type frozen<tuple<int, text, double>>",
                              "INSERT INTO %s (k, m) VALUES (0, {1 : (1, '1', 1.0, 1)})");
     }
+
+    @Test
+    public void testSelectionOfEmptyCollections() throws Throwable
+    {
+        createTable("CREATE TABLE %s (k int PRIMARY KEY, m frozen<map<text, int>>, s frozen<set<int>>)");
+
+        execute("INSERT INTO %s(k) VALUES (0)");
+        execute("INSERT INTO %s(k, m, s) VALUES (1, {}, {})");
+        execute("INSERT INTO %s(k, m, s) VALUES (2, ?, ?)", map(), set());
+        execute("INSERT INTO %s(k, m, s) VALUES (3, {'2':2}, {2})");
+
+        beforeAndAfterFlush(() ->
+        {
+            assertRows(execute("SELECT m, s FROM %s WHERE k = 0"), row(null, null));
+            assertRows(execute("SELECT m['0'], s[0] FROM %s WHERE k = 0"), row(null, null));
+            assertRows(execute("SELECT m['0'..'1'], s[0..1] FROM %s WHERE k = 0"), row(null, null));
+            assertRows(execute("SELECT m['0'..'1']['3'..'5'], s[0..1][3..5] FROM %s WHERE k = 0"), row(null, null));
+
+            assertRows(execute("SELECT m, s FROM %s WHERE k = 1"), row(map(), set()));
+            assertRows(execute("SELECT m['0'], s[0] FROM %s WHERE k = 1"), row(null, null));
+            assertRows(execute("SELECT m['0'..'1'], s[0..1] FROM %s WHERE k = 1"), row(map(), set()));
+            assertRows(execute("SELECT m['0'..'1']['3'..'5'], s[0..1][3..5] FROM %s WHERE k = 1"), row(map(), set()));
+
+            assertRows(execute("SELECT m, s FROM %s WHERE k = 2"), row(map(), set()));
+            assertRows(execute("SELECT m['0'], s[0] FROM %s WHERE k = 2"), row(null, null));
+            assertRows(execute("SELECT m['0'..'1'], s[0..1] FROM %s WHERE k = 2"), row(map(), set()));
+            assertRows(execute("SELECT m['0'..'1']['3'..'5'], s[0..1][3..5] FROM %s WHERE k = 2"), row(map(), set()));
+
+            assertRows(execute("SELECT m, s FROM %s WHERE k = 3"), row(map("2", 2), set(2)));
+            assertRows(execute("SELECT m['0'], s[0] FROM %s WHERE k = 3"), row(null, null));
+            assertRows(execute("SELECT m['0'..'1'], s[0..1] FROM %s WHERE k = 3"), row(map(), set()));
+            assertRows(execute("SELECT m['0'..'1']['3'..'5'], s[0..1][3..5] FROM %s WHERE k = 3"), row(map(), set()));
+        });
+
+        createTable("CREATE TABLE %s (k int PRIMARY KEY, m map<text, int>, s set<int>)");
+
+        execute("INSERT INTO %s(k) VALUES (0)");
+        execute("INSERT INTO %s(k, m, s) VALUES (1, {}, {})");
+        execute("INSERT INTO %s(k, m, s) VALUES (2, ?, ?)", map(), set());
+        execute("INSERT INTO %s(k, m, s) VALUES (3, {'2':2}, {2})");
+
+        beforeAndAfterFlush(() ->
+        {
+            assertRows(execute("SELECT m, s FROM %s WHERE k = 0"), row(null, null));
+            assertRows(execute("SELECT m['0'], s[0] FROM %s WHERE k = 0"), row(null, null));
+            assertRows(execute("SELECT m['0'..'1'], s[0..1] FROM %s WHERE k = 0"), row(null, null));
+            assertRows(execute("SELECT m['0'..'1']['3'..'5'], s[0..1][3..5] FROM %s WHERE k = 0"), row(null, null));
+
+            assertRows(execute("SELECT m, s FROM %s WHERE k = 1"), row(null, null));
+            assertRows(execute("SELECT m['0'], s[0] FROM %s WHERE k = 1"), row(null, null));
+            assertRows(execute("SELECT m['0'..'1'], s[0..1] FROM %s WHERE k = 1"), row(null, null));
+            assertRows(execute("SELECT m['0'..'1']['3'..'5'], s[0..1][3..5] FROM %s WHERE k = 1"), row(null, null));
+
+            assertRows(execute("SELECT m, s FROM %s WHERE k = 2"), row(null, null));
+            assertRows(execute("SELECT m['0'], s[0] FROM %s WHERE k = 2"), row(null, null));
+            assertRows(execute("SELECT m['0'..'1'], s[0..1] FROM %s WHERE k = 2"), row(null, null));
+            assertRows(execute("SELECT m['0'..'1']['3'..'5'], s[0..1][3..5] FROM %s WHERE k = 2"), row(null, null));
+
+            assertRows(execute("SELECT m, s FROM %s WHERE k = 3"), row(map("2", 2), set(2)));
+            assertRows(execute("SELECT m['0'], s[0] FROM %s WHERE k = 3"), row(null, null));
+            assertRows(execute("SELECT m['0'..'1'], s[0..1] FROM %s WHERE k = 3"), row(null, null));
+            assertRows(execute("SELECT m['0'..'1']['3'..'5'], s[0..1][3..5] FROM %s WHERE k = 3"), row(null, null));
+        });
+    }
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org