Return-Path: X-Original-To: apmail-cassandra-commits-archive@www.apache.org Delivered-To: apmail-cassandra-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 66B6611B9C for ; Wed, 2 Apr 2014 09:59:09 +0000 (UTC) Received: (qmail 4325 invoked by uid 500); 2 Apr 2014 09:59:09 -0000 Delivered-To: apmail-cassandra-commits-archive@cassandra.apache.org Received: (qmail 4103 invoked by uid 500); 2 Apr 2014 09:59:08 -0000 Mailing-List: contact commits-help@cassandra.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@cassandra.apache.org Delivered-To: mailing list commits@cassandra.apache.org Received: (qmail 4081 invoked by uid 99); 2 Apr 2014 09:59:05 -0000 Received: from tyr.zones.apache.org (HELO tyr.zones.apache.org) (140.211.11.114) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 02 Apr 2014 09:59:05 +0000 Received: by tyr.zones.apache.org (Postfix, from userid 65534) id 5D922925D33; Wed, 2 Apr 2014 09:59:04 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: slebresne@apache.org To: commits@cassandra.apache.org Message-Id: X-Mailer: ASF-Git Admin Mailer Subject: git commit: Fix Cassandra 2.0.x validates Thrift columns incorrectly Date: Wed, 2 Apr 2014 09:59:04 +0000 (UTC) Repository: cassandra Updated Branches: refs/heads/cassandra-2.0 79da4a6e0 -> b42feea64 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/cassandra-2.0 Commit: b42feea6432905946083e0e287b3545a5799e4a7 Parents: 79da4a6 Author: Sylvain Lebresne Authored: Wed Apr 2 11:58:00 2014 +0200 Committer: Sylvain Lebresne 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 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 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 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)))); + } }