Repository: cassandra Updated Branches: refs/heads/trunk 4ca7a8cfd -> c19aa154a
Remove 64k limit on collection elements patch by Benjamin Lerer; reviewed by Sylvain Lebresne for CASSANDRA-10374 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/bc9a61fc Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/bc9a61fc Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/bc9a61fc Branch: refs/heads/trunk Commit: bc9a61fc851791926e8d2b6134f2bbca68a0ae11 Parents: 4b01e8f Author: Benjamin Lerer <b.le...@gmail.com> Authored: Thu Dec 3 22:51:03 2015 +0100 Committer: Benjamin Lerer <b.le...@gmail.com> Committed: Thu Dec 3 22:51:03 2015 +0100 ---------------------------------------------------------------------- CHANGES.txt | 1 + src/java/org/apache/cassandra/cql3/Lists.java | 13 -- src/java/org/apache/cassandra/cql3/Maps.java | 17 +- src/java/org/apache/cassandra/cql3/Sets.java | 7 - .../validation/entities/CollectionsTest.java | 166 +++++++++++++++++++ .../entities/FrozenCollectionsTest.java | 98 +++++++++++ 6 files changed, 266 insertions(+), 36 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/bc9a61fc/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index dd5d22b..f6aed18 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 3.0.1 + * Remove 64k limit on collection elements (CASSANDRA-10374) * Remove unclear Indexer.indexes() method (CASSANDRA-10690) * Fix NPE on stream read error (CASSANDRA-10771) * Normalize cqlsh DESC output (CASSANDRA-10431) http://git-wip-us.apache.org/repos/asf/cassandra/blob/bc9a61fc/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 830561e..4b41a9d 100644 --- a/src/java/org/apache/cassandra/cql3/Lists.java +++ b/src/java/org/apache/cassandra/cql3/Lists.java @@ -36,7 +36,6 @@ import org.apache.cassandra.serializers.CollectionSerializer; import org.apache.cassandra.serializers.MarshalException; import org.apache.cassandra.transport.Server; import org.apache.cassandra.utils.ByteBufferUtil; -import org.apache.cassandra.utils.FBUtilities; import org.apache.cassandra.utils.UUIDGen; /** @@ -211,12 +210,6 @@ public abstract class Lists if (bytes == ByteBufferUtil.UNSET_BYTE_BUFFER) return UNSET_VALUE; - // 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); @@ -370,12 +363,6 @@ public abstract class Lists } else if (value != ByteBufferUtil.UNSET_BYTE_BUFFER) { - // We don't support value > 64K because the serialization format encode the length as an unsigned short. - if (value.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, - value.remaining())); - params.addCell(column, elementPath, value); } } http://git-wip-us.apache.org/repos/asf/cassandra/blob/bc9a61fc/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 d5df279..fbb7ee3 100644 --- a/src/java/org/apache/cassandra/cql3/Maps.java +++ b/src/java/org/apache/cassandra/cql3/Maps.java @@ -35,7 +35,6 @@ import org.apache.cassandra.serializers.CollectionSerializer; import org.apache.cassandra.serializers.MarshalException; import org.apache.cassandra.transport.Server; import org.apache.cassandra.utils.ByteBufferUtil; -import org.apache.cassandra.utils.FBUtilities; import org.apache.cassandra.utils.Pair; /** @@ -225,14 +224,11 @@ public abstract class Maps { // We don't support values > 64K because the serialization format encode the length as an unsigned short. ByteBuffer keyBytes = entry.getKey().bindAndGet(options); + if (keyBytes == null) throw new InvalidRequestException("null is not supported inside collections"); if (keyBytes == ByteBufferUtil.UNSET_BYTE_BUFFER) throw new InvalidRequestException("unset value is not supported for map keys"); - 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(options); if (valueBytes == null) @@ -240,11 +236,6 @@ public abstract class Maps if (valueBytes == ByteBufferUtil.UNSET_BYTE_BUFFER) return UNSET_VALUE; - 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); @@ -331,12 +322,6 @@ public abstract class Maps } else if (value != ByteBufferUtil.UNSET_BYTE_BUFFER) { - // We don't support value > 64K because the serialization format encode the length as an unsigned short. - if (value.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, - value.remaining())); - params.addCell(column, path, value); } } http://git-wip-us.apache.org/repos/asf/cassandra/blob/bc9a61fc/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 010abaa..1e93144 100644 --- a/src/java/org/apache/cassandra/cql3/Sets.java +++ b/src/java/org/apache/cassandra/cql3/Sets.java @@ -35,7 +35,6 @@ import org.apache.cassandra.serializers.CollectionSerializer; import org.apache.cassandra.serializers.MarshalException; import org.apache.cassandra.transport.Server; import org.apache.cassandra.utils.ByteBufferUtil; -import org.apache.cassandra.utils.FBUtilities; /** * Static helper methods and classes for sets. @@ -213,12 +212,6 @@ public abstract class Sets if (bytes == ByteBufferUtil.UNSET_BYTE_BUFFER) return UNSET_VALUE; - // 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); http://git-wip-us.apache.org/repos/asf/cassandra/blob/bc9a61fc/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 df8d507..48e5ad3 100644 --- a/test/unit/org/apache/cassandra/cql3/validation/entities/CollectionsTest.java +++ b/test/unit/org/apache/cassandra/cql3/validation/entities/CollectionsTest.java @@ -20,11 +20,13 @@ package org.apache.cassandra.cql3.validation.entities; import java.util.Map; import java.util.Random; import java.util.Set; +import java.util.Arrays; import java.util.UUID; import org.junit.Test; import org.apache.cassandra.cql3.CQLTester; +import org.apache.cassandra.utils.FBUtilities; import static org.junit.Assert.assertEquals; @@ -686,4 +688,168 @@ public class CollectionsTest extends CQLTester execute("delete s_list[0] from %s where k1='a'"); assertRows(execute("select s_list from %s where k1='a'"), row(list(0))); } + + @Test + public void testListWithElementsBiggerThan64K() throws Throwable + { + createTable("CREATE TABLE %s (k int PRIMARY KEY, l list<text>)"); + + byte[] bytes = new byte[FBUtilities.MAX_UNSIGNED_SHORT + 10]; + Arrays.fill(bytes, (byte) 1); + String largeText = new String(bytes); + + bytes = new byte[FBUtilities.MAX_UNSIGNED_SHORT + 10]; + Arrays.fill(bytes, (byte) 2); + String largeText2 = new String(bytes); + + execute("INSERT INTO %s(k, l) VALUES (0, ?)", list(largeText, "v2")); + flush(); + + assertRows(execute("SELECT l FROM %s WHERE k = 0"), row(list(largeText, "v2"))); + + execute("DELETE l[?] FROM %s WHERE k = 0", 0); + + assertRows(execute("SELECT l FROM %s WHERE k = 0"), row(list("v2"))); + + execute("UPDATE %s SET l[?] = ? WHERE k = 0", 0, largeText); + + assertRows(execute("SELECT l FROM %s WHERE k = 0"), row(list(largeText))); + + // Full overwrite + execute("UPDATE %s SET l = ? WHERE k = 0", list("v1", largeText)); + flush(); + + assertRows(execute("SELECT l FROM %s WHERE k = 0"), row(list("v1", largeText))); + + execute("UPDATE %s SET l = l + ? WHERE k = 0", list("v2", largeText2)); + + assertRows(execute("SELECT l FROM %s WHERE k = 0"), row(list("v1", largeText, "v2", largeText2))); + + execute("UPDATE %s SET l = l - ? WHERE k = 0", list(largeText, "v2")); + + assertRows(execute("SELECT l FROM %s WHERE k = 0"), row(list("v1", largeText2))); + + execute("DELETE l FROM %s WHERE k = 0"); + + assertRows(execute("SELECT l FROM %s WHERE k = 0"), row((Object) null)); + + execute("INSERT INTO %s(k, l) VALUES (0, ['" + largeText + "', 'v2'])"); + flush(); + + assertRows(execute("SELECT l FROM %s WHERE k = 0"), row(list(largeText, "v2"))); + } + + @Test + public void testMapsWithElementsBiggerThan64K() throws Throwable + { + byte[] bytes = new byte[FBUtilities.MAX_UNSIGNED_SHORT + 10]; + Arrays.fill(bytes, (byte) 1); + String largeText = new String(bytes); + bytes = new byte[FBUtilities.MAX_UNSIGNED_SHORT + 10]; + Arrays.fill(bytes, (byte) 2); + String largeText2 = new String(bytes); + + createTable("CREATE TABLE %s (k int PRIMARY KEY, m map<text, text>)"); + + execute("INSERT INTO %s(k, m) VALUES (0, ?)", map("k1", largeText, largeText, "v2")); + flush(); + + assertRows(execute("SELECT m FROM %s WHERE k = 0"), + row(map("k1", largeText, largeText, "v2"))); + + execute("UPDATE %s SET m[?] = ? WHERE k = 0", "k3", largeText); + + assertRows(execute("SELECT m FROM %s WHERE k = 0"), + row(map("k1", largeText, largeText, "v2", "k3", largeText))); + + execute("UPDATE %s SET m[?] = ? WHERE k = 0", largeText2, "v4"); + + assertRows(execute("SELECT m FROM %s WHERE k = 0"), + row(map("k1", largeText, largeText, "v2", "k3", largeText, largeText2, "v4"))); + + execute("DELETE m[?] FROM %s WHERE k = 0", "k1"); + + assertRows(execute("SELECT m FROM %s WHERE k = 0"), + row(map(largeText, "v2", "k3", largeText, largeText2, "v4"))); + + execute("DELETE m[?] FROM %s WHERE k = 0", largeText2); + flush(); + + assertRows(execute("SELECT m FROM %s WHERE k = 0"), + row(map(largeText, "v2", "k3", largeText))); + + // Full overwrite + execute("UPDATE %s SET m = ? WHERE k = 0", map("k5", largeText, largeText, "v6")); + flush(); + + assertRows(execute("SELECT m FROM %s WHERE k = 0"), + row(map("k5", largeText, largeText, "v6"))); + + execute("UPDATE %s SET m = m + ? WHERE k = 0", map("k7", largeText)); + + assertRows(execute("SELECT m FROM %s WHERE k = 0"), + row(map("k5", largeText, largeText, "v6", "k7", largeText))); + + execute("UPDATE %s SET m = m + ? WHERE k = 0", map(largeText2, "v8")); + flush(); + + assertRows(execute("SELECT m FROM %s WHERE k = 0"), + row(map("k5", largeText, largeText, "v6", "k7", largeText, largeText2, "v8"))); + + execute("DELETE m FROM %s WHERE k = 0"); + + assertRows(execute("SELECT m FROM %s WHERE k = 0"), row((Object) null)); + + execute("INSERT INTO %s(k, m) VALUES (0, {'" + largeText + "' : 'v1', 'k2' : '" + largeText + "'})"); + flush(); + + assertRows(execute("SELECT m FROM %s WHERE k = 0"), + row(map(largeText, "v1", "k2", largeText))); + } + + @Test + public void testSetsWithElementsBiggerThan64K() throws Throwable + { + createTable("CREATE TABLE %s (k int PRIMARY KEY, s set<text>)"); + + byte[] bytes = new byte[FBUtilities.MAX_UNSIGNED_SHORT + 10]; + Arrays.fill(bytes, (byte) 1); + String largeText = new String(bytes); + + bytes = new byte[FBUtilities.MAX_UNSIGNED_SHORT + 10]; + Arrays.fill(bytes, (byte) 2); + String largeText2 = new String(bytes); + + execute("INSERT INTO %s(k, s) VALUES (0, ?)", set(largeText, "v2")); + flush(); + + assertRows(execute("SELECT s FROM %s WHERE k = 0"), row(set(largeText, "v2"))); + + execute("DELETE s[?] FROM %s WHERE k = 0", largeText); + + assertRows(execute("SELECT s FROM %s WHERE k = 0"), row(set("v2"))); + + // Full overwrite + execute("UPDATE %s SET s = ? WHERE k = 0", set("v1", largeText)); + flush(); + + assertRows(execute("SELECT s FROM %s WHERE k = 0"), row(set("v1", largeText))); + + execute("UPDATE %s SET s = s + ? WHERE k = 0", set("v2", largeText2)); + + assertRows(execute("SELECT s FROM %s WHERE k = 0"), row(set("v1", largeText, "v2", largeText2))); + + execute("UPDATE %s SET s = s - ? WHERE k = 0", set(largeText, "v2")); + + assertRows(execute("SELECT s FROM %s WHERE k = 0"), row(set("v1", largeText2))); + + execute("DELETE s FROM %s WHERE k = 0"); + + assertRows(execute("SELECT s FROM %s WHERE k = 0"), row((Object) null)); + + execute("INSERT INTO %s(k, s) VALUES (0, {'" + largeText + "', 'v2'})"); + flush(); + + assertRows(execute("SELECT s FROM %s WHERE k = 0"), row(set(largeText, "v2"))); + } } http://git-wip-us.apache.org/repos/asf/cassandra/blob/bc9a61fc/test/unit/org/apache/cassandra/cql3/validation/entities/FrozenCollectionsTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/cql3/validation/entities/FrozenCollectionsTest.java b/test/unit/org/apache/cassandra/cql3/validation/entities/FrozenCollectionsTest.java index a0d64be..523a1ed 100644 --- a/test/unit/org/apache/cassandra/cql3/validation/entities/FrozenCollectionsTest.java +++ b/test/unit/org/apache/cassandra/cql3/validation/entities/FrozenCollectionsTest.java @@ -33,6 +33,7 @@ import org.apache.cassandra.exceptions.ConfigurationException; import org.apache.cassandra.exceptions.InvalidRequestException; import org.apache.cassandra.exceptions.SyntaxException; import org.apache.cassandra.service.StorageService; +import org.apache.cassandra.utils.FBUtilities; import static org.junit.Assert.assertEquals; @@ -1113,4 +1114,101 @@ public class FrozenCollectionsTest extends CQLTester TupleType tuple = new TupleType(types); assertEquals("TupleType(SetType(Int32Type))", clean(tuple.toString())); } + + @Test + public void testListWithElementsBiggerThan64K() throws Throwable + { + createTable("CREATE TABLE %s (k int PRIMARY KEY, l frozen<list<text>>)"); + + byte[] bytes = new byte[FBUtilities.MAX_UNSIGNED_SHORT + 10]; + Arrays.fill(bytes, (byte) 1); + String largeText = new String(bytes); + + execute("INSERT INTO %s(k, l) VALUES (0, ?)", list(largeText, "v2")); + flush(); + + assertRows(execute("SELECT l FROM %s WHERE k = 0"), row(list(largeText, "v2"))); + + // Full overwrite + execute("UPDATE %s SET l = ? WHERE k = 0", list("v1", largeText)); + flush(); + + assertRows(execute("SELECT l FROM %s WHERE k = 0"), row(list("v1", largeText))); + + execute("DELETE l FROM %s WHERE k = 0"); + + assertRows(execute("SELECT l FROM %s WHERE k = 0"), row((Object) null)); + + execute("INSERT INTO %s(k, l) VALUES (0, ['" + largeText + "', 'v2'])"); + flush(); + + assertRows(execute("SELECT l FROM %s WHERE k = 0"), row(list(largeText, "v2"))); + } + + @Test + public void testMapsWithElementsBiggerThan64K() throws Throwable + { + byte[] bytes = new byte[FBUtilities.MAX_UNSIGNED_SHORT + 10]; + Arrays.fill(bytes, (byte) 1); + String largeText = new String(bytes); + + bytes = new byte[FBUtilities.MAX_UNSIGNED_SHORT + 10]; + Arrays.fill(bytes, (byte) 2); + String largeText2 = new String(bytes); + + createTable("CREATE TABLE %s (k int PRIMARY KEY, m frozen<map<text, text>>)"); + + execute("INSERT INTO %s(k, m) VALUES (0, ?)", map(largeText, "v1", "k2", largeText)); + flush(); + + assertRows(execute("SELECT m FROM %s WHERE k = 0"), + row(map(largeText, "v1", "k2", largeText))); + + // Full overwrite + execute("UPDATE %s SET m = ? WHERE k = 0", map("k5", largeText, largeText2, "v6")); + flush(); + + assertRows(execute("SELECT m FROM %s WHERE k = 0"), + row(map("k5", largeText, largeText2, "v6"))); + + execute("DELETE m FROM %s WHERE k = 0"); + + assertRows(execute("SELECT m FROM %s WHERE k = 0"), row((Object) null)); + + execute("INSERT INTO %s(k, m) VALUES (0, {'" + largeText + "' : 'v1', 'k2' : '" + largeText + "'})"); + flush(); + + assertRows(execute("SELECT m FROM %s WHERE k = 0"), + row(map(largeText, "v1", "k2", largeText))); + } + + @Test + public void testSetsWithElementsBiggerThan64K() throws Throwable + { + createTable("CREATE TABLE %s (k int PRIMARY KEY, s frozen<set<text>>)"); + + byte[] bytes = new byte[FBUtilities.MAX_UNSIGNED_SHORT + 10]; + Arrays.fill(bytes, (byte) 1); + String largeText = new String(bytes); + + execute("INSERT INTO %s(k, s) VALUES (0, ?)", set(largeText, "v1", "v2")); + flush(); + + assertRows(execute("SELECT s FROM %s WHERE k = 0"), row(set(largeText, "v1", "v2"))); + + // Full overwrite + execute("UPDATE %s SET s = ? WHERE k = 0", set(largeText, "v3")); + flush(); + + assertRows(execute("SELECT s FROM %s WHERE k = 0"), row(set(largeText, "v3"))); + + execute("DELETE s FROM %s WHERE k = 0"); + + assertRows(execute("SELECT s FROM %s WHERE k = 0"), row((Object) null)); + + execute("INSERT INTO %s(k, s) VALUES (0, {'" + largeText + "', 'v1', 'v2'})"); + flush(); + + assertRows(execute("SELECT s FROM %s WHERE k = 0"), row(set(largeText, "v1", "v2"))); + } }