You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by sl...@apache.org on 2014/04/02 14:02:41 UTC
[2/4] git commit: Fix Cassandra 2.0.x validates Thrift columns
incorrectly
Fix Cassandra 2.0.x validates Thrift columns incorrectly
patch by thobbs and slebresne; reviewed by thobbs and slebresne for CASSANDRA-6892
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/b42feea6
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/b42feea6
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/b42feea6
Branch: refs/heads/trunk
Commit: b42feea6432905946083e0e287b3545a5799e4a7
Parents: 79da4a6
Author: Sylvain Lebresne <sy...@datastax.com>
Authored: Wed Apr 2 11:58:00 2014 +0200
Committer: Sylvain Lebresne <sy...@datastax.com>
Committed: Wed Apr 2 11:58:00 2014 +0200
----------------------------------------------------------------------
CHANGES.txt | 1 +
.../org/apache/cassandra/config/CFMetaData.java | 32 ++++++----
.../cassandra/config/ColumnDefinition.java | 9 +++
.../org/apache/cassandra/config/Schema.java | 14 ----
.../apache/cassandra/cql/QueryProcessor.java | 4 +-
.../apache/cassandra/cql/SelectStatement.java | 5 --
.../apache/cassandra/cql/UpdateStatement.java | 7 +-
src/java/org/apache/cassandra/db/Column.java | 10 +--
.../cassandra/thrift/ThriftValidation.java | 7 +-
.../apache/cassandra/tools/SSTableExport.java | 2 +-
.../apache/cassandra/tools/SSTableImport.java | 2 +-
.../unit/org/apache/cassandra/SchemaLoader.java | 18 +++++-
.../unit/org/apache/cassandra/db/ScrubTest.java | 67 +++++++++++++++++++-
.../cassandra/thrift/ThriftValidationTest.java | 53 ++++++++++++++--
.../cassandra/tools/SSTableExportTest.java | 42 ++++++++++--
15 files changed, 204 insertions(+), 69 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cassandra/blob/b42feea6/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 8bfc8b9..9d27ebf 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -38,6 +38,7 @@
* Track liveRatio per-memtable, not per-CF (CASSANDRA-6945)
* Make sure upgradesstables keeps sstable level (CASSANDRA-6958)
* Fix LIMT with static columns (CASSANDRA-6956)
+ * Fix clash with CQL column name in thrift validation (CASSANDRA-6892)
Merged from 1.2:
* Add UNLOGGED, COUNTER options to BATCH documentation (CASSANDRA-6816)
* add extra SSL cipher suites (CASSANDRA-6613)
http://git-wip-us.apache.org/repos/asf/cassandra/blob/b42feea6/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 1f25cea..be18d1b 100644
--- a/src/java/org/apache/cassandra/config/CFMetaData.java
+++ b/src/java/org/apache/cassandra/config/CFMetaData.java
@@ -853,16 +853,13 @@ public final class CFMetaData
.toHashCode();
}
- public AbstractType<?> getValueValidator(ByteBuffer column)
- {
- return getValueValidator(getColumnDefinition(column));
- }
-
- public AbstractType<?> getValueValidator(ColumnDefinition columnDefinition)
+ /**
+ * Like getColumnDefinitionFromColumnName, the argument must be an internal column/cell name.
+ */
+ public AbstractType<?> getValueValidatorFromColumnName(ByteBuffer columnName)
{
- return columnDefinition == null
- ? defaultValidator
- : columnDefinition.getValidator();
+ ColumnDefinition def = getColumnDefinitionFromColumnName(columnName);
+ return def == null ? defaultValidator : def.getValidator();
}
/** applies implicit defaults to cf definition. useful in updates */
@@ -1212,7 +1209,7 @@ public final class CFMetaData
{
CompositeType composite = (CompositeType)comparator;
ByteBuffer[] components = composite.split(columnName);
- for (ColumnDefinition def : column_metadata.values())
+ for (ColumnDefinition def : regularAndStaticColumns())
{
ByteBuffer toCompare;
if (def.componentIndex == null)
@@ -1233,12 +1230,19 @@ public final class CFMetaData
}
else
{
- return column_metadata.get(columnName);
+ ColumnDefinition def = column_metadata.get(columnName);
+ // It's possible that the def is a PRIMARY KEY or COMPACT_VALUE one in case a concrete cell
+ // name conflicts with a CQL column name, which can happen in 2 cases:
+ // 1) because the user inserted a cell through Thrift that conflicts with a default "alias" used
+ // by CQL for thrift tables (see #6892).
+ // 2) for COMPACT STORAGE tables with a single utf8 clustering column, the cell name can be anything,
+ // including a CQL column name (without this being a problem).
+ // In any case, this is fine, this just mean that columnDefinition is not the ColumnDefinition we are
+ // looking for.
+ return def != null && def.isPartOfCellName() ? def : null;
}
}
-
-
public ColumnDefinition getColumnDefinitionForIndex(String indexName)
{
for (ColumnDefinition def : column_metadata.values())
@@ -1855,7 +1859,7 @@ public final class CFMetaData
if (column_metadata.get(to) != null)
throw new InvalidRequestException(String.format("Cannot rename column %s to %s in keyspace %s; another column of that name already exist", strFrom, strTo, cfName));
- if (def.type == ColumnDefinition.Type.REGULAR || def.type == ColumnDefinition.Type.STATIC)
+ if (def.isPartOfCellName())
{
throw new InvalidRequestException(String.format("Cannot rename non PRIMARY KEY part %s", strFrom));
}
http://git-wip-us.apache.org/repos/asf/cassandra/blob/b42feea6/src/java/org/apache/cassandra/config/ColumnDefinition.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/config/ColumnDefinition.java b/src/java/org/apache/cassandra/config/ColumnDefinition.java
index 11340e7..1223db8 100644
--- a/src/java/org/apache/cassandra/config/ColumnDefinition.java
+++ b/src/java/org/apache/cassandra/config/ColumnDefinition.java
@@ -193,6 +193,15 @@ public class ColumnDefinition
return thriftDefs;
}
+ /**
+ * Whether the name of this definition is serialized in the cell nane, i.e. whether
+ * it's not just a non-stored CQL metadata.
+ */
+ public boolean isPartOfCellName()
+ {
+ return type == Type.REGULAR || type == Type.STATIC;
+ }
+
public ColumnDef toThrift()
{
ColumnDef cd = new ColumnDef();
http://git-wip-us.apache.org/repos/asf/cassandra/blob/b42feea6/src/java/org/apache/cassandra/config/Schema.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/config/Schema.java b/src/java/org/apache/cassandra/config/Schema.java
index f0a49dc..0da65ce 100644
--- a/src/java/org/apache/cassandra/config/Schema.java
+++ b/src/java/org/apache/cassandra/config/Schema.java
@@ -235,20 +235,6 @@ public class Schema
}
/**
- * Get value validator for specific column
- *
- * @param ksName The keyspace name
- * @param cfName The ColumnFamily name
- * @param column The name of the column
- *
- * @return value validator specific to the column or default (per-cf) one
- */
- public AbstractType<?> getValueValidator(String ksName, String cfName, ByteBuffer column)
- {
- return getCFMetaData(ksName, cfName).getValueValidator(column);
- }
-
- /**
* Get metadata about keyspace by its name
*
* @param keyspaceName The name of the keyspace
http://git-wip-us.apache.org/repos/asf/cassandra/blob/b42feea6/src/java/org/apache/cassandra/cql/QueryProcessor.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql/QueryProcessor.java b/src/java/org/apache/cassandra/cql/QueryProcessor.java
index f54f5ef..f160374 100644
--- a/src/java/org/apache/cassandra/cql/QueryProcessor.java
+++ b/src/java/org/apache/cassandra/cql/QueryProcessor.java
@@ -190,7 +190,7 @@ public class QueryProcessor
{
// Left and right side of relational expression encoded according to comparator/validator.
ByteBuffer entity = columnRelation.getEntity().getByteBuffer(metadata.comparator, variables);
- ByteBuffer value = columnRelation.getValue().getByteBuffer(select.getValueValidator(metadata.ksName, entity), variables);
+ ByteBuffer value = columnRelation.getValue().getByteBuffer(metadata.getValueValidatorFromColumnName(entity), variables);
expressions.add(new IndexExpression(entity,
IndexOperator.valueOf(columnRelation.operator().toString()),
@@ -326,7 +326,7 @@ public class QueryProcessor
throws InvalidRequestException
{
validateColumnName(name);
- AbstractType<?> validator = metadata.getValueValidator(name);
+ AbstractType<?> validator = metadata.getValueValidatorFromColumnName(name);
try
{
http://git-wip-us.apache.org/repos/asf/cassandra/blob/b42feea6/src/java/org/apache/cassandra/cql/SelectStatement.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql/SelectStatement.java b/src/java/org/apache/cassandra/cql/SelectStatement.java
index 2314b73..1126738 100644
--- a/src/java/org/apache/cassandra/cql/SelectStatement.java
+++ b/src/java/org/apache/cassandra/cql/SelectStatement.java
@@ -177,11 +177,6 @@ public class SelectStatement
return Schema.instance.getComparator(keyspace, columnFamily);
}
- public AbstractType<?> getValueValidator(String keyspace, ByteBuffer column)
- {
- return Schema.instance.getValueValidator(keyspace, columnFamily, column);
- }
-
public List<Relation> getClauseRelations()
{
return clause.getClauseRelations();
http://git-wip-us.apache.org/repos/asf/cassandra/blob/b42feea6/src/java/org/apache/cassandra/cql/UpdateStatement.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql/UpdateStatement.java b/src/java/org/apache/cassandra/cql/UpdateStatement.java
index 0e1250f..f99f5c2 100644
--- a/src/java/org/apache/cassandra/cql/UpdateStatement.java
+++ b/src/java/org/apache/cassandra/cql/UpdateStatement.java
@@ -193,7 +193,7 @@ public class UpdateStatement extends AbstractModification
if (hasCounterColumn)
throw new InvalidRequestException("Mix of commutative and non-commutative operations is not allowed.");
- ByteBuffer colValue = op.a.getByteBuffer(getValueValidator(keyspace, colName),variables);
+ ByteBuffer colValue = op.a.getByteBuffer(metadata.getValueValidatorFromColumnName(colName),variables);
validateColumn(metadata, colName, colValue);
rm.add(columnFamily,
@@ -282,11 +282,6 @@ public class UpdateStatement extends AbstractModification
return Schema.instance.getComparator(keyspace, columnFamily);
}
- public AbstractType<?> getValueValidator(String keyspace, ByteBuffer column)
- {
- return Schema.instance.getValueValidator(keyspace, columnFamily, column);
- }
-
public List<Term> getColumnNames()
{
return columnNames;
http://git-wip-us.apache.org/repos/asf/cassandra/blob/b42feea6/src/java/org/apache/cassandra/db/Column.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/Column.java b/src/java/org/apache/cassandra/db/Column.java
index b0d22fb..72cbae1 100644
--- a/src/java/org/apache/cassandra/db/Column.java
+++ b/src/java/org/apache/cassandra/db/Column.java
@@ -298,15 +298,7 @@ public class Column implements OnDiskAtom
public void validateFields(CFMetaData metadata) throws MarshalException
{
validateName(metadata);
- CFDefinition cfdef = metadata.getCfDef();
-
- // If this is a CQL table, we need to pull out the CQL column name to look up the correct column type.
- // (Note that COMPACT composites are handled by validateName, above.)
- ByteBuffer internalName = (cfdef.isComposite && !cfdef.isCompact)
- ? ((CompositeType) metadata.comparator).extractLastComponent(name)
- : name;
-
- AbstractType<?> valueValidator = metadata.getValueValidator(internalName);
+ AbstractType<?> valueValidator = metadata.getValueValidatorFromColumnName(name);
if (valueValidator != null)
valueValidator.validate(value());
}
http://git-wip-us.apache.org/repos/asf/cassandra/blob/b42feea6/src/java/org/apache/cassandra/thrift/ThriftValidation.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/thrift/ThriftValidation.java b/src/java/org/apache/cassandra/thrift/ThriftValidation.java
index 2b435ff..b387871 100644
--- a/src/java/org/apache/cassandra/thrift/ThriftValidation.java
+++ b/src/java/org/apache/cassandra/thrift/ThriftValidation.java
@@ -443,10 +443,9 @@ public class ThriftValidation
if (!column.isSetTimestamp())
throw new org.apache.cassandra.exceptions.InvalidRequestException("Column timestamp is required");
- ColumnDefinition columnDef = metadata.getColumnDefinitionFromColumnName(column.name);
try
{
- AbstractType<?> validator = metadata.getValueValidator(columnDef);
+ AbstractType<?> validator = metadata.getValueValidatorFromColumnName(column.name);
if (validator != null)
validator.validate(column.value);
}
@@ -466,7 +465,7 @@ public class ThriftValidation
if (!Keyspace.open(metadata.ksName).getColumnFamilyStore(metadata.cfName).indexManager.validate(asDBColumn(column)))
throw new org.apache.cassandra.exceptions.InvalidRequestException(String.format("Can't index column value of size %d for index %s in CF %s of KS %s",
column.value.remaining(),
- columnDef.getIndexName(),
+ metadata.getColumnDefinitionFromColumnName(column.name).getIndexName(),
metadata.cfName,
metadata.ksName));
}
@@ -597,7 +596,7 @@ public class ThriftValidation
if (expression.value.remaining() > 0xFFFF)
throw new org.apache.cassandra.exceptions.InvalidRequestException("Index expression values may not be larger than 64K");
- AbstractType<?> valueValidator = Schema.instance.getValueValidator(metadata.ksName, metadata.cfName, expression.column_name);
+ AbstractType<?> valueValidator = metadata.getValueValidatorFromColumnName(expression.column_name);
try
{
valueValidator.validate(expression.value);
http://git-wip-us.apache.org/repos/asf/cassandra/blob/b42feea6/src/java/org/apache/cassandra/tools/SSTableExport.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/tools/SSTableExport.java b/src/java/org/apache/cassandra/tools/SSTableExport.java
index 1568b00..197585b 100644
--- a/src/java/org/apache/cassandra/tools/SSTableExport.java
+++ b/src/java/org/apache/cassandra/tools/SSTableExport.java
@@ -187,7 +187,7 @@ public class SSTableExport
}
else
{
- AbstractType<?> validator = cfMetaData.getValueValidator(cfMetaData.getColumnDefinitionFromColumnName(name));
+ AbstractType<?> validator = cfMetaData.getValueValidatorFromColumnName(name);
serializedColumn.add(validator.getString(value));
}
serializedColumn.add(column.timestamp());
http://git-wip-us.apache.org/repos/asf/cassandra/blob/b42feea6/src/java/org/apache/cassandra/tools/SSTableImport.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/tools/SSTableImport.java b/src/java/org/apache/cassandra/tools/SSTableImport.java
index 6e0e9a7..11bfc81 100644
--- a/src/java/org/apache/cassandra/tools/SSTableImport.java
+++ b/src/java/org/apache/cassandra/tools/SSTableImport.java
@@ -161,7 +161,7 @@ public class SSTableImport
}
else
{
- value = stringAsType((String) fields.get(1), meta.getValueValidator(meta.getColumnDefinitionFromColumnName(name)));
+ value = stringAsType((String) fields.get(1), meta.getValueValidatorFromColumnName(name));
}
}
}
http://git-wip-us.apache.org/repos/asf/cassandra/blob/b42feea6/test/unit/org/apache/cassandra/SchemaLoader.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/SchemaLoader.java b/test/unit/org/apache/cassandra/SchemaLoader.java
index 058e1e3..daff0de 100644
--- a/test/unit/org/apache/cassandra/SchemaLoader.java
+++ b/test/unit/org/apache/cassandra/SchemaLoader.java
@@ -173,6 +173,11 @@ public class SchemaLoader
IntegerType.instance,
null),
new CFMetaData(ks1,
+ "StandardLong3",
+ st,
+ LongType.instance,
+ null),
+ new CFMetaData(ks1,
"Counter1",
st,
bytes,
@@ -210,7 +215,18 @@ public class SchemaLoader
.compactionStrategyOptions(leveledOptions),
standardCFMD(ks1, "legacyleveled")
.compactionStrategyClass(LeveledCompactionStrategy.class)
- .compactionStrategyOptions(leveledOptions)));
+ .compactionStrategyOptions(leveledOptions),
+ standardCFMD(ks1, "UUIDKeys").keyValidator(UUIDType.instance),
+ new CFMetaData(ks1,
+ "MixedTypes",
+ st,
+ LongType.instance,
+ null).keyValidator(UUIDType.instance).defaultValidator(BooleanType.instance),
+ new CFMetaData(ks1,
+ "MixedTypesComposite",
+ st,
+ composite,
+ null).keyValidator(composite).defaultValidator(BooleanType.instance)));
// Keyspace 2
schema.add(KSMetaData.testMetadata(ks2,
http://git-wip-us.apache.org/repos/asf/cassandra/blob/b42feea6/test/unit/org/apache/cassandra/db/ScrubTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/db/ScrubTest.java b/test/unit/org/apache/cassandra/db/ScrubTest.java
index 08dd435..23e9381 100644
--- a/test/unit/org/apache/cassandra/db/ScrubTest.java
+++ b/test/unit/org/apache/cassandra/db/ScrubTest.java
@@ -23,16 +23,22 @@ package org.apache.cassandra.db;
import java.io.*;
import java.util.Collections;
import java.util.HashSet;
+import java.util.Iterator;
import java.util.List;
import java.util.Set;
import java.util.concurrent.ExecutionException;
+import org.apache.cassandra.cql3.QueryProcessor;
import org.apache.cassandra.db.compaction.OperationType;
+import org.apache.cassandra.exceptions.RequestExecutionException;
+import org.apache.cassandra.utils.UUIDGen;
import org.apache.commons.lang3.StringUtils;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.apache.cassandra.OrderedJUnit4ClassRunner;
+import org.apache.cassandra.cql3.QueryProcessor;
+import org.apache.cassandra.cql3.UntypedResultSet;
import org.apache.cassandra.SchemaLoader;
import org.apache.cassandra.Util;
import org.apache.cassandra.config.CFMetaData;
@@ -272,4 +278,63 @@ public class ScrubTest extends SchemaLoader
cfs.forceBlockingFlush();
}
-}
\ No newline at end of file
+ @Test
+ public void testScrubColumnValidation() throws InterruptedException, RequestExecutionException, ExecutionException
+ {
+ QueryProcessor.process("CREATE TABLE \"Keyspace1\".test_compact_static_columns (a bigint, b timeuuid, c boolean static, d text, PRIMARY KEY (a, b))", ConsistencyLevel.ONE);
+
+ Keyspace keyspace = Keyspace.open("Keyspace1");
+ ColumnFamilyStore cfs = keyspace.getColumnFamilyStore("test_compact_static_columns");
+
+ QueryProcessor.processInternal("INSERT INTO \"Keyspace1\".test_compact_static_columns (a, b, c, d) VALUES (123, c3db07e8-b602-11e3-bc6b-e0b9a54a6d93, true, 'foobar')");
+ cfs.forceBlockingFlush();
+ CompactionManager.instance.performScrub(cfs, false);
+ }
+
+ /**
+ * Tests CASSANDRA-6892 (key aliases being used improperly for validation)
+ */
+ @Test
+ public void testColumnNameEqualToDefaultKeyAlias() throws ExecutionException, InterruptedException
+ {
+ Keyspace keyspace = Keyspace.open("Keyspace1");
+ ColumnFamilyStore cfs = keyspace.getColumnFamilyStore("UUIDKeys");
+
+ ColumnFamily cf = TreeMapBackedSortedColumns.factory.create("Keyspace1", "UUIDKeys");
+ cf.addColumn(column(CFMetaData.DEFAULT_KEY_ALIAS, "not a uuid", 1L));
+ RowMutation rm = new RowMutation("Keyspace1", ByteBufferUtil.bytes(UUIDGen.getTimeUUID()), cf);
+ rm.applyUnsafe();
+ cfs.forceBlockingFlush();
+ CompactionManager.instance.performScrub(cfs, false);
+
+ assertEquals(1, cfs.getSSTables().size());
+ }
+
+ /**
+ * For CASSANDRA-6892 too, check that for a compact table with one cluster column, we can insert whatever
+ * we want as value for the clustering column, including something that would conflict with a CQL column definition.
+ */
+ @Test
+ public void testValidationCompactStorage() throws Exception
+ {
+ QueryProcessor.process("CREATE TABLE \"Keyspace1\".test_compact_dynamic_columns (a int, b text, c text, PRIMARY KEY (a, b)) WITH COMPACT STORAGE", ConsistencyLevel.ONE);
+
+ Keyspace keyspace = Keyspace.open("Keyspace1");
+ ColumnFamilyStore cfs = keyspace.getColumnFamilyStore("test_compact_dynamic_columns");
+
+ QueryProcessor.processInternal("INSERT INTO \"Keyspace1\".test_compact_dynamic_columns (a, b, c) VALUES (0, 'a', 'foo')");
+ QueryProcessor.processInternal("INSERT INTO \"Keyspace1\".test_compact_dynamic_columns (a, b, c) VALUES (0, 'b', 'bar')");
+ QueryProcessor.processInternal("INSERT INTO \"Keyspace1\".test_compact_dynamic_columns (a, b, c) VALUES (0, 'c', 'boo')");
+ cfs.forceBlockingFlush();
+ CompactionManager.instance.performScrub(cfs, true);
+
+ // Scrub is silent, but it will remove broken records. So reading everything back to make sure nothing to "scrubbed away"
+ UntypedResultSet rs = QueryProcessor.processInternal("SELECT * FROM \"Keyspace1\".test_compact_dynamic_columns");
+ assertEquals(3, rs.size());
+
+ Iterator<UntypedResultSet.Row> iter = rs.iterator();
+ assertEquals("foo", iter.next().getString("c"));
+ assertEquals("bar", iter.next().getString("c"));
+ assertEquals("boo", iter.next().getString("c"));
+ }
+}
http://git-wip-us.apache.org/repos/asf/cassandra/blob/b42feea6/test/unit/org/apache/cassandra/thrift/ThriftValidationTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/thrift/ThriftValidationTest.java b/test/unit/org/apache/cassandra/thrift/ThriftValidationTest.java
index fdf0ebb..0e8bbb8 100644
--- a/test/unit/org/apache/cassandra/thrift/ThriftValidationTest.java
+++ b/test/unit/org/apache/cassandra/thrift/ThriftValidationTest.java
@@ -20,17 +20,22 @@ package org.apache.cassandra.thrift;
*
*/
+import org.apache.cassandra.db.marshal.LongType;
+import org.apache.cassandra.exceptions.*;
import org.junit.Test;
import org.apache.cassandra.SchemaLoader;
import org.apache.cassandra.config.*;
import org.apache.cassandra.db.marshal.AsciiType;
-import org.apache.cassandra.db.marshal.UTF8Type;
-import org.apache.cassandra.exceptions.ConfigurationException;
import org.apache.cassandra.locator.LocalStrategy;
import org.apache.cassandra.locator.NetworkTopologyStrategy;
import org.apache.cassandra.utils.ByteBufferUtil;
+import java.util.Arrays;
+
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertEquals;
+
public class ThriftValidationTest extends SchemaLoader
{
@Test(expected=org.apache.cassandra.exceptions.InvalidRequestException.class)
@@ -46,7 +51,7 @@ public class ThriftValidationTest extends SchemaLoader
}
@Test
- public void testColumnNameEqualToKeyAlias()
+ public void testColumnNameEqualToKeyAlias() throws org.apache.cassandra.exceptions.InvalidRequestException
{
CFMetaData metaData = Schema.instance.getCFMetaData("Keyspace1", "Standard1");
CFMetaData newMetadata = metaData.clone();
@@ -57,7 +62,7 @@ public class ThriftValidationTest extends SchemaLoader
// should not throw IRE here
try
{
- newMetadata.addColumnDefinition(ColumnDefinition.partitionKeyDef(AsciiType.instance.decompose("id"), UTF8Type.instance, null));
+ newMetadata.addColumnDefinition(ColumnDefinition.partitionKeyDef(AsciiType.instance.decompose("id"), LongType.instance, null));
newMetadata.validate();
}
catch (ConfigurationException e)
@@ -73,7 +78,7 @@ public class ThriftValidationTest extends SchemaLoader
// add a column with name = "id"
try
{
- newMetadata.addColumnDefinition(ColumnDefinition.regularDef(ByteBufferUtil.bytes("id"), UTF8Type.instance, null));
+ newMetadata.addColumnDefinition(ColumnDefinition.regularDef(ByteBufferUtil.bytes("id"), LongType.instance, null));
newMetadata.validate();
}
catch (ConfigurationException e)
@@ -82,6 +87,44 @@ public class ThriftValidationTest extends SchemaLoader
}
assert gotException : "expected ConfigurationException but not received.";
+
+ // make sure the key alias does not affect validation of columns with the same name (CASSANDRA-6892)
+ Column column = new Column(ByteBufferUtil.bytes("id"));
+ column.setValue(ByteBufferUtil.bytes("not a long"));
+ column.setTimestamp(1234);
+ ThriftValidation.validateColumnData(newMetadata, column, false);
+ }
+
+ @Test
+ public void testColumnNameEqualToDefaultKeyAlias() throws org.apache.cassandra.exceptions.InvalidRequestException
+ {
+ CFMetaData metaData = Schema.instance.getCFMetaData("Keyspace1", "UUIDKeys");
+ ColumnDefinition definition = metaData.getColumnDefinition(ByteBufferUtil.bytes(CFMetaData.DEFAULT_KEY_ALIAS));
+ assertNotNull(definition);
+ assertEquals(ColumnDefinition.Type.PARTITION_KEY, definition.type);
+
+ // make sure the key alias does not affect validation of columns with the same name (CASSANDRA-6892)
+ Column column = new Column(ByteBufferUtil.bytes(CFMetaData.DEFAULT_KEY_ALIAS));
+ column.setValue(ByteBufferUtil.bytes("not a uuid"));
+ column.setTimestamp(1234);
+ ThriftValidation.validateColumnData(metaData, column, false);
+
+ IndexExpression expression = new IndexExpression(ByteBufferUtil.bytes(CFMetaData.DEFAULT_KEY_ALIAS), IndexOperator.EQ, ByteBufferUtil.bytes("a"));
+ ThriftValidation.validateFilterClauses(metaData, Arrays.asList(expression));
+ }
+
+ @Test
+ public void testColumnNameEqualToDefaultColumnAlias() throws org.apache.cassandra.exceptions.InvalidRequestException
+ {
+ CFMetaData metaData = Schema.instance.getCFMetaData("Keyspace1", "StandardLong3");
+ ColumnDefinition definition = metaData.getColumnDefinition(ByteBufferUtil.bytes(CFMetaData.DEFAULT_COLUMN_ALIAS + 1));
+ assertNotNull(definition);
+
+ // make sure the column alias does not affect validation of columns with the same name (CASSANDRA-6892)
+ Column column = new Column(ByteBufferUtil.bytes(CFMetaData.DEFAULT_COLUMN_ALIAS + 1));
+ column.setValue(ByteBufferUtil.bytes("not a long"));
+ column.setTimestamp(1234);
+ ThriftValidation.validateColumnData(metaData, column, false);
}
@Test
http://git-wip-us.apache.org/repos/asf/cassandra/blob/b42feea6/test/unit/org/apache/cassandra/tools/SSTableExportTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/tools/SSTableExportTest.java b/test/unit/org/apache/cassandra/tools/SSTableExportTest.java
index 47aa2c8..d0ab6a2 100644
--- a/test/unit/org/apache/cassandra/tools/SSTableExportTest.java
+++ b/test/unit/org/apache/cassandra/tools/SSTableExportTest.java
@@ -18,6 +18,7 @@
*/
package org.apache.cassandra.tools;
+import static org.apache.cassandra.Util.column;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.apache.cassandra.io.sstable.SSTableUtils.tempSSTableFile;
@@ -35,12 +36,8 @@ import java.util.SortedSet;
import org.apache.cassandra.SchemaLoader;
import org.apache.cassandra.Util;
-import org.apache.cassandra.db.Column;
-import org.apache.cassandra.db.ColumnFamily;
-import org.apache.cassandra.db.CounterColumn;
-import org.apache.cassandra.db.DeletionInfo;
-import org.apache.cassandra.db.ExpiringColumn;
-import org.apache.cassandra.db.TreeMapBackedSortedColumns;
+import org.apache.cassandra.config.CFMetaData;
+import org.apache.cassandra.db.*;
import org.apache.cassandra.db.filter.QueryFilter;
import org.apache.cassandra.db.marshal.UTF8Type;
import org.apache.cassandra.io.sstable.Descriptor;
@@ -48,6 +45,7 @@ import org.apache.cassandra.io.sstable.SSTableReader;
import org.apache.cassandra.io.sstable.SSTableWriter;
import org.apache.cassandra.utils.ByteBufferUtil;
import org.apache.cassandra.utils.FBUtilities;
+import org.apache.cassandra.utils.UUIDGen;
import org.json.simple.JSONArray;
import org.json.simple.JSONObject;
import org.json.simple.JSONValue;
@@ -338,4 +336,36 @@ public class SSTableExportTest extends SchemaLoader
assertEquals("column value did not match", ByteBufferUtil.bytes("val1"), hexToBytes((String) col2.get(1)));
}
+
+ /**
+ * Tests CASSANDRA-6892 (key aliases being used improperly for validation)
+ */
+ @Test
+ public void testColumnNameEqualToDefaultKeyAlias() throws IOException, ParseException
+ {
+ File tempSS = tempSSTableFile("Keyspace1", "UUIDKeys");
+ ColumnFamily cfamily = TreeMapBackedSortedColumns.factory.create("Keyspace1", "UUIDKeys");
+ SSTableWriter writer = new SSTableWriter(tempSS.getPath(), 2);
+
+ // Add a row
+ cfamily.addColumn(column(CFMetaData.DEFAULT_KEY_ALIAS, "not a uuid", 1L));
+ writer.append(Util.dk(ByteBufferUtil.bytes(UUIDGen.getTimeUUID())), cfamily);
+
+ SSTableReader reader = writer.closeAndOpenReader();
+ // Export to JSON and verify
+ File tempJson = File.createTempFile("CFWithColumnNameEqualToDefaultKeyAlias", ".json");
+ SSTableExport.export(reader, new PrintStream(tempJson.getPath()), new String[0]);
+
+ JSONArray json = (JSONArray)JSONValue.parseWithException(new FileReader(tempJson));
+ assertEquals(1, json.size());
+
+ JSONObject row = (JSONObject)json.get(0);
+ JSONArray cols = (JSONArray) row.get("columns");
+ assertEquals(1, cols.size());
+
+ // check column name and value
+ JSONArray col = (JSONArray) cols.get(0);
+ assertEquals(CFMetaData.DEFAULT_KEY_ALIAS, ByteBufferUtil.string(hexToBytes((String) col.get(0))));
+ assertEquals("not a uuid", ByteBufferUtil.string(hexToBytes((String) col.get(1))));
+ }
}