cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From slebre...@apache.org
Subject [2/3] git commit: Fix Cassandra 2.0.x validates Thrift columns incorrectly
Date Wed, 02 Apr 2014 12:02:08 GMT
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.1
Commit: b42feea6432905946083e0e287b3545a5799e4a7
Parents: 79da4a6
Author: Sylvain Lebresne <sylvain@datastax.com>
Authored: Wed Apr 2 11:58:00 2014 +0200
Committer: Sylvain Lebresne <sylvain@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))));
+    }
 }


Mime
View raw message