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 <sylv...@datastax.com> Authored: Thu Jan 26 16:20:55 2012 +0100 Committer: Sylvain Lebresne <sylv...@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);