You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by sl...@apache.org on 2014/10/02 11:20:21 UTC
git commit: Fix potential exception with ReversedType in
DynamicCompositeType
Repository: cassandra
Updated Branches:
refs/heads/cassandra-2.0 7110d980c -> b93ca58d4
Fix potential exception with ReversedType in DynamicCompositeType
patch by timw; reviewed by slebresne for CASSANDRA-7898
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/b93ca58d
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/b93ca58d
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/b93ca58d
Branch: refs/heads/cassandra-2.0
Commit: b93ca58d4f364cb833dd0b836943d5603f3cfcb5
Parents: 7110d98
Author: Tim Whittington <ti...@apache.org>
Authored: Thu Oct 2 11:17:36 2014 +0200
Committer: Sylvain Lebresne <sy...@datastax.com>
Committed: Thu Oct 2 11:20:13 2014 +0200
----------------------------------------------------------------------
CHANGES.txt | 2 +
.../db/marshal/DynamicCompositeType.java | 13 +++-
.../unit/org/apache/cassandra/SchemaLoader.java | 2 +
.../db/marshal/DynamicCompositeTypeTest.java | 79 ++++++++++++++++++--
4 files changed, 90 insertions(+), 6 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cassandra/blob/b93ca58d/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index c9b4e59..f3d5998 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,6 @@
2.0.11:
+ * Fix potential exception when using ReversedType in DynamicCompositeType
+ (CASSANDRA-7898)
* Better validation of collection values (CASSANDRA-7833)
* Track min/max timestamps correctly (CASSANDRA-7969)
* Fix possible overflow while sorting CL segments for replay (CASSANDRA-7992)
http://git-wip-us.apache.org/repos/asf/cassandra/blob/b93ca58d/src/java/org/apache/cassandra/db/marshal/DynamicCompositeType.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/marshal/DynamicCompositeType.java b/src/java/org/apache/cassandra/db/marshal/DynamicCompositeType.java
index 4285d9c..cddbd1d 100644
--- a/src/java/org/apache/cassandra/db/marshal/DynamicCompositeType.java
+++ b/src/java/org/apache/cassandra/db/marshal/DynamicCompositeType.java
@@ -119,6 +119,16 @@ public class DynamicCompositeType extends AbstractCompositeType
{
AbstractType<?> comp1 = getComparator(bb1);
AbstractType<?> comp2 = getComparator(bb2);
+ AbstractType<?> rawComp = comp1;
+
+ /*
+ * If both types are ReversedType(Type), we need to compare on the wrapped type (which may differ between the two types) to avoid
+ * incompatible comparisons being made.
+ */
+ if ((comp1 instanceof ReversedType) && (comp2 instanceof ReversedType)) {
+ comp1 = ((ReversedType<?>) comp1).baseType;
+ comp2 = ((ReversedType<?>) comp2).baseType;
+ }
// Fast test if the comparator uses singleton instances
if (comp1 != comp2)
@@ -140,7 +150,8 @@ public class DynamicCompositeType extends AbstractCompositeType
// if cmp == 0, we're actually having the same type, but one that
// did not have a singleton instance. It's ok (though inefficient).
}
- return comp1;
+ // Use the raw comparator (prior to ReversedType unwrapping)
+ return rawComp;
}
protected AbstractType<?> getAndAppendComparator(int i, ByteBuffer bb, StringBuilder sb)
http://git-wip-us.apache.org/repos/asf/cassandra/blob/b93ca58d/test/unit/org/apache/cassandra/SchemaLoader.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/SchemaLoader.java b/test/unit/org/apache/cassandra/SchemaLoader.java
index e4929ee..7dea52c 100644
--- a/test/unit/org/apache/cassandra/SchemaLoader.java
+++ b/test/unit/org/apache/cassandra/SchemaLoader.java
@@ -126,6 +126,8 @@ public class SchemaLoader
Map<Byte, AbstractType<?>> aliases = new HashMap<Byte, AbstractType<?>>();
aliases.put((byte)'b', BytesType.instance);
aliases.put((byte)'t', TimeUUIDType.instance);
+ aliases.put((byte)'B', ReversedType.getInstance(BytesType.instance));
+ aliases.put((byte)'T', ReversedType.getInstance(TimeUUIDType.instance));
AbstractType<?> dynamicComposite = DynamicCompositeType.getInstance(aliases);
// these column definitions will will be applied to the jdbc utf and integer column familes respectively.
http://git-wip-us.apache.org/repos/asf/cassandra/blob/b93ca58d/test/unit/org/apache/cassandra/db/marshal/DynamicCompositeTypeTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/db/marshal/DynamicCompositeTypeTest.java b/test/unit/org/apache/cassandra/db/marshal/DynamicCompositeTypeTest.java
index f8e2fb6..763779d 100644
--- a/test/unit/org/apache/cassandra/db/marshal/DynamicCompositeTypeTest.java
+++ b/test/unit/org/apache/cassandra/db/marshal/DynamicCompositeTypeTest.java
@@ -43,7 +43,9 @@ public class DynamicCompositeTypeTest extends SchemaLoader
{
Map<Byte, AbstractType<?>> aliases = new HashMap<Byte, AbstractType<?>>();
aliases.put((byte)'b', BytesType.instance);
+ aliases.put((byte)'B', ReversedType.getInstance(BytesType.instance));
aliases.put((byte)'t', TimeUUIDType.instance);
+ aliases.put((byte)'T', ReversedType.getInstance(TimeUUIDType.instance));
comparator = DynamicCompositeType.getInstance(aliases);
}
@@ -192,6 +194,38 @@ public class DynamicCompositeTypeTest extends SchemaLoader
}
@Test
+ public void testFullRoundReversed() throws Exception
+ {
+ Keyspace keyspace = Keyspace.open("Keyspace1");
+ ColumnFamilyStore cfs = keyspace.getColumnFamilyStore(cfName);
+
+ ByteBuffer cname1 = createDynamicCompositeKey("test1", null, -1, false, true);
+ ByteBuffer cname2 = createDynamicCompositeKey("test1", uuids[0], 24, false, true);
+ ByteBuffer cname3 = createDynamicCompositeKey("test1", uuids[0], 42, false, true);
+ ByteBuffer cname4 = createDynamicCompositeKey("test2", uuids[0], -1, false, true);
+ ByteBuffer cname5 = createDynamicCompositeKey("test2", uuids[1], 42, false, true);
+
+ ByteBuffer key = ByteBufferUtil.bytes("kr");
+ RowMutation rm = new RowMutation("Keyspace1", key);
+ addColumn(rm, cname5);
+ addColumn(rm, cname1);
+ addColumn(rm, cname4);
+ addColumn(rm, cname2);
+ addColumn(rm, cname3);
+ rm.apply();
+
+ ColumnFamily cf = cfs.getColumnFamily(QueryFilter.getIdentityFilter(Util.dk("kr"), cfName, System.currentTimeMillis()));
+
+ Iterator<Column> iter = cf.getSortedColumns().iterator();
+
+ assert iter.next().name().equals(cname5);
+ assert iter.next().name().equals(cname4);
+ assert iter.next().name().equals(cname1); // null UUID < reversed value
+ assert iter.next().name().equals(cname3);
+ assert iter.next().name().equals(cname2);
+ }
+
+ @Test
public void testUncomparableColumns()
{
ByteBuffer bytes = ByteBuffer.allocate(2 + 2 + 4 + 1);
@@ -219,6 +253,34 @@ public class DynamicCompositeTypeTest extends SchemaLoader
}
}
+ @Test
+ public void testUncomparableReversedColumns()
+ {
+ ByteBuffer uuid = ByteBuffer.allocate(2 + 2 + 16 + 1);
+ uuid.putShort((short)(0x8000 | 'T'));
+ uuid.putShort((short) 16);
+ uuid.put(UUIDGen.decompose(uuids[0]));
+ uuid.put((byte) 0);
+ uuid.rewind();
+
+ ByteBuffer bytes = ByteBuffer.allocate(2 + 2 + 4 + 1);
+ bytes.putShort((short)(0x8000 | 'B'));
+ bytes.putShort((short) 4);
+ bytes.put(new byte[4]);
+ bytes.put((byte) 0);
+ bytes.rewind();
+
+ try
+ {
+ int c = comparator.compare(uuid, bytes);
+ assert c == 1 : "Expecting bytes to sort before uuid, but got " + c;
+ }
+ catch (Exception e)
+ {
+ fail("Shouldn't throw exception");
+ }
+ }
+
public void testCompatibility() throws Exception
{
assert TypeParser.parse("DynamicCompositeType()").isCompatibleWith(TypeParser.parse("DynamicCompositeType()"));
@@ -236,6 +298,13 @@ public class DynamicCompositeTypeTest extends SchemaLoader
private ByteBuffer createDynamicCompositeKey(String s, UUID uuid, int i, boolean lastIsOne)
{
+ return createDynamicCompositeKey(s, uuid, i, lastIsOne, false);
+ }
+
+ private ByteBuffer createDynamicCompositeKey(String s, UUID uuid, int i, boolean lastIsOne,
+ final boolean reversed)
+ {
+ String intType = (reversed ? "ReversedType(IntegerType)" : "IntegerType");
ByteBuffer bytes = ByteBufferUtil.bytes(s);
int totalSize = 0;
if (s != null)
@@ -246,7 +315,7 @@ public class DynamicCompositeTypeTest extends SchemaLoader
totalSize += 2 + 2 + 16 + 1;
if (i != -1)
{
- totalSize += 2 + "IntegerType".length() + 2 + 1 + 1;
+ totalSize += 2 + intType.length() + 2 + 1 + 1;
}
}
}
@@ -255,20 +324,20 @@ public class DynamicCompositeTypeTest extends SchemaLoader
if (s != null)
{
- bb.putShort((short)(0x8000 | 'b'));
+ bb.putShort((short)(0x8000 | (reversed ? 'B' : 'b')));
bb.putShort((short) bytes.remaining());
bb.put(bytes);
bb.put(uuid == null && lastIsOne ? (byte)1 : (byte)0);
if (uuid != null)
{
- bb.putShort((short)(0x8000 | 't'));
+ bb.putShort((short)(0x8000 | (reversed ? 'T' : 't')));
bb.putShort((short) 16);
bb.put(UUIDGen.decompose(uuid));
bb.put(i == -1 && lastIsOne ? (byte)1 : (byte)0);
if (i != -1)
{
- bb.putShort((short) "IntegerType".length());
- bb.put(ByteBufferUtil.bytes("IntegerType"));
+ bb.putShort((short) intType.length());
+ bb.put(ByteBufferUtil.bytes(intType));
// We are putting a byte only because our test use ints that fit in a byte *and* IntegerType.fromString() will
// return something compatible (i.e, putting a full int here would break 'fromStringTest')
bb.putShort((short) 1);