cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sn...@apache.org
Subject [1/2] cassandra git commit: Prevent ALTER TYPE from creating circular references
Date Fri, 18 Sep 2015 07:54:23 GMT
Repository: cassandra
Updated Branches:
  refs/heads/cassandra-2.2 93745c05c -> 1de63e951


Prevent ALTER TYPE from creating circular references

patch by Robert Stupp; reviewed by Sylvain Lebresne for CASSANDRA-10339


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

Branch: refs/heads/cassandra-2.2
Commit: 98c4a7cd02022c55f174da16d074e3fc62203aac
Parents: 0b8b67b
Author: Robert Stupp <snazy@snazy.de>
Authored: Fri Sep 18 09:48:17 2015 +0200
Committer: Robert Stupp <snazy@snazy.de>
Committed: Fri Sep 18 09:48:17 2015 +0200

----------------------------------------------------------------------
 CHANGES.txt                                     |  1 +
 .../cql3/statements/AlterTypeStatement.java     |  6 ++-
 .../cassandra/db/marshal/AbstractType.java      |  8 ++++
 .../cassandra/db/marshal/CompositeType.java     | 11 +++++
 .../apache/cassandra/db/marshal/ListType.java   |  6 +++
 .../apache/cassandra/db/marshal/MapType.java    |  6 +++
 .../cassandra/db/marshal/ReversedType.java      |  5 ++
 .../apache/cassandra/db/marshal/SetType.java    |  6 +++
 .../apache/cassandra/db/marshal/TupleType.java  | 13 +++++-
 .../cql3/validation/entities/UserTypesTest.java | 49 ++++++++++++++++++++
 10 files changed, 108 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/98c4a7cd/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index e2dd83a..166106d 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
 2.1.10
+ * Prevent ALTER TYPE from creating circular references (CASSANDRA-10339)
  * Fix cache handling of 2i and base tables (CASSANDRA-10155, 10359)
  * Fix NPE in nodetool compactionhistory (CASSANDRA-9758)
  * (Pig) support BulkOutputFormat as a URL parameter (CASSANDRA-7410)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/98c4a7cd/src/java/org/apache/cassandra/cql3/statements/AlterTypeStatement.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/statements/AlterTypeStatement.java b/src/java/org/apache/cassandra/cql3/statements/AlterTypeStatement.java
index 74fafd6..6459e6b 100644
--- a/src/java/org/apache/cassandra/cql3/statements/AlterTypeStatement.java
+++ b/src/java/org/apache/cassandra/cql3/statements/AlterTypeStatement.java
@@ -294,9 +294,13 @@ public abstract class AlterTypeStatement extends SchemaAlteringStatement
             newNames.addAll(toUpdate.fieldNames());
             newNames.add(fieldName.bytes);
 
