cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From slebre...@apache.org
Subject git commit: Fix potential exception with ReversedType in DynamicCompositeType
Date Thu, 02 Oct 2014 09:20:21 GMT
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 <timw@apache.org>
Authored: Thu Oct 2 11:17:36 2014 +0200
Committer: Sylvain Lebresne <sylvain@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);


Mime
View raw message