Repository: cassandra
Updated Branches:
refs/heads/trunk 7a7fb8e36 -> 9515fca76
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/trunk
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
|