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 <[email protected]> Authored: Mon Jan 22 13:31:02 2018 +0100 Committer: Benjamin Lerer <[email protected]> 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: [email protected] For additional commands, e-mail: [email protected]