+            AbstractType<?> addType = type.prepare(keyspace()).getType();
+            if (addType.references(toUpdate))
+                throw new InvalidRequestException(String.format("Cannot add new field %s
of type %s to type %s as this would create a circular reference", fieldName, type, name));
+
             List<AbstractType<?>> newTypes = new ArrayList<>(toUpdate.size()
+ 1);
             newTypes.addAll(toUpdate.fieldTypes());
-            newTypes.add(type.prepare(keyspace()).getType());
+            newTypes.add(addType);
 
             return new UserType(toUpdate.keyspace, toUpdate.name, newNames, newTypes);
         }

http://git-wip-us.apache.org/repos/asf/cassandra/blob/98c4a7cd/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 863cd47..bcb33dc 100644
--- a/src/java/org/apache/cassandra/db/marshal/AbstractType.java
+++ b/src/java/org/apache/cassandra/db/marshal/AbstractType.java
@@ -262,6 +262,14 @@ public abstract class AbstractType<T> implements Comparator<ByteBuffer>
     }
 
     /**
+     * Checks whether this type or any of the types this type contains references the given
type.
+     */
+    public boolean references(AbstractType<?> check)
+    {
+        return this.equals(check);
+    }
+
+    /**
      * 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/98c4a7cd/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 9ee9fb3..f8ac22d 100644
--- a/src/java/org/apache/cassandra/db/marshal/CompositeType.java
+++ b/src/java/org/apache/cassandra/db/marshal/CompositeType.java
@@ -115,6 +115,17 @@ public class CompositeType extends AbstractCompositeType
         this.types = ImmutableList.copyOf(types);
     }
 
+    @Override
+    public boolean references(AbstractType<?> check)
+    {
+        if (super.references(check))
+            return true;
+        for (AbstractType<?> type : types)
+            if (type.references(check))
+                return true;
+        return false;
+    }
+
     protected AbstractType<?> getComparator(int i, ByteBuffer bb)
     {
         try

http://git-wip-us.apache.org/repos/asf/cassandra/blob/98c4a7cd/src/java/org/apache/cassandra/db/marshal/ListType.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/marshal/ListType.java b/src/java/org/apache/cassandra/db/marshal/ListType.java
index 510a526..c2f36c0 100644
--- a/src/java/org/apache/cassandra/db/marshal/ListType.java
+++ b/src/java/org/apache/cassandra/db/marshal/ListType.java
@@ -69,6 +69,12 @@ public class ListType<T> extends CollectionType<List<T>>
         this.isMultiCell = isMultiCell;
     }
 
+    @Override
+    public boolean references(AbstractType<?> check)
+    {
+        return super.references(check) || elements.references(check);
+    }
+
     public AbstractType<T> getElementsType()
     {
         return elements;

http://git-wip-us.apache.org/repos/asf/cassandra/blob/98c4a7cd/src/java/org/apache/cassandra/db/marshal/MapType.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/marshal/MapType.java b/src/java/org/apache/cassandra/db/marshal/MapType.java
index 64f3e2a..0fb545d 100644
--- a/src/java/org/apache/cassandra/db/marshal/MapType.java
+++ b/src/java/org/apache/cassandra/db/marshal/MapType.java
@@ -70,6 +70,12 @@ public class MapType<K, V> extends CollectionType<Map<K, V>>
         this.isMultiCell = isMultiCell;
     }
 
+    @Override
+    public boolean references(AbstractType<?> check)
+    {
+        return super.references(check) || keys.references(check) || values.references(check);
+    }
+
     public AbstractType<K> getKeysType()
     {
         return keys;

http://git-wip-us.apache.org/repos/asf/cassandra/blob/98c4a7cd/src/java/org/apache/cassandra/db/marshal/ReversedType.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/marshal/ReversedType.java b/src/java/org/apache/cassandra/db/marshal/ReversedType.java
index 1323dc6..0389a32 100644
--- a/src/java/org/apache/cassandra/db/marshal/ReversedType.java
+++ b/src/java/org/apache/cassandra/db/marshal/ReversedType.java
@@ -109,6 +109,11 @@ public class ReversedType<T> extends AbstractType<T>
         return baseType.getSerializer();
     }
 
+    public boolean references(AbstractType<?> check)
+    {
+        return super.references(check) || baseType.references(check);
+    }
+
     @Override
     public String toString()
     {

http://git-wip-us.apache.org/repos/asf/cassandra/blob/98c4a7cd/src/java/org/apache/cassandra/db/marshal/SetType.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/marshal/SetType.java b/src/java/org/apache/cassandra/db/marshal/SetType.java
index e10f2a1..b635208 100644
--- a/src/java/org/apache/cassandra/db/marshal/SetType.java
+++ b/src/java/org/apache/cassandra/db/marshal/SetType.java
@@ -64,6 +64,12 @@ public class SetType<T> extends CollectionType<Set<T>>
         this.isMultiCell = isMultiCell;
     }
 
+    @Override
+    public boolean references(AbstractType<?> check)
+    {
+        return super.references(check) || elements.references(check);
+    }
+
     public AbstractType<T> getElementsType()
     {
         return elements;

http://git-wip-us.apache.org/repos/asf/cassandra/blob/98c4a7cd/src/java/org/apache/cassandra/db/marshal/TupleType.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/marshal/TupleType.java b/src/java/org/apache/cassandra/db/marshal/TupleType.java
index e07319b..dbce0db 100644
--- a/src/java/org/apache/cassandra/db/marshal/TupleType.java
+++ b/src/java/org/apache/cassandra/db/marshal/TupleType.java
@@ -23,8 +23,6 @@ import java.util.List;
 
 import com.google.common.base.Objects;
 
-import org.apache.cassandra.exceptions.InvalidRequestException;
-
 import org.apache.cassandra.cql3.CQL3Type;
 import org.apache.cassandra.exceptions.ConfigurationException;
 import org.apache.cassandra.exceptions.SyntaxException;
@@ -54,6 +52,17 @@ public class TupleType extends AbstractType<ByteBuffer>
         return new TupleType(types);
     }
 
+    @Override
+    public boolean references(AbstractType<?> check)
+    {
+        if (super.references(check))
+            return true;
+        for (AbstractType<?> type : types)
+            if (type.references(check))
+                return true;
+        return false;
+    }
+
     public AbstractType<?> type(int i)
     {
         return types.get(i);

http://git-wip-us.apache.org/repos/asf/cassandra/blob/98c4a7cd/test/unit/org/apache/cassandra/cql3/validation/entities/UserTypesTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/cql3/validation/entities/UserTypesTest.java b/test/unit/org/apache/cassandra/cql3/validation/entities/UserTypesTest.java
index ee647d6..d96abdb 100644
--- a/test/unit/org/apache/cassandra/cql3/validation/entities/UserTypesTest.java
+++ b/test/unit/org/apache/cassandra/cql3/validation/entities/UserTypesTest.java
@@ -386,4 +386,53 @@ public class UserTypesTest extends CQLTester
         execute("ALTER TYPE " + keyspace() + "." + typeName + " ADD foomap map <int,text>");
         execute("INSERT INTO %s (key, data) VALUES (1, {fooint: 1, fooset: {'2'}, foomap:
{3 : 'bar'}})");
     }
+
+    @Test
+    public void testCircularReferences() throws Throwable
+    {
+        String type1 = createType("CREATE TYPE %s (foo int)");
+
+        String typeX = createType("CREATE TYPE %s (bar frozen<" + typeWithKs(type1) +
">)");
+        assertInvalidMessage("would create a circular reference", "ALTER TYPE " + typeWithKs(type1)
+ " ADD needs_to_fail frozen<" + typeWithKs(typeX) + '>');
+
+        typeX = createType("CREATE TYPE %s (bar frozen<list<" + typeWithKs(type1) +
">>)");
+        assertInvalidMessage("would create a circular reference", "ALTER TYPE " + typeWithKs(type1)
+ " ADD needs_to_fail frozen<" + typeWithKs(typeX) + '>');
+
+        typeX = createType("CREATE TYPE %s (bar frozen<set<" + typeWithKs(type1) +
">>)");
+        assertInvalidMessage("would create a circular reference", "ALTER TYPE " + typeWithKs(type1)
+ " ADD needs_to_fail frozen<" + typeWithKs(typeX) + '>');
+
+        typeX = createType("CREATE TYPE %s (bar frozen<map<text, " + typeWithKs(type1)
+ ">>)");
+        assertInvalidMessage("would create a circular reference", "ALTER TYPE " + typeWithKs(type1)
+ " ADD needs_to_fail frozen<" + typeWithKs(typeX) + '>');
+
+        typeX = createType("CREATE TYPE %s (bar frozen<map<" + typeWithKs(type1) +
", text>>)");
+        assertInvalidMessage("would create a circular reference", "ALTER TYPE " + typeWithKs(type1)
+ " ADD needs_to_fail frozen<" + typeWithKs(typeX) + '>');
+
+        //
+
+        String type2 = createType("CREATE TYPE %s (foo frozen<" + typeWithKs(type1) +
">)");
+
+        typeX = createType("CREATE TYPE %s (bar frozen<" + keyspace() + '.' + type2 +
">)");
+        assertInvalidMessage("would create a circular reference", "ALTER TYPE " + typeWithKs(type1)
+ " ADD needs_to_fail frozen<" + typeWithKs(typeX) + '>');
+
+        typeX = createType("CREATE TYPE %s (bar frozen<list<" + keyspace() + '.' +
type2 + ">>)");
+        assertInvalidMessage("would create a circular reference", "ALTER TYPE " + typeWithKs(type1)
+ " ADD needs_to_fail frozen<" + typeWithKs(typeX) + '>');
+
+        typeX = createType("CREATE TYPE %s (bar frozen<set<" + keyspace() + '.' + type2
+ ">>)");
+        assertInvalidMessage("would create a circular reference", "ALTER TYPE " + typeWithKs(type1)
+ " ADD needs_to_fail frozen<" + typeWithKs(typeX) + '>');
+
+        typeX = createType("CREATE TYPE %s (bar frozen<map<text, " + keyspace() + '.'
+ type2 + ">>)");
+        assertInvalidMessage("would create a circular reference", "ALTER TYPE " + typeWithKs(type1)
+ " ADD needs_to_fail frozen<" + typeWithKs(typeX) + '>');
+
+        typeX = createType("CREATE TYPE %s (bar frozen<map<" + keyspace() + '.' + type2
+ ", text>>)");
+        assertInvalidMessage("would create a circular reference", "ALTER TYPE " + typeWithKs(type1)
+ " ADD needs_to_fail frozen<" + typeWithKs(typeX) + '>');
+
+        //
+
+        assertInvalidMessage("would create a circular reference", "ALTER TYPE " + typeWithKs(type1)
+ " ADD needs_to_fail frozen<list<" + typeWithKs(type1) + ">>");
+    }
+
+    private String typeWithKs(String type1)
+    {
+        return keyspace() + '.' + type1;
+    }
 }


Mime
View raw message