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 <c...@apache.org> Authored: Wed Sep 30 13:38:38 2015 -0400 Committer: Sylvain Lebresne <sylv...@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
