Author: xedin
Date: Wed Aug 17 22:17:03 2011
New Revision: 1158939
URL: http://svn.apache.org/viewvc?rev=1158939&view=rev
Log:
Validate that column names in column_metadata does not equal to key_alias on
create/update of the ColumnFamily and CQL 'ALTER' statement.
patch by Pavel Yaskevich; reviewed by Jonathan Ellis for CASSANDRA-3036
Modified:
cassandra/branches/cassandra-0.8/CHANGES.txt
cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cql/AlterTableStatement.java
cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cql/CreateColumnFamilyStatement.java
cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cql/QueryProcessor.java
cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/thrift/ThriftValidation.java
cassandra/branches/cassandra-0.8/test/system/test_cql.py
cassandra/branches/cassandra-0.8/test/unit/org/apache/cassandra/thrift/ThriftValidationTest.java
Modified: cassandra/branches/cassandra-0.8/CHANGES.txt
URL:
http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/CHANGES.txt?rev=1158939&r1=1158938&r2=1158939&view=diff
==============================================================================
--- cassandra/branches/cassandra-0.8/CHANGES.txt (original)
+++ cassandra/branches/cassandra-0.8/CHANGES.txt Wed Aug 17 22:17:03 2011
@@ -13,7 +13,8 @@
(rows containing nothing but expired tombstones) (CASSANDRA-3039)
* work around native memory leak in com.sun.management.GarbageCollectorMXBean
(CASSANDRA-2868)
-
+ * validate that column names in column_metadata does not equal to key_alias
+ on create/update of the ColumnFamily and CQL 'ALTER' statement
(CASSANDRA-3036)
0.8.4
* include files-to-be-streamed in StreamInSession.getSources (CASSANDRA-2972)
Modified:
cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cql/AlterTableStatement.java
URL:
http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cql/AlterTableStatement.java?rev=1158939&r1=1158938&r2=1158939&view=diff
==============================================================================
---
cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cql/AlterTableStatement.java
(original)
+++
cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cql/AlterTableStatement.java
Wed Aug 17 22:17:03 2011
@@ -65,10 +65,15 @@ public class AlterTableStatement
switch (oType)
{
case ADD:
+ if (cfDef.key_alias != null &&
cfDef.key_alias.equals(columnName))
+ throw new InvalidRequestException("Invalid column name: "
+ + this.columnName
+ + ", because it equals
to key_alias.");
+
cfDef.column_metadata.add(new ColumnDefinition(columnName,
-
TypeParser.parse(validator),
- null,
-
null).deflate());
+
TypeParser.parse(validator),
+ null,
+
null).deflate());
break;
case ALTER:
Modified:
cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cql/CreateColumnFamilyStatement.java
URL:
http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cql/CreateColumnFamilyStatement.java?rev=1158939&r1=1158938&r2=1158939&view=diff
==============================================================================
---
cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cql/CreateColumnFamilyStatement.java
(original)
+++
cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cql/CreateColumnFamilyStatement.java
Wed Aug 17 22:17:03 2011
@@ -169,6 +169,28 @@ public class CreateColumnFamilyStatement
throw new InvalidRequestException("You must specify a PRIMARY
KEY");
else if (keyValidator.size() > 1)
throw new InvalidRequestException("You may only specify one
PRIMARY KEY");
+
+ AbstractType<?> comparator;
+
+ try
+ {
+ comparator = getComparator();
+ }
+ catch (ConfigurationException e)
+ {
+ throw new InvalidRequestException(e.toString());
+ }
+
+ for (Map.Entry<Term, String> column : columns.entrySet())
+ {
+ ByteBuffer name = column.getKey().getByteBuffer(comparator);
+
+ if (keyAlias != null && keyAlias.equals(name))
+ throw new InvalidRequestException("Invalid column name: "
+ + column.getKey().getText()
+ + ", because it equals to
the key_alias.");
+
+ }
}
/** Map a column name to a validator for its value */
@@ -215,7 +237,7 @@ public class CreateColumnFamilyStatement
{
try
{
- ByteBuffer columnName = col.getKey().getByteBuffer(comparator);
+ ByteBuffer columnName =
comparator.fromString(col.getKey().getText());
String validatorClassName =
comparators.containsKey(col.getValue()) ? comparators.get(col.getValue()) :
col.getValue();
AbstractType<?> validator =
TypeParser.parse(validatorClassName);
columnDefs.put(columnName, new ColumnDefinition(columnName,
validator, null, null));
@@ -230,7 +252,26 @@ public class CreateColumnFamilyStatement
return columnDefs;
}
-
+
+ /* If not comparator/validator is not specified, default to text
(BytesType is the wrong default for CQL
+ * since it uses hex terms). If the value specified is not found in the
comparators map, assume the user
+ * knows what they are doing (a custom comparator/validator for example),
and pass it on as-is.
+ */
+
+ private AbstractType<?> getComparator() throws ConfigurationException
+ {
+ return
TypeParser.parse((comparators.get(getPropertyString(KW_COMPARATOR, "text")) !=
null)
+ ?
comparators.get(getPropertyString(KW_COMPARATOR, "text"))
+ : getPropertyString(KW_COMPARATOR, "text"));
+ }
+
+ private AbstractType<?> getValidator() throws ConfigurationException
+ {
+ return
TypeParser.parse((comparators.get(getPropertyString(KW_DEFAULTVALIDATION,
"text")) != null)
+ ?
comparators.get(getPropertyString(KW_DEFAULTVALIDATION, "text"))
+ : getPropertyString(KW_DEFAULTVALIDATION,
"text"));
+ }
+
/**
* Returns a CFMetaData instance based on the parameters parsed from this
* <code>CREATE</code> statement, or defaults where applicable.
@@ -246,17 +287,7 @@ public class CreateColumnFamilyStatement
CFMetaData newCFMD;
try
{
- /* If not comparator/validator is not specified, default to text
(BytesType is the wrong default for CQL
- * since it uses hex terms). If the value specified is not found
in the comparators map, assume the user
- * knows what they are doing (a custom comparator/validator for
example), and pass it on as-is.
- */
- String comparatorString =
(comparators.get(getPropertyString(KW_COMPARATOR, "text")) != null)
- ?
comparators.get(getPropertyString(KW_COMPARATOR, "text"))
- : getPropertyString(KW_COMPARATOR,
"text");
- String validatorString =
(comparators.get(getPropertyString(KW_DEFAULTVALIDATION, "text")) != null)
- ?
comparators.get(getPropertyString(KW_DEFAULTVALIDATION, "text"))
- : getPropertyString(KW_DEFAULTVALIDATION,
"text");
- AbstractType<?> comparator = TypeParser.parse(comparatorString);
+ AbstractType<?> comparator = getComparator();
newCFMD = new CFMetaData(keyspace,
name,
@@ -270,7 +301,7 @@ public class CreateColumnFamilyStatement
.readRepairChance(getPropertyDouble(KW_READREPAIRCHANCE,
CFMetaData.DEFAULT_READ_REPAIR_CHANCE))
.replicateOnWrite(getPropertyBoolean(KW_REPLICATEONWRITE,
CFMetaData.DEFAULT_REPLICATE_ON_WRITE))
.gcGraceSeconds(getPropertyInt(KW_GCGRACESECONDS,
CFMetaData.DEFAULT_GC_GRACE_SECONDS))
- .defaultValidator(TypeParser.parse(validatorString))
+ .defaultValidator(getValidator())
.minCompactionThreshold(getPropertyInt(KW_MINCOMPACTIONTHRESHOLD,
CFMetaData.DEFAULT_MIN_COMPACTION_THRESHOLD))
.maxCompactionThreshold(getPropertyInt(KW_MAXCOMPACTIONTHRESHOLD,
CFMetaData.DEFAULT_MAX_COMPACTION_THRESHOLD))
.rowCacheSavePeriod(getPropertyInt(KW_ROWCACHESAVEPERIODSECS,
CFMetaData.DEFAULT_ROW_CACHE_SAVE_PERIOD_IN_SECONDS))
Modified:
cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cql/QueryProcessor.java
URL:
http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cql/QueryProcessor.java?rev=1158939&r1=1158938&r2=1158939&view=diff
==============================================================================
---
cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cql/QueryProcessor.java
(original)
+++
cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/cql/QueryProcessor.java
Wed Aug 17 22:17:03 2011
@@ -845,8 +845,6 @@ public class QueryProcessor
case ALTER_TABLE:
AlterTableStatement alterTable = (AlterTableStatement)
statement.statement;
- System.out.println(alterTable);
-
validateColumnFamily(keyspace, alterTable.columnFamily);
clientState.hasColumnFamilyAccess(alterTable.columnFamily,
Permission.WRITE);
validateSchemaAgreement();
Modified:
cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/thrift/ThriftValidation.java
URL:
http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/thrift/ThriftValidation.java?rev=1158939&r1=1158938&r2=1158939&view=diff
==============================================================================
---
cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/thrift/ThriftValidation.java
(original)
+++
cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/thrift/ThriftValidation.java
Wed Aug 17 22:17:03 2011
@@ -553,22 +553,6 @@ public class ThriftValidation
}
}
- if (cf_def.key_alias != null)
- {
- if (!cf_def.key_alias.hasRemaining())
- throw new InvalidRequestException("key_alias may not be
empty");
- try
- {
- // it's hard to use a key in a select statement if we
can't type it.
- // for now let's keep it simple and require ascii.
- AsciiType.instance.validate(cf_def.key_alias);
- }
- catch (MarshalException e)
- {
- throw new InvalidRequestException("Key aliases must be
ascii");
- }
- }
-
ColumnFamilyType cfType =
ColumnFamilyType.create(cf_def.column_type);
if (cfType == null)
throw new InvalidRequestException("invalid column type " +
cf_def.column_type);
@@ -586,6 +570,18 @@ public class ThriftValidation
? TypeParser.parse(cf_def.comparator_type)
:
TypeParser.parse(cf_def.subcomparator_type);
+ if (cf_def.key_alias != null)
+ {
+ // check if any of the columns has name equal to the
cf.key_alias
+ for (ColumnDef columnDef : cf_def.column_metadata)
+ {
+ if (cf_def.key_alias.equals(columnDef.name))
+ throw new InvalidRequestException("Invalid column
name: "
+ +
AsciiType.instance.compose(cf_def.key_alias)
+ + ", because it
equals to the key_alias.");
+ }
+ }
+
// initialize a set of names NOT in the CF under consideration
Set<String> indexNames = new HashSet<String>();
for (ColumnFamilyStore cfs : ColumnFamilyStore.all())
Modified: cassandra/branches/cassandra-0.8/test/system/test_cql.py
URL:
http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/test/system/test_cql.py?rev=1158939&r1=1158938&r2=1158939&view=diff
==============================================================================
--- cassandra/branches/cassandra-0.8/test/system/test_cql.py (original)
+++ cassandra/branches/cassandra-0.8/test/system/test_cql.py Wed Aug 17
22:17:03 2011
@@ -477,6 +477,9 @@ class TestCql(ThriftTester):
# Missing primary key
assert_raises(cql.ProgrammingError, cursor.execute, "CREATE
COLUMNFAMILY NewCf2")
+ # column name should not match key alias
+ assert_raises(cql.ProgrammingError, cursor.execute, "CREATE
COLUMNFAMILY NewCf2 (id 'utf8' primary key, id int)")
+
# Too many primary keys
assert_raises(cql.ProgrammingError,
cursor.execute,
@@ -1140,7 +1143,7 @@ class TestCql(ThriftTester):
cursor.execute("USE AlterTableKS;")
cursor.execute("""
- CREATE COLUMNFAMILY NewCf1 (KEY varint PRIMARY KEY) WITH
default_validation = ascii;
+ CREATE COLUMNFAMILY NewCf1 (id_key varint PRIMARY KEY) WITH
default_validation = ascii;
""")
# TODO: temporary (until this can be done with CQL).
@@ -1204,6 +1207,12 @@ class TestCql(ThriftTester):
assert_raises(cql.ProgrammingError,
cursor.execute,
"ALTER COLUMNFAMILY NewCf1 DROP name")
+
+ # should raise error when column name equals key alias
+ assert_raises(cql.ProgrammingError,
+ cursor.execute,
+ "ALTER COLUMNFAMILY NewCf1 ADD id_key utf8")
+
def test_counter_column_support(self):
"update statement should be able to work with counter columns"
Modified:
cassandra/branches/cassandra-0.8/test/unit/org/apache/cassandra/thrift/ThriftValidationTest.java
URL:
http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/test/unit/org/apache/cassandra/thrift/ThriftValidationTest.java?rev=1158939&r1=1158938&r2=1158939&view=diff
==============================================================================
---
cassandra/branches/cassandra-0.8/test/unit/org/apache/cassandra/thrift/ThriftValidationTest.java
(original)
+++
cassandra/branches/cassandra-0.8/test/unit/org/apache/cassandra/thrift/ThriftValidationTest.java
Wed Aug 17 22:17:03 2011
@@ -21,10 +21,17 @@ package org.apache.cassandra.thrift;
*/
+import org.apache.cassandra.config.CFMetaData;
+import org.apache.cassandra.config.DatabaseDescriptor;
+import org.apache.cassandra.db.marshal.AsciiType;
+import org.apache.cassandra.db.marshal.UTF8Type;
+
import org.junit.Test;
import org.apache.cassandra.CleanupHelper;
+import java.util.concurrent.Callable;
+
public class ThriftValidationTest extends CleanupHelper
{
@Test(expected=InvalidRequestException.class)
@@ -38,4 +45,46 @@ public class ThriftValidationTest extend
{
ThriftValidation.validateColumnFamily("Keyspace1", "Counter1", true);
}
+
+ @Test
+ public void testColumnNameEqualToKeyAlias()
+ {
+ CFMetaData metaData = DatabaseDescriptor.getCFMetaData("Keyspace1",
"Standard1");
+ CfDef newMetadata = CFMetaData.convertToThrift(metaData);
+
+ boolean gotException = false;
+
+ // add a key_alias = "id"
+ newMetadata.setKey_alias(AsciiType.instance.decompose("id"));
+
+ // should not throw IRE here
+ try
+ {
+ ThriftValidation.validateCfDef(newMetadata, metaData);
+ }
+ catch (InvalidRequestException e)
+ {
+ gotException = true;
+ }
+
+ assert !gotException : "got unexpected InvalidRequestException";
+
+ // add a column with name = "id"
+ newMetadata.addToColumn_metadata(new
ColumnDef(UTF8Type.instance.decompose("id"),
+
"org.apache.cassandra.db.marshal.UTF8Type"));
+
+
+ gotException = false;
+
+ try
+ {
+ ThriftValidation.validateCfDef(newMetadata, metaData);
+ }
+ catch (InvalidRequestException e)
+ {
+ gotException = true;
+ }
+
+ assert gotException : "expected InvalidRequestException but not
received.";
+ }
}
