cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From slebre...@apache.org
Subject cassandra git commit: Validate ALTER for involved views as well as to the base table
Date Thu, 08 Oct 2015 10:06:59 GMT
Repository: cassandra
Updated Branches:
  refs/heads/cassandra-3.0 4f4918f6f -> d76d3a5d5


Validate ALTER for involved views as well as to the base table

patch by carlyeks; reviewed by slebresne for CASSANDRA-10424


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

Branch: refs/heads/cassandra-3.0
Commit: d76d3a5d59447a1584f49a2ca5a120d9580a4c1f
Parents: 4f4918f
Author: Carl Yeksigian <carl@apache.org>
Authored: Wed Sep 30 13:38:38 2015 -0400
Committer: Sylvain Lebresne <sylvain@datastax.com>
Committed: Thu Oct 8 12:06:48 2015 +0200

----------------------------------------------------------------------
 .../org/apache/cassandra/config/CFMetaData.java |  2 +-
 .../cql3/statements/AlterTableStatement.java    | 99 +++++++++++---------
 .../org/apache/cassandra/cql3/ViewTest.java     | 76 +++++++++++++++
 .../cql3/validation/operations/AlterTest.java   | 14 +++
 4 files changed, 148 insertions(+), 43 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/d76d3a5d/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 00ca704..0387060 100644
--- a/src/java/org/apache/cassandra/config/CFMetaData.java
+++ b/src/java/org/apache/cassandra/config/CFMetaData.java
@@ -786,7 +786,7 @@ public final class CFMetaData
             throw new ConfigurationException("types do not match.");
 
         if (!cfm.comparator.isCompatibleWith(comparator))
-            throw new ConfigurationException(String.format("Column family comparators do
not match or are not compatible (found %s; expected %s).", cfm.comparator.getClass().getSimpleName(),
comparator.getClass().getSimpleName()));
+            throw new ConfigurationException(String.format("Column family comparators do
not match or are not compatible (found %s; expected %s).", cfm.comparator.toString(), comparator.toString()));
     }
 
 

http://git-wip-us.apache.org/repos/asf/cassandra/blob/d76d3a5d/src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java b/src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java
index c410f10..879f618 100644
--- a/src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java
+++ b/src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java
@@ -32,6 +32,7 @@ import org.apache.cassandra.db.Keyspace;
 import org.apache.cassandra.db.marshal.AbstractType;
 import org.apache.cassandra.db.marshal.CollectionType;
 import org.apache.cassandra.db.marshal.CounterColumnType;
+import org.apache.cassandra.db.marshal.ReversedType;
 import org.apache.cassandra.db.view.View;
 import org.apache.cassandra.exceptions.*;
 import org.apache.cassandra.schema.IndexMetadata;
@@ -186,55 +187,25 @@ public class AlterTableStatement extends SchemaAlteringStatement
                 if (def == null)
                     throw new InvalidRequestException(String.format("Column %s was not found
in table %s", columnName, columnFamily()));
 
-                AbstractType<?> validatorType = validator.getType();
-                switch (def.kind)
-                {
-                    case PARTITION_KEY:
-                        if (validatorType instanceof CounterColumnType)
-                            throw new InvalidRequestException(String.format("counter type
is not supported for PRIMARY KEY part %s", columnName));
-
-                        AbstractType<?> currentType = cfm.getKeyValidatorAsClusteringComparator().subtype(def.position());
-                        if (!validatorType.isValueCompatibleWith(currentType))
-                            throw new ConfigurationException(String.format("Cannot change
%s from type %s to type %s: types are incompatible.",
-                                                                           columnName,
-                                                                           currentType.asCQL3Type(),
-                                                                           validator));
-                        break;
-                    case CLUSTERING:
-                        AbstractType<?> oldType = cfm.comparator.subtype(def.position());
-                        // Note that CFMetaData.validateCompatibility already validate the
change we're about to do. However, the error message it
-                        // sends is a bit cryptic for a CQL3 user, so validating here for
a sake of returning a better error message
-                        // Do note that we need isCompatibleWith here, not just isValueCompatibleWith.
-                        if (!validatorType.isCompatibleWith(oldType))
-                            throw new ConfigurationException(String.format("Cannot change
%s from type %s to type %s: types are not order-compatible.",
-                                                                           columnName,
-                                                                           oldType.asCQL3Type(),
-                                                                           validator));
-
-                        break;
-                    case REGULAR:
-                    case STATIC:
-                        // Thrift allows to change a column validator so CFMetaData.validateCompatibility
will let it slide
-                        // if we change to an incompatible type (contrarily to the comparator
case). But we don't want to
-                        // allow it for CQL3 (see #5882) so validating it explicitly here.
We only care about value compatibility
-                        // though since we won't compare values (except when there is an
index, but that is validated by
-                        // ColumnDefinition already).
-                        if (!validatorType.isValueCompatibleWith(def.type))
-                            throw new ConfigurationException(String.format("Cannot change
%s from type %s to type %s: types are incompatible.",
-                                                                           columnName,
-                                                                           def.type.asCQL3Type(),
-                                                                           validator));
-                        break;
-                }
+                AbstractType<?> validatorType = def.isReversedType() && !validator.getType().isReversed()
+                                                ? ReversedType.getInstance(validator.getType())
+                                                : validator.getType();
+                validateAlter(cfm, def, validatorType);
                 // In any case, we update the column definition
                 cfm.addOrReplaceColumnDefinition(def.withNewType(validatorType));
 
-                // We have to alter the schema of the view table as well; it doesn't affect
the definition however
+                // We also have to validate the view types here. If we have a view which
includes a column as part of
+                // the clustering key, we need to make sure that it is indeed compatible.
                 for (ViewDefinition view : views)
                 {
                     if (!view.includes(columnName)) continue;
                     ViewDefinition viewCopy = view.copy();
-                    viewCopy.metadata.addOrReplaceColumnDefinition(def.withNewType(validatorType));
+                    ColumnDefinition viewDef = view.metadata.getColumnDefinition(columnName);
+                    AbstractType viewType = viewDef.isReversedType() && !validator.getType().isReversed()
+                                            ? ReversedType.getInstance(validator.getType())
+                                            : validator.getType();
+                    validateAlter(view.metadata, viewDef, viewType);
+                    viewCopy.metadata.addOrReplaceColumnDefinition(viewDef.withNewType(viewType));
 
                     if (viewUpdates == null)
                         viewUpdates = new ArrayList<>();
@@ -361,6 +332,50 @@ public class AlterTableStatement extends SchemaAlteringStatement
         return true;
     }
 
+    private static void validateAlter(CFMetaData cfm, ColumnDefinition def, AbstractType<?>
validatorType)
+    {
+        switch (def.kind)
+        {
+            case PARTITION_KEY:
+                if (validatorType instanceof CounterColumnType)
+                    throw new InvalidRequestException(String.format("counter type is not
supported for PRIMARY KEY part %s", def.name));
+
+                AbstractType<?> currentType = cfm.getKeyValidatorAsClusteringComparator().subtype(def.position());
+                if (!validatorType.isValueCompatibleWith(currentType))
+                    throw new ConfigurationException(String.format("Cannot change %s from
type %s to type %s: types are incompatible.",
+                                                                   def.name,
+                                                                   currentType.asCQL3Type(),
+                                                                   validatorType.asCQL3Type()));
+                break;
+            case CLUSTERING:
+                AbstractType<?> oldType = cfm.comparator.subtype(def.position());
+                // Note that CFMetaData.validateCompatibility already validate the change
we're about to do. However, the error message it
+                // sends is a bit cryptic for a CQL3 user, so validating here for a sake
of returning a better error message
+                // Do note that we need isCompatibleWith here, not just isValueCompatibleWith.
+                if (!validatorType.isCompatibleWith(oldType))
+                {
+                    throw new ConfigurationException(String.format("Cannot change %s from
type %s to type %s: types are not order-compatible.",
+                                                                   def.name,
+                                                                   oldType.asCQL3Type(),
+                                                                   validatorType.asCQL3Type()));
+                }
+                break;
+            case REGULAR:
+            case STATIC:
+                // Thrift allows to change a column validator so CFMetaData.validateCompatibility
will let it slide
+                // if we change to an incompatible type (contrarily to the comparator case).
But we don't want to
+                // allow it for CQL3 (see #5882) so validating it explicitly here. We only
care about value compatibility
+                // though since we won't compare values (except when there is an index, but
that is validated by
+                // ColumnDefinition already).
+                if (!validatorType.isValueCompatibleWith(def.type))
+                    throw new ConfigurationException(String.format("Cannot change %s from
type %s to type %s: types are incompatible.",
+                                                                   def.name,
+                                                                   def.type.asCQL3Type(),
+                                                                   validatorType.asCQL3Type()));
+                break;
+        }
+    }
+
     public String toString()
     {
         return String.format("AlterTableStatement(name=%s, type=%s, column=%s, validator=%s)",

http://git-wip-us.apache.org/repos/asf/cassandra/blob/d76d3a5d/test/unit/org/apache/cassandra/cql3/ViewTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/cql3/ViewTest.java b/test/unit/org/apache/cassandra/cql3/ViewTest.java
index 5d65115..55e7e1f 100644
--- a/test/unit/org/apache/cassandra/cql3/ViewTest.java
+++ b/test/unit/org/apache/cassandra/cql3/ViewTest.java
@@ -1456,4 +1456,80 @@ public class ViewTest extends CQLTester
         ResultSet mvRows = executeNet(protocolVersion, "SELECT a, b FROM mv1");
         assertRowsNet(protocolVersion, mvRows, row(1, 1));
     }
+
+    @Test
+    public void testAlterTable() throws Throwable
+    {
+        createTable("CREATE TABLE %s (" +
+                    "a int," +
+                    "b text," +
+                    "PRIMARY KEY (a, b))");
+
+        executeNet(protocolVersion, "USE " + keyspace());
+
+        createView("mv1", "CREATE MATERIALIZED VIEW %s AS SELECT * FROM %%s WHERE a IS NOT
NULL AND b IS NOT NULL PRIMARY KEY (b, a)");
+
+        alterTable("ALTER TABLE %s ALTER b TYPE blob");
+    }
+
+    @Test
+    public void testAlterReversedTypeBaseTable() throws Throwable
+    {
+        createTable("CREATE TABLE %s (" +
+                    "a int," +
+                    "b text," +
+                    "PRIMARY KEY (a, b))" +
+                    "WITH CLUSTERING ORDER BY (b DESC)");
+
+        executeNet(protocolVersion, "USE " + keyspace());
+
+        createView("mv1", "CREATE MATERIALIZED VIEW %s AS SELECT * FROM %%s WHERE a IS NOT
NULL AND b IS NOT NULL PRIMARY KEY (a, b) WITH CLUSTERING ORDER BY (b ASC)");
+
+        alterTable("ALTER TABLE %s ALTER b TYPE blob");
+    }
+
+    @Test
+    public void testAlterReversedTypeViewTable() throws Throwable
+    {
+        createTable("CREATE TABLE %s (" +
+                    "a int," +
+                    "b text," +
+                    "PRIMARY KEY (a, b))");
+
+        executeNet(protocolVersion, "USE " + keyspace());
+
+        createView("mv1", "CREATE MATERIALIZED VIEW %s AS SELECT * FROM %%s WHERE a IS NOT
NULL AND b IS NOT NULL PRIMARY KEY (a, b) WITH CLUSTERING ORDER BY (b DESC)");
+
+        alterTable("ALTER TABLE %s ALTER b TYPE blob");
+    }
+
+    @Test
+    public void testAlterClusteringViewTable() throws Throwable
+    {
+        createTable("CREATE TABLE %s (" +
+                    "a int," +
+                    "b text," +
+                    "PRIMARY KEY (a))");
+
+        executeNet(protocolVersion, "USE " + keyspace());
+
+        createView("mv1", "CREATE MATERIALIZED VIEW %s AS SELECT * FROM %%s WHERE a IS NOT
NULL AND b IS NOT NULL PRIMARY KEY (a, b) WITH CLUSTERING ORDER BY (b DESC)");
+
+        alterTable("ALTER TABLE %s ALTER b TYPE blob");
+    }
+
+    @Test
+    public void testAlterViewTableValue() throws Throwable
+    {
+        createTable("CREATE TABLE %s (" +
+                    "a int," +
+                    "b int," +
+                    "PRIMARY KEY (a))");
+
+        executeNet(protocolVersion, "USE " + keyspace());
+
+        createView("mv1", "CREATE MATERIALIZED VIEW %s AS SELECT * FROM %%s WHERE a IS NOT
NULL AND b IS NOT NULL PRIMARY KEY (a, b) WITH CLUSTERING ORDER BY (b DESC)");
+
+        assertInvalid("ALTER TABLE %s ALTER b TYPE blob");
+    }
 }

http://git-wip-us.apache.org/repos/asf/cassandra/blob/d76d3a5d/test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java b/test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java
index b7f814b..a56ccc9 100644
--- a/test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java
+++ b/test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java
@@ -273,6 +273,20 @@ public class AlterTest extends CQLTester
                                            "ALTER TABLE %s WITH compression = { 'class' :
'SnappyCompressor', 'chunk_length_kb' : 32 , 'chunk_length_in_kb' : 32 };");
     }
 
+    @Test
+    public void testAlterType() throws Throwable
+    {
+        createTable("CREATE TABLE %s (id text PRIMARY KEY, content text);");
+        alterTable("ALTER TABLE %s ALTER content TYPE blob");
+
+        createTable("CREATE TABLE %s (pk int, ck text, value blob, PRIMARY KEY (pk, ck))
WITH CLUSTERING ORDER BY (ck DESC)");
+        alterTable("ALTER TABLE %s ALTER ck TYPE blob");
+
+        createTable("CREATE TABLE %s (pk int, ck int, value blob, PRIMARY KEY (pk, ck))");
+        assertThrowsConfigurationException("Cannot change value from type blob to type text:
types are incompatible.",
+                                           "ALTER TABLE %s ALTER value TYPE TEXT;");
+    }
+
     private void assertThrowsConfigurationException(String errorMsg, String alterStmt) throws
Throwable
     {
         try


Mime
View raw message