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:07 UTC
[1/3] git commit: Fix prepared statement on thrift post-6659
Repository: cassandra
Updated Branches:
refs/heads/cassandra-2.1 92b40fd90 -> 8afe1090b
Fix prepared statement on thrift post-6659
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/79da4a6e
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/79da4a6e
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/79da4a6e
Branch: refs/heads/cassandra-2.1
Commit: 79da4a6e0e1b33f880f6228e93905d85122238a2
Parents: 80b3e6c
Author: Sylvain Lebresne <sy...@datastax.com>
Authored: Wed Apr 2 11:32:48 2014 +0200
Committer: Sylvain Lebresne <sy...@datastax.com>
Committed: Wed Apr 2 11:33:05 2014 +0200
----------------------------------------------------------------------
src/java/org/apache/cassandra/cql3/QueryProcessor.java | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cassandra/blob/79da4a6e/src/java/org/apache/cassandra/cql3/QueryProcessor.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/QueryProcessor.java b/src/java/org/apache/cassandra/cql3/QueryProcessor.java
index fe818fd..64ea5e5 100644
--- a/src/java/org/apache/cassandra/cql3/QueryProcessor.java
+++ b/src/java/org/apache/cassandra/cql3/QueryProcessor.java
@@ -35,6 +35,7 @@ import org.apache.cassandra.db.*;
import org.apache.cassandra.exceptions.*;
import org.apache.cassandra.service.ClientState;
import org.apache.cassandra.service.QueryState;
+import org.apache.cassandra.thrift.ThriftClientState;
import org.apache.cassandra.tracing.Tracing;
import org.apache.cassandra.utils.FBUtilities;
import org.apache.cassandra.utils.MD5Digest;
@@ -237,7 +238,8 @@ public class QueryProcessor implements QueryHandler
public ResultMessage.Prepared prepare(String queryString, QueryState queryState)
throws RequestValidationException
{
- return prepare(queryString, queryState.getClientState(), false);
+ ClientState cState = queryState.getClientState();
+ return prepare(queryString, cState, cState instanceof ThriftClientState);
}
public static ResultMessage.Prepared prepare(String queryString, ClientState clientState, boolean forThrift)
[3/3] git commit: Merge branch 'cassandra-2.0' into cassandra-2.1
Posted by sl...@apache.org.
Merge branch 'cassandra-2.0' into cassandra-2.1
Conflicts:
src/java/org/apache/cassandra/config/CFMetaData.java
src/java/org/apache/cassandra/config/Schema.java
src/java/org/apache/cassandra/cql/QueryProcessor.java
src/java/org/apache/cassandra/cql/SelectStatement.java
src/java/org/apache/cassandra/cql/UpdateStatement.java
src/java/org/apache/cassandra/db/Column.java
src/java/org/apache/cassandra/thrift/ThriftValidation.java
src/java/org/apache/cassandra/tools/SSTableExport.java
src/java/org/apache/cassandra/tools/SSTableImport.java
test/unit/org/apache/cassandra/SchemaLoader.java
test/unit/org/apache/cassandra/db/ScrubTest.java
test/unit/org/apache/cassandra/thrift/ThriftValidationTest.java
test/unit/org/apache/cassandra/tools/SSTableExportTest.java
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/8afe1090
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/8afe1090
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/8afe1090
Branch: refs/heads/cassandra-2.1
Commit: 8afe1090bc4d04346197be6aa696b3af174f3674
Parents: 92b40fd b42feea
Author: Sylvain Lebresne <sy...@datastax.com>
Authored: Wed Apr 2 14:01:43 2014 +0200
Committer: Sylvain Lebresne <sy...@datastax.com>
Committed: Wed Apr 2 14:01:43 2014 +0200
----------------------------------------------------------------------
CHANGES.txt | 1 +
.../org/apache/cassandra/config/CFMetaData.java | 39 +++++-------
.../cassandra/config/ColumnDefinition.java | 9 +++
.../apache/cassandra/cql/QueryProcessor.java | 2 +-
.../apache/cassandra/cql3/QueryProcessor.java | 4 +-
.../cassandra/thrift/ThriftValidation.java | 5 +-
.../apache/cassandra/tools/SSTableImport.java | 2 +-
.../unit/org/apache/cassandra/SchemaLoader.java | 7 ++-
.../unit/org/apache/cassandra/db/ScrubTest.java | 65 ++++++++++++++++++++
.../cassandra/thrift/ThriftValidationTest.java | 53 ++++++++++++++--
.../cassandra/tools/SSTableExportTest.java | 35 +++++++++++
11 files changed, 188 insertions(+), 34 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cassandra/blob/8afe1090/CHANGES.txt
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cassandra/blob/8afe1090/src/java/org/apache/cassandra/config/CFMetaData.java
----------------------------------------------------------------------
diff --cc src/java/org/apache/cassandra/config/CFMetaData.java
index b46fafd,be18d1b..e930de4
--- a/src/java/org/apache/cassandra/config/CFMetaData.java
+++ b/src/java/org/apache/cassandra/config/CFMetaData.java
@@@ -919,25 -853,13 +919,10 @@@ public final class CFMetaDat
.toHashCode();
}
- public AbstractType<?> getValueValidator(CellName name)
- /**
- * Like getColumnDefinitionFromColumnName, the argument must be an internal column/cell name.
- */
- public AbstractType<?> getValueValidatorFromColumnName(ByteBuffer columnName)
++ public AbstractType<?> getValueValidator(CellName cellName)
{
- return getValueValidator(getColumnDefinition(name));
- }
-
- public AbstractType<?> getValueValidatorForFullCellName(ByteBuffer name)
- {
- // If this is a CQL table, we need to pull out the CQL column name to look up the correct column type.
- if (!isCQL3Table())
- return getValueValidator(getColumnDefinition(name));
-
- return getValueValidator(getColumnDefinition(comparator.cellFromByteBuffer(name)));
- }
-
- public AbstractType<?> getValueValidator(ColumnDefinition columnDefinition)
- {
- return columnDefinition == null
- ? defaultValidator
- : columnDefinition.type;
- ColumnDefinition def = getColumnDefinitionFromColumnName(columnName);
- return def == null ? defaultValidator : def.getValidator();
++ ColumnDefinition def = getColumnDefinition(cellName);
++ return def == null ? defaultValidator : def.type;
}
/** applies implicit defaults to cf definition. useful in updates */
@@@ -1312,14 -1201,46 +1297,24 @@@
}
/**
- * Returns a ColumnDefinition given a full (internal) column name.
+ * Returns a ColumnDefinition given a cell name.
*/
- public ColumnDefinition getColumnDefinitionFromColumnName(ByteBuffer columnName)
+ public ColumnDefinition getColumnDefinition(CellName cellName)
{
- if (!isSuper() && (comparator instanceof CompositeType))
- {
- CompositeType composite = (CompositeType)comparator;
- ByteBuffer[] components = composite.split(columnName);
- for (ColumnDefinition def : regularAndStaticColumns())
- {
- ByteBuffer toCompare;
- if (def.componentIndex == null)
- {
- toCompare = columnName;
- }
- else
- {
- if (def.componentIndex >= components.length)
- break;
+ ColumnIdentifier id = cellName.cql3ColumnName();
- return id == null
- ? getColumnDefinition(cellName.toByteBuffer()) // Means a dense layout, try the full column name
- : getColumnDefinition(id);
++ ColumnDefinition def = id == null
++ ? getColumnDefinition(cellName.toByteBuffer()) // Means a dense layout, try the full column name
++ : getColumnDefinition(id);
+
- toCompare = components[def.componentIndex];
- }
- if (def.name.equals(toCompare))
- return def;
- }
- return null;
- }
- else
- {
- 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;
- }
++ // 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)
@@@ -1930,18 -1850,18 +1925,18 @@@
droppedColumns.put(def.name, FBUtilities.timestampMicros());
}
- public void renameColumn(ByteBuffer from, String strFrom, ByteBuffer to, String strTo) throws InvalidRequestException
+ public void renameColumn(ColumnIdentifier from, ColumnIdentifier to) throws InvalidRequestException
{
- ColumnDefinition def = column_metadata.get(from);
+ ColumnDefinition def = getColumnDefinition(from);
if (def == null)
- throw new InvalidRequestException(String.format("Cannot rename unknown column %s in keyspace %s", strFrom, cfName));
+ throw new InvalidRequestException(String.format("Cannot rename unknown column %s in keyspace %s", from, cfName));
- 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 (getColumnDefinition(to) != null)
+ throw new InvalidRequestException(String.format("Cannot rename column %s to %s in keyspace %s; another column of that name already exist", from, to, cfName));
- if (def.kind == ColumnDefinition.Kind.REGULAR || def.kind == ColumnDefinition.Kind.STATIC)
+ if (def.isPartOfCellName())
{
- throw new InvalidRequestException(String.format("Cannot rename non PRIMARY KEY part %s", strFrom));
+ throw new InvalidRequestException(String.format("Cannot rename non PRIMARY KEY part %s", from));
}
else if (def.isIndexed())
{
http://git-wip-us.apache.org/repos/asf/cassandra/blob/8afe1090/src/java/org/apache/cassandra/config/ColumnDefinition.java
----------------------------------------------------------------------
diff --cc src/java/org/apache/cassandra/config/ColumnDefinition.java
index bb1dd71,1223db8..ee8884e
--- a/src/java/org/apache/cassandra/config/ColumnDefinition.java
+++ b/src/java/org/apache/cassandra/config/ColumnDefinition.java
@@@ -243,6 -193,15 +243,15 @@@ public class ColumnDefinition extends C
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;
++ return kind == Kind.REGULAR || kind == Kind.STATIC;
+ }
+
public ColumnDef toThrift()
{
ColumnDef cd = new ColumnDef();
http://git-wip-us.apache.org/repos/asf/cassandra/blob/8afe1090/src/java/org/apache/cassandra/cql/QueryProcessor.java
----------------------------------------------------------------------
diff --cc src/java/org/apache/cassandra/cql/QueryProcessor.java
index 458f131,f160374..baf89b2
--- a/src/java/org/apache/cassandra/cql/QueryProcessor.java
+++ b/src/java/org/apache/cassandra/cql/QueryProcessor.java
@@@ -189,11 -189,11 +189,11 @@@ public class QueryProcesso
for (Relation columnRelation : columnRelations)
{
// 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(metadata.getValueValidatorFromColumnName(entity), variables);
+ ByteBuffer entity = columnRelation.getEntity().getByteBuffer(metadata.comparator.asAbstractType(), variables);
- ByteBuffer value = columnRelation.getValue().getByteBuffer(metadata.getValueValidatorForFullCellName(entity), variables);
++ ByteBuffer value = columnRelation.getValue().getByteBuffer(metadata.getValueValidator(metadata.comparator.cellFromByteBuffer(entity)), variables);
expressions.add(new IndexExpression(entity,
- IndexOperator.valueOf(columnRelation.operator().toString()),
+ IndexExpression.Operator.valueOf(columnRelation.operator().toString()),
value));
}
http://git-wip-us.apache.org/repos/asf/cassandra/blob/8afe1090/src/java/org/apache/cassandra/cql3/QueryProcessor.java
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cassandra/blob/8afe1090/src/java/org/apache/cassandra/thrift/ThriftValidation.java
----------------------------------------------------------------------
diff --cc src/java/org/apache/cassandra/thrift/ThriftValidation.java
index 49cf39b,b387871..222a904
--- a/src/java/org/apache/cassandra/thrift/ThriftValidation.java
+++ b/src/java/org/apache/cassandra/thrift/ThriftValidation.java
@@@ -439,13 -443,9 +439,12 @@@ public class ThriftValidatio
if (!column.isSetTimestamp())
throw new org.apache.cassandra.exceptions.InvalidRequestException("Column timestamp is required");
+ CellName cn = scName == null
+ ? metadata.comparator.cellFromByteBuffer(column.name)
+ : metadata.comparator.makeCellName(scName, column.name);
- ColumnDefinition columnDef = metadata.getColumnDefinition(cn);
try
{
- AbstractType<?> validator = metadata.getValueValidator(columnDef);
- AbstractType<?> validator = metadata.getValueValidatorFromColumnName(column.name);
++ AbstractType<?> validator = metadata.getValueValidator(cn);
if (validator != null)
validator.validate(column.value);
}
@@@ -462,10 -462,10 +461,10 @@@
}
// Indexed column values cannot be larger than 64K. See CASSANDRA-3057/4240 for more details
- if (!Keyspace.open(metadata.ksName).getColumnFamilyStore(metadata.cfName).indexManager.validate(asDBColumn(column)))
+ if (!Keyspace.open(metadata.ksName).getColumnFamilyStore(metadata.cfName).indexManager.validate(asDBColumn(cn, 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.getColumnDefinition(cn).getIndexName(),
metadata.cfName,
metadata.ksName));
}
http://git-wip-us.apache.org/repos/asf/cassandra/blob/8afe1090/src/java/org/apache/cassandra/tools/SSTableImport.java
----------------------------------------------------------------------
diff --cc src/java/org/apache/cassandra/tools/SSTableImport.java
index 2a516a4,11bfc81..c0d8e00
--- a/src/java/org/apache/cassandra/tools/SSTableImport.java
+++ b/src/java/org/apache/cassandra/tools/SSTableImport.java
@@@ -162,7 -161,7 +162,7 @@@ public class SSTableImpor
}
else
{
- value = stringAsType((String) fields.get(1), meta.getValueValidatorForFullCellName(name));
- value = stringAsType((String) fields.get(1), meta.getValueValidatorFromColumnName(name));
++ value = stringAsType((String) fields.get(1), meta.getValueValidator(meta.comparator.cellFromByteBuffer(name)));
}
}
}
http://git-wip-us.apache.org/repos/asf/cassandra/blob/8afe1090/test/unit/org/apache/cassandra/SchemaLoader.java
----------------------------------------------------------------------
diff --cc test/unit/org/apache/cassandra/SchemaLoader.java
index 5fb5697,daff0de..43a6dc3
--- a/test/unit/org/apache/cassandra/SchemaLoader.java
+++ b/test/unit/org/apache/cassandra/SchemaLoader.java
@@@ -147,12 -167,31 +147,13 @@@ public class SchemaLoade
superCFMD(ks1, "Super6", LexicalUUIDType.instance, UTF8Type.instance),
indexCFMD(ks1, "Indexed1", true),
indexCFMD(ks1, "Indexed2", false),
- new CFMetaData(ks1,
- "StandardInteger1",
- st,
- IntegerType.instance,
- null),
- new CFMetaData(ks1,
- "StandardLong3",
- st,
- LongType.instance,
- null),
- new CFMetaData(ks1,
- "Counter1",
- st,
- bytes,
- null)
- .defaultValidator(CounterColumnType.instance),
- new CFMetaData(ks1,
- "SuperCounter1",
- su,
- bytes,
- bytes)
- .defaultValidator(CounterColumnType.instance),
+ CFMetaData.denseCFMetaData(ks1, "StandardInteger1", IntegerType.instance),
++ CFMetaData.denseCFMetaData(ks1, "StandardLong3", IntegerType.instance),
+ CFMetaData.denseCFMetaData(ks1, "Counter1", bytes).defaultValidator(CounterColumnType.instance),
+ CFMetaData.denseCFMetaData(ks1, "SuperCounter1", bytes, bytes).defaultValidator(CounterColumnType.instance),
superCFMD(ks1, "SuperDirectGC", BytesType.instance).gcGraceSeconds(0),
- jdbcCFMD(ks1, "JdbcInteger", IntegerType.instance).columnMetadata(integerColumn),
- jdbcCFMD(ks1, "JdbcUtf8", UTF8Type.instance).columnMetadata(utf8Column),
+ jdbcSparseCFMD(ks1, "JdbcInteger", IntegerType.instance).addColumnDefinition(integerColumn(ks1, "JdbcInteger")),
+ jdbcSparseCFMD(ks1, "JdbcUtf8", UTF8Type.instance).addColumnDefinition(utf8Column(ks1, "JdbcUtf8")),
jdbcCFMD(ks1, "JdbcLong", LongType.instance),
jdbcCFMD(ks1, "JdbcBytes", bytes),
jdbcCFMD(ks1, "JdbcAscii", AsciiType.instance),
@@@ -165,10 -216,17 +166,14 @@@
standardCFMD(ks1, "legacyleveled")
.compactionStrategyClass(LeveledCompactionStrategy.class)
.compactionStrategyOptions(leveledOptions),
+ standardCFMD(ks1, "StandardLowIndexInterval").minIndexInterval(8)
+ .maxIndexInterval(256)
- .caching(CachingOptions.NONE)));
++ .caching(CachingOptions.NONE),
+
+ 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)));
++ CFMetaData.denseCFMetaData(ks1, "MixedTypes", LongType.instance).keyValidator(UUIDType.instance).defaultValidator(BooleanType.instance),
++ CFMetaData.denseCFMetaData(ks1, "MixedTypesComposite", composite).keyValidator(composite).defaultValidator(BooleanType.instance)
++ ));
// Keyspace 2
schema.add(KSMetaData.testMetadata(ks2,
http://git-wip-us.apache.org/repos/asf/cassandra/blob/8afe1090/test/unit/org/apache/cassandra/db/ScrubTest.java
----------------------------------------------------------------------
diff --cc test/unit/org/apache/cassandra/db/ScrubTest.java
index ebdce0e,23e9381..b8c7980
--- a/test/unit/org/apache/cassandra/db/ScrubTest.java
+++ b/test/unit/org/apache/cassandra/db/ScrubTest.java
@@@ -273,4 -278,63 +279,63 @@@ public class ScrubTest extends SchemaLo
cfs.forceBlockingFlush();
}
+ @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");
++ ColumnFamily cf = ArrayBackedSortedColumns.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();
++ Mutation mutation = new Mutation("Keyspace1", ByteBufferUtil.bytes(UUIDGen.getTimeUUID()), cf);
++ mutation.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/8afe1090/test/unit/org/apache/cassandra/thrift/ThriftValidationTest.java
----------------------------------------------------------------------
diff --cc test/unit/org/apache/cassandra/thrift/ThriftValidationTest.java
index 05ac588,0e8bbb8..1d47c18
--- a/test/unit/org/apache/cassandra/thrift/ThriftValidationTest.java
+++ b/test/unit/org/apache/cassandra/thrift/ThriftValidationTest.java
@@@ -46,10 -51,10 +51,10 @@@ public class ThriftValidationTest exten
}
@Test
- public void testColumnNameEqualToKeyAlias()
+ public void testColumnNameEqualToKeyAlias() throws org.apache.cassandra.exceptions.InvalidRequestException
{
CFMetaData metaData = Schema.instance.getCFMetaData("Keyspace1", "Standard1");
- CFMetaData newMetadata = metaData.clone();
+ CFMetaData newMetadata = metaData.copy();
boolean gotException = false;
@@@ -57,7 -62,7 +62,7 @@@
// should not throw IRE here
try
{
- newMetadata.addColumnDefinition(ColumnDefinition.partitionKeyDef(metaData, AsciiType.instance.decompose("id"), UTF8Type.instance, null));
- newMetadata.addColumnDefinition(ColumnDefinition.partitionKeyDef(AsciiType.instance.decompose("id"), LongType.instance, null));
++ newMetadata.addColumnDefinition(ColumnDefinition.partitionKeyDef(metaData, AsciiType.instance.decompose("id"), LongType.instance, null));
newMetadata.validate();
}
catch (ConfigurationException e)
@@@ -73,7 -78,7 +78,7 @@@
// add a column with name = "id"
try
{
- newMetadata.addColumnDefinition(ColumnDefinition.regularDef(metaData, ByteBufferUtil.bytes("id"), UTF8Type.instance, null));
- newMetadata.addColumnDefinition(ColumnDefinition.regularDef(ByteBufferUtil.bytes("id"), LongType.instance, null));
++ newMetadata.addColumnDefinition(ColumnDefinition.regularDef(metaData, ByteBufferUtil.bytes("id"), LongType.instance, null));
newMetadata.validate();
}
catch (ConfigurationException e)
@@@ -82,6 -87,44 +87,44 @@@
}
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);
++ ThriftValidation.validateColumnData(newMetadata, null, column);
+ }
+
+ @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);
++ assertEquals(ColumnDefinition.Kind.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);
++ ThriftValidation.validateColumnData(metaData, null, column);
+
+ 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);
++ ThriftValidation.validateColumnData(metaData, null, column);
}
@Test
http://git-wip-us.apache.org/repos/asf/cassandra/blob/8afe1090/test/unit/org/apache/cassandra/tools/SSTableExportTest.java
----------------------------------------------------------------------
diff --cc test/unit/org/apache/cassandra/tools/SSTableExportTest.java
index b74ba1a,d0ab6a2..454600d
--- a/test/unit/org/apache/cassandra/tools/SSTableExportTest.java
+++ b/test/unit/org/apache/cassandra/tools/SSTableExportTest.java
@@@ -38,8 -43,9 +40,9 @@@ import org.apache.cassandra.db.marshal.
import org.apache.cassandra.io.sstable.Descriptor;
import org.apache.cassandra.io.sstable.SSTableReader;
import org.apache.cassandra.io.sstable.SSTableWriter;
+import org.apache.cassandra.service.ActiveRepairService;
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;
@@@ -330,4 -336,36 +333,36 @@@ public class SSTableExportTest extends
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);
++ ColumnFamily cfamily = ArrayBackedSortedColumns.factory.create("Keyspace1", "UUIDKeys");
++ SSTableWriter writer = new SSTableWriter(tempSS.getPath(), 2, ActiveRepairService.UNREPAIRED_SSTABLE);
+
+ // 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))));
+ }
}
[2/3] git commit: Fix Cassandra 2.0.x validates Thrift columns
incorrectly
Posted by sl...@apache.org.
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 <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))));
+ }
}