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 2012/01/26 16:22:08 UTC

git commit: Allow extending composite comparator

Updated Branches:
  refs/heads/trunk 7e70f4e93 -> 0fdab63d3


Allow extending composite comparator

patch by slebresne; reviewed by zznate for CASSANDRA-3657


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

Branch: refs/heads/trunk
Commit: 0fdab63d366810c4225221624082a30c332d3d3d
Parents: 7e70f4e
Author: Sylvain Lebresne <sy...@datastax.com>
Authored: Thu Jan 26 16:20:55 2012 +0100
Committer: Sylvain Lebresne <sy...@datastax.com>
Committed: Thu Jan 26 16:20:55 2012 +0100

----------------------------------------------------------------------
 CHANGES.txt                                        |    1 +
 .../org/apache/cassandra/config/CFMetaData.java    |   26 ++++++++++----
 .../apache/cassandra/db/marshal/AbstractType.java  |   15 ++++++++
 .../org/apache/cassandra/db/marshal/BytesType.java |    8 ++++
 .../apache/cassandra/db/marshal/CompositeType.java |   24 +++++++++++++
 .../cassandra/db/marshal/DynamicCompositeType.java |   27 +++++++++++++++
 .../org/apache/cassandra/db/marshal/UTF8Type.java  |    8 ++++
 .../cassandra/db/marshal/CompositeTypeTest.java    |   10 +++++
 .../db/marshal/DynamicCompositeTypeTest.java       |   10 +++++
 9 files changed, 122 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/0fdab63d/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 53a6c67..96d41de 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -58,6 +58,7 @@
    (CASSANDRA_3559)
  * Add initial code for CQL 3.0-beta (CASSANDRA-3781)
  * Add wide row support for ColumnFamilyInputFormat (CASSANDRA-3264)
+ * Allow extending CompositeType comparator (CASSANDRA-3657)
 
 
 1.0.8

http://git-wip-us.apache.org/repos/asf/cassandra/blob/0fdab63d/src/java/org/apache/cassandra/config/CFMetaData.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/config/CFMetaData.java b/src/java/org/apache/cassandra/config/CFMetaData.java
index a82639d..c5b2633 100644
--- a/src/java/org/apache/cassandra/config/CFMetaData.java
+++ b/src/java/org/apache/cassandra/config/CFMetaData.java
@@ -158,8 +158,8 @@ public final class CFMetaData
     public final String ksName;                       // name of keyspace
     public final String cfName;                       // name of this column family
     public final ColumnFamilyType cfType;             // standard, super
-    public final AbstractType<?> comparator;          // bytes, long, timeuuid, utf8, etc.
-    public final AbstractType<?> subcolumnComparator; // like comparator, for supercolumns
+    public AbstractType<?> comparator;          // bytes, long, timeuuid, utf8, etc.
+    public AbstractType<?> subcolumnComparator; // like comparator, for supercolumns
 
     //OPTIONAL
     private String comment;                           // default none, for humans only
@@ -722,16 +722,28 @@ public final class CFMetaData
 
         if (!cf_def.column_type.equals(cfType.name()))
             throw new ConfigurationException("types do not match.");
-        if (comparator != TypeParser.parse(cf_def.comparator_type))
-            throw new ConfigurationException("comparators do not match.");
-        if (cf_def.subcomparator_type == null || cf_def.subcomparator_type.equals(""))
+
+        AbstractType<?> newComparator = TypeParser.parse(cf_def.comparator_type);
+        AbstractType<?> newSubComparator = (cf_def.subcomparator_type == null || cf_def.subcomparator_type.equals(""))
+                                         ? null
+                                         : TypeParser.parse(cf_def.subcomparator_type);
+
+        if (!newComparator.isCompatibleWith(comparator))
+            throw new ConfigurationException("comparators do not match or are not compatible.");
+        if (newSubComparator == null)
         {
             if (subcolumnComparator != null)
                 throw new ConfigurationException("subcolumncomparators do not match.");
             // else, it's null and we're good.
         }
-        else if (subcolumnComparator != TypeParser.parse(cf_def.subcomparator_type))
-            throw new ConfigurationException("subcolumncomparators do not match.");
+        else if (!newSubComparator.isCompatibleWith(subcolumnComparator))
+            throw new ConfigurationException("subcolumncomparators do not match or are note compatible.");
+
+        // TODO: this method should probably return a new CFMetaData so that
+        // 1) we can keep comparator and subcolumnComparator final
+        // 2) updates are applied atomically
+        comparator = newComparator;
+        subcolumnComparator = newSubComparator;
 
         validateMinMaxCompactionThresholds(cf_def);
 

http://git-wip-us.apache.org/repos/asf/cassandra/blob/0fdab63d/src/java/org/apache/cassandra/db/marshal/AbstractType.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/marshal/AbstractType.java b/src/java/org/apache/cassandra/db/marshal/AbstractType.java
index adc99f8..750d883 100644
--- a/src/java/org/apache/cassandra/db/marshal/AbstractType.java
+++ b/src/java/org/apache/cassandra/db/marshal/AbstractType.java
@@ -159,6 +159,21 @@ public abstract class AbstractType<T> implements Comparator<ByteBuffer>
     }
 
     /**
+     * Returns true if this comparator is compatible with the provided
+     * previous comparator, that is if previous can safely be replaced by this.
+     * A comparator cn should be compatible with a previous one cp if forall columns c1 and c2,
+     * if   cn.validate(c1) and cn.validate(c2) and cn.compare(c1, c2) == v,
+     * then cp.validate(c1) and cp.validate(c2) and cp.compare(c1, c2) == v.
+     *
+     * Note that a type should be compatible with at least itself and when in
+     * doubt, keep the default behavior of not being compatible with any other comparator!
+     */
+    public boolean isCompatibleWith(AbstractType<?> previous)
+    {
+        return this == previous;
+    }
+
+    /**
      * This must be overriden by subclasses if necessary so that for any
      * AbstractType, this == TypeParser.parse(toString()).
      *

http://git-wip-us.apache.org/repos/asf/cassandra/blob/0fdab63d/src/java/org/apache/cassandra/db/marshal/BytesType.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/marshal/BytesType.java b/src/java/org/apache/cassandra/db/marshal/BytesType.java
index 7ab70df..3c7257f 100644
--- a/src/java/org/apache/cassandra/db/marshal/BytesType.java
+++ b/src/java/org/apache/cassandra/db/marshal/BytesType.java
@@ -79,4 +79,12 @@ public class BytesType extends AbstractType<ByteBuffer>
     {
         // all bytes are legal.
     }
+
+    @Override
+    public boolean isCompatibleWith(AbstractType<?> previous)
+    {
+        // Both asciiType and utf8Type really use bytes comparison and
+        // bytesType validate everything, so it is compatible with the former.
+        return this == previous || previous == AsciiType.instance || previous == UTF8Type.instance;
+    }
 }

http://git-wip-us.apache.org/repos/asf/cassandra/blob/0fdab63d/src/java/org/apache/cassandra/db/marshal/CompositeType.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/marshal/CompositeType.java b/src/java/org/apache/cassandra/db/marshal/CompositeType.java
index 3b71000..27cfea1 100644
--- a/src/java/org/apache/cassandra/db/marshal/CompositeType.java
+++ b/src/java/org/apache/cassandra/db/marshal/CompositeType.java
@@ -108,6 +108,30 @@ public class CompositeType extends AbstractCompositeType
         return types.get(i);
     }
 
+    @Override
+    public boolean isCompatibleWith(AbstractType<?> previous)
+    {
+        if (this == previous)
+            return true;
+
+        if (!(previous instanceof CompositeType))
+            return false;
+
+        // Extending with new components is fine
+        CompositeType cp = (CompositeType)previous;
+        if (types.size() < cp.types.size())
+            return false;
+
+        for (int i = 0; i < cp.types.size(); i++)
+        {
+            AbstractType tprev = cp.types.get(i);
+            AbstractType tnew = types.get(i);
+            if (!tnew.isCompatibleWith(tprev))
+                return false;
+        }
+        return true;
+    }
+
     private static class StaticParsedComparator implements ParsedComparator
     {
         final AbstractType<?> type;

http://git-wip-us.apache.org/repos/asf/cassandra/blob/0fdab63d/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 f508758..154aaf8 100644
--- a/src/java/org/apache/cassandra/db/marshal/DynamicCompositeType.java
+++ b/src/java/org/apache/cassandra/db/marshal/DynamicCompositeType.java
@@ -200,6 +200,33 @@ public class DynamicCompositeType extends AbstractCompositeType
             return comparator;
     }
 
+    @Override
+    public boolean isCompatibleWith(AbstractType<?> previous)
+    {
+        if (this == previous)
+            return true;
+
+        if (!(previous instanceof DynamicCompositeType))
+            return false;
+
+        // Adding new aliases is fine (but removing is not)
+        // Note that modifying the type for an alias to a compatible type is
+        // *not* fine since this would deal correctly with mixed aliased/not
+        // aliased component.
+        DynamicCompositeType cp = (DynamicCompositeType)previous;
+        if (aliases.size() < cp.aliases.size())
+            return false;
+
+        for (Map.Entry<Byte, AbstractType<?>> entry : cp.aliases.entrySet())
+        {
+            AbstractType<?> tprev = entry.getValue();
+            AbstractType<?> tnew = aliases.get(entry.getKey());
+            if (tnew == null || tnew != tprev)
+                return false;
+        }
+        return true;
+    }
+
     private class DynamicParsedComparator implements ParsedComparator
     {
         final AbstractType<?> type;

http://git-wip-us.apache.org/repos/asf/cassandra/blob/0fdab63d/src/java/org/apache/cassandra/db/marshal/UTF8Type.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/marshal/UTF8Type.java b/src/java/org/apache/cassandra/db/marshal/UTF8Type.java
index e743e96..f4467e2 100644
--- a/src/java/org/apache/cassandra/db/marshal/UTF8Type.java
+++ b/src/java/org/apache/cassandra/db/marshal/UTF8Type.java
@@ -185,4 +185,12 @@ public class UTF8Type extends AbstractType<String>
             return state == State.START;
         }
     }
+
+    @Override
+    public boolean isCompatibleWith(AbstractType<?> previous)
+    {
+        // Anything that is ascii is also utf8, and they both use bytes
+        // comparison
+        return this == previous || previous == AsciiType.instance;
+    }
 }

http://git-wip-us.apache.org/repos/asf/cassandra/blob/0fdab63d/test/unit/org/apache/cassandra/db/marshal/CompositeTypeTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/db/marshal/CompositeTypeTest.java b/test/unit/org/apache/cassandra/db/marshal/CompositeTypeTest.java
index fa2e876..e32943b 100644
--- a/test/unit/org/apache/cassandra/db/marshal/CompositeTypeTest.java
+++ b/test/unit/org/apache/cassandra/db/marshal/CompositeTypeTest.java
@@ -217,6 +217,16 @@ public class CompositeTypeTest extends CleanupHelper
         catch (ConfigurationException e) {}
     }
 
+    @Test
+    public void testCompatibility() throws Exception
+    {
+        assert TypeParser.parse("CompositeType(IntegerType, BytesType)").isCompatibleWith(TypeParser.parse("CompositeType(IntegerType)"));
+        assert TypeParser.parse("CompositeType(IntegerType, BytesType)").isCompatibleWith(TypeParser.parse("CompositeType(IntegerType, BytesType)"));
+        assert TypeParser.parse("CompositeType(BytesType, BytesType)").isCompatibleWith(TypeParser.parse("CompositeType(AsciiType, BytesType)"));
+
+        assert !TypeParser.parse("CompositeType(IntegerType)").isCompatibleWith(TypeParser.parse("CompositeType(IntegerType, BytesType)"));
+        assert !TypeParser.parse("CompositeType(IntegerType)").isCompatibleWith(TypeParser.parse("CompositeType(BytesType)"));
+    }
 
     private void addColumn(RowMutation rm, ByteBuffer cname)
     {

http://git-wip-us.apache.org/repos/asf/cassandra/blob/0fdab63d/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 f3b0ec7..022b2a0 100644
--- a/test/unit/org/apache/cassandra/db/marshal/DynamicCompositeTypeTest.java
+++ b/test/unit/org/apache/cassandra/db/marshal/DynamicCompositeTypeTest.java
@@ -220,6 +220,16 @@ public class DynamicCompositeTypeTest extends CleanupHelper
         }
     }
 
+    public void testCompatibility() throws Exception
+    {
+        assert TypeParser.parse("DynamicCompositeType()").isCompatibleWith(TypeParser.parse("DynamicCompositeType()"));
+        assert TypeParser.parse("DynamicCompositeType(a => IntegerType)").isCompatibleWith(TypeParser.parse("DynamicCompositeType()"));
+        assert TypeParser.parse("DynamicCompositeType(b => BytesType, a => IntegerType)").isCompatibleWith(TypeParser.parse("DynamicCompositeType(a => IntegerType)"));
+
+        assert !TypeParser.parse("DynamicCompositeType(a => BytesType)").isCompatibleWith(TypeParser.parse("DynamicCompositeType(a => AsciiType)"));
+        assert !TypeParser.parse("DynamicCompositeType(a => BytesType)").isCompatibleWith(TypeParser.parse("DynamicCompositeType(a => BytesType, b => AsciiType)"));
+    }
+
     private void addColumn(RowMutation rm, ByteBuffer cname)
     {
         rm.add(new QueryPath(cfName, null , cname), ByteBufferUtil.EMPTY_BYTE_BUFFER, 0);