You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by jb...@apache.org on 2009/08/20 18:37:14 UTC
svn commit: r806245 - in /incubator/cassandra/trunk:
src/java/org/apache/cassandra/db/ src/java/org/apache/cassandra/service/
test/system/
Author: jbellis
Date: Thu Aug 20 16:37:14 2009
New Revision: 806245
URL: http://svn.apache.org/viewvc?rev=806245&view=rev
Log:
sanity check start, finish slice args
patch by jbellis; reviewed by Evan Weaver for CASSANDRA-376
Modified:
incubator/cassandra/trunk/src/java/org/apache/cassandra/db/ColumnFamily.java
incubator/cassandra/trunk/src/java/org/apache/cassandra/db/ReadCommand.java
incubator/cassandra/trunk/src/java/org/apache/cassandra/db/SliceByNamesReadCommand.java
incubator/cassandra/trunk/src/java/org/apache/cassandra/db/SliceFromReadCommand.java
incubator/cassandra/trunk/src/java/org/apache/cassandra/db/SuperColumn.java
incubator/cassandra/trunk/src/java/org/apache/cassandra/service/CassandraServer.java
incubator/cassandra/trunk/src/java/org/apache/cassandra/service/ThriftValidation.java
incubator/cassandra/trunk/test/system/test_server.py
Modified: incubator/cassandra/trunk/src/java/org/apache/cassandra/db/ColumnFamily.java
URL: http://svn.apache.org/viewvc/incubator/cassandra/trunk/src/java/org/apache/cassandra/db/ColumnFamily.java?rev=806245&r1=806244&r2=806245&view=diff
==============================================================================
--- incubator/cassandra/trunk/src/java/org/apache/cassandra/db/ColumnFamily.java (original)
+++ incubator/cassandra/trunk/src/java/org/apache/cassandra/db/ColumnFamily.java Thu Aug 20 16:37:14 2009
@@ -168,27 +168,11 @@
IColumn column;
if (path.superColumnName == null)
{
- try
- {
- getComparator().validate(path.columnName);
- }
- catch (Exception e)
- {
- throw new MarshalException("Invalid column name in " + path.columnFamilyName + " for " + getComparator().getClass().getName());
- }
column = new Column(path.columnName, value, timestamp, deleted);
}
else
{
assert isSuper();
- try
- {
- getComparator().validate(path.superColumnName);
- }
- catch (Exception e)
- {
- throw new MarshalException("Invalid supercolumn name in " + path.columnFamilyName + " for " + getComparator().getClass().getName());
- }
column = new SuperColumn(path.superColumnName, getSubComparator());
column.addColumn(new Column(path.columnName, value, timestamp, deleted)); // checks subcolumn name
}
@@ -436,4 +420,11 @@
}
return cf;
}
+
+ public static AbstractType getComparatorFor(String table, String columnFamilyName, byte[] superColumnName)
+ {
+ return superColumnName == null
+ ? DatabaseDescriptor.getComparator(table, columnFamilyName)
+ : DatabaseDescriptor.getSubComparator(table, columnFamilyName);
+ }
}
Modified: incubator/cassandra/trunk/src/java/org/apache/cassandra/db/ReadCommand.java
URL: http://svn.apache.org/viewvc/incubator/cassandra/trunk/src/java/org/apache/cassandra/db/ReadCommand.java?rev=806245&r1=806244&r2=806245&view=diff
==============================================================================
--- incubator/cassandra/trunk/src/java/org/apache/cassandra/db/ReadCommand.java (original)
+++ incubator/cassandra/trunk/src/java/org/apache/cassandra/db/ReadCommand.java Thu Aug 20 16:37:14 2009
@@ -30,7 +30,6 @@
import org.apache.cassandra.service.StorageService;
import org.apache.cassandra.db.marshal.AbstractType;
import org.apache.cassandra.db.filter.QueryPath;
-import org.apache.cassandra.config.DatabaseDescriptor;
public abstract class ReadCommand
@@ -80,7 +79,10 @@
this.isDigestQuery = isDigestQuery;
}
- public abstract String getColumnFamilyName();
+ public String getColumnFamilyName()
+ {
+ return queryPath.columnFamilyName;
+ }
public abstract ReadCommand copy();
@@ -88,9 +90,7 @@
protected AbstractType getComparator()
{
- return queryPath.superColumnName == null
- ? DatabaseDescriptor.getComparator(table, getColumnFamilyName())
- : DatabaseDescriptor.getSubComparator(table, getColumnFamilyName());
+ return ColumnFamily.getComparatorFor(table, getColumnFamilyName(), queryPath.superColumnName);
}
}
Modified: incubator/cassandra/trunk/src/java/org/apache/cassandra/db/SliceByNamesReadCommand.java
URL: http://svn.apache.org/viewvc/incubator/cassandra/trunk/src/java/org/apache/cassandra/db/SliceByNamesReadCommand.java?rev=806245&r1=806244&r2=806245&view=diff
==============================================================================
--- incubator/cassandra/trunk/src/java/org/apache/cassandra/db/SliceByNamesReadCommand.java (original)
+++ incubator/cassandra/trunk/src/java/org/apache/cassandra/db/SliceByNamesReadCommand.java Thu Aug 20 16:37:14 2009
@@ -43,12 +43,6 @@
}
@Override
- public String getColumnFamilyName()
- {
- return queryPath.columnFamilyName;
- }
-
- @Override
public ReadCommand copy()
{
ReadCommand readCommand= new SliceByNamesReadCommand(table, key, queryPath, columnNames);
Modified: incubator/cassandra/trunk/src/java/org/apache/cassandra/db/SliceFromReadCommand.java
URL: http://svn.apache.org/viewvc/incubator/cassandra/trunk/src/java/org/apache/cassandra/db/SliceFromReadCommand.java?rev=806245&r1=806244&r2=806245&view=diff
==============================================================================
--- incubator/cassandra/trunk/src/java/org/apache/cassandra/db/SliceFromReadCommand.java (original)
+++ incubator/cassandra/trunk/src/java/org/apache/cassandra/db/SliceFromReadCommand.java Thu Aug 20 16:37:14 2009
@@ -46,12 +46,6 @@
}
@Override
- public String getColumnFamilyName()
- {
- return queryPath.columnFamilyName;
- }
-
- @Override
public ReadCommand copy()
{
ReadCommand readCommand = new SliceFromReadCommand(table, key, queryPath, start, finish, reversed, count);
Modified: incubator/cassandra/trunk/src/java/org/apache/cassandra/db/SuperColumn.java
URL: http://svn.apache.org/viewvc/incubator/cassandra/trunk/src/java/org/apache/cassandra/db/SuperColumn.java?rev=806245&r1=806244&r2=806245&view=diff
==============================================================================
--- incubator/cassandra/trunk/src/java/org/apache/cassandra/db/SuperColumn.java (original)
+++ incubator/cassandra/trunk/src/java/org/apache/cassandra/db/SuperColumn.java Thu Aug 20 16:37:14 2009
@@ -163,14 +163,6 @@
{
if (!(column instanceof Column))
throw new UnsupportedOperationException("A super column can only contain simple columns.");
- try
- {
- getComparator().validate(column.name());
- }
- catch (Exception e)
- {
- throw new MarshalException("Invalid column name in supercolumn for " + getComparator().getClass().getName());
- }
IColumn oldColumn = columns_.get(column.name());
if ( oldColumn == null )
{
Modified: incubator/cassandra/trunk/src/java/org/apache/cassandra/service/CassandraServer.java
URL: http://svn.apache.org/viewvc/incubator/cassandra/trunk/src/java/org/apache/cassandra/service/CassandraServer.java?rev=806245&r1=806244&r2=806245&view=diff
==============================================================================
--- incubator/cassandra/trunk/src/java/org/apache/cassandra/service/CassandraServer.java (original)
+++ incubator/cassandra/trunk/src/java/org/apache/cassandra/service/CassandraServer.java Thu Aug 20 16:37:14 2009
@@ -199,13 +199,13 @@
if (predicate.column_names != null)
{
+ ThriftValidation.validateColumns(keyspace, column_parent, predicate.column_names);
return getSlice(new SliceByNamesReadCommand(keyspace, key, column_parent, predicate.column_names), consistency_level);
}
else
{
SliceRange range = predicate.slice_range;
- if (range.count < 0)
- throw new InvalidRequestException("get_slice requires non-negative count");
+ ThriftValidation.validateRange(keyspace, column_parent, range);
return getSlice(new SliceFromReadCommand(keyspace, key, column_parent, range.start, range.finish, range.reversed, range.count), consistency_level);
}
}
@@ -319,11 +319,16 @@
{
if (logger.isDebugEnabled())
logger.debug("batch_insert");
- RowMutation rm = RowMutation.getRowMutation(table, batch_mutation);
- Set<String> cfNames = rm.columnFamilyNames();
- ThriftValidation.validateKeyCommand(rm.key(), rm.table(), cfNames.toArray(new String[cfNames.size()]));
- doInsert(consistency_level, rm);
+ for (String cfName : batch_mutation.cfmap.keySet())
+ {
+ for (Column c : batch_mutation.cfmap.get(cfName))
+ {
+ ThriftValidation.validateColumnPath(table, new ColumnPath(cfName, null, c.name));
+ }
+ }
+
+ doInsert(consistency_level, RowMutation.getRowMutation(table, batch_mutation));
}
public void remove(String table, String key, ColumnPath column_path, long timestamp, int consistency_level)
@@ -356,11 +361,19 @@
{
if (logger.isDebugEnabled())
logger.debug("batch_insert_SuperColumn");
- RowMutation rm = RowMutation.getRowMutation(table, batch_mutation_super);
- Set<String> cfNames = rm.columnFamilyNames();
- ThriftValidation.validateKeyCommand(rm.key(), rm.table(), cfNames.toArray(new String[cfNames.size()]));
- doInsert(consistency_level, rm);
+ for (String cfName : batch_mutation_super.cfmap.keySet())
+ {
+ for (SuperColumn sc : batch_mutation_super.cfmap.get(cfName))
+ {
+ for (Column c : sc.columns)
+ {
+ ThriftValidation.validateColumnPath(table, new ColumnPath(cfName, sc.name, c.name));
+ }
+ }
+ }
+
+ doInsert(consistency_level, RowMutation.getRowMutation(table, batch_mutation_super));
}
public String get_string_property(String propertyName)
Modified: incubator/cassandra/trunk/src/java/org/apache/cassandra/service/ThriftValidation.java
URL: http://svn.apache.org/viewvc/incubator/cassandra/trunk/src/java/org/apache/cassandra/service/ThriftValidation.java?rev=806245&r1=806244&r2=806245&view=diff
==============================================================================
--- incubator/cassandra/trunk/src/java/org/apache/cassandra/service/ThriftValidation.java (original)
+++ incubator/cassandra/trunk/src/java/org/apache/cassandra/service/ThriftValidation.java Thu Aug 20 16:37:14 2009
@@ -21,18 +21,19 @@
*/
+import java.util.List;
+import java.util.Comparator;
+import java.util.Arrays;
+
import org.apache.cassandra.db.KeyspaceNotDefinedException;
import org.apache.cassandra.db.ColumnFamilyNotDefinedException;
+import org.apache.cassandra.db.ColumnFamily;
+import org.apache.cassandra.db.marshal.AbstractType;
+import org.apache.cassandra.db.marshal.MarshalException;
import org.apache.cassandra.config.DatabaseDescriptor;
public class ThriftValidation
{
- static void validateKeyCommand(String key, String tablename, String... columnFamilyNames) throws InvalidRequestException
- {
- validateKey(key);
- validateCommand(tablename, columnFamilyNames);
- }
-
static void validateKey(String key) throws InvalidRequestException
{
if (key.isEmpty())
@@ -90,9 +91,18 @@
throw new InvalidRequestException("column parameter is not optional for standard CF " + column_path.column_family);
}
}
- else if (column_path.super_column == null)
+ else
+ {
+ if (column_path.super_column == null)
+ throw new InvalidRequestException("column parameter is not optional for super CF " + column_path.column_family);
+ }
+ if (column_path.column != null)
+ {
+ validateColumns(tablename, column_path.column_family, column_path.super_column, Arrays.asList(column_path.column));
+ }
+ if (column_path.super_column != null)
{
- throw new InvalidRequestException("column parameter is not optional for super CF " + column_path.column_family);
+ validateColumns(tablename, column_path.column_family, null, Arrays.asList(column_path.super_column));
}
}
@@ -107,6 +117,10 @@
throw new InvalidRequestException("columnfamily alone is required for standard CF " + column_parent.column_family);
}
}
+ if (column_parent.super_column != null)
+ {
+ validateColumns(tablename, column_parent.column_family, null, Arrays.asList(column_parent.super_column));
+ }
}
// column_path_or_parent is a ColumnPath for remove, where the "column" is optional even for a standard CF
@@ -121,5 +135,60 @@
throw new InvalidRequestException("supercolumn may not be specified for standard CF " + column_path_or_parent.column_family);
}
}
+ if (column_path_or_parent.column != null)
+ {
+ validateColumns(tablename, column_path_or_parent.column_family, column_path_or_parent.super_column, Arrays.asList(column_path_or_parent.column));
+ }
+ if (column_path_or_parent.super_column != null)
+ {
+ validateColumns(tablename, column_path_or_parent.column_family, null, Arrays.asList(column_path_or_parent.super_column));
+ }
+ }
+
+ private static void validateColumns(String keyspace, String columnFamilyName, byte[] superColumnName, Iterable<byte[]> column_names)
+ throws InvalidRequestException
+ {
+ AbstractType comparator = ColumnFamily.getComparatorFor(keyspace, columnFamilyName, superColumnName);
+ for (byte[] name : column_names)
+ {
+ try
+ {
+ comparator.validate(name);
+ }
+ catch (MarshalException e)
+ {
+ throw new InvalidRequestException(e.getMessage());
+ }
+ }
+ }
+
+ public static void validateColumns(String keyspace, ColumnParent column_parent, Iterable<byte[]> column_names) throws InvalidRequestException
+ {
+ validateColumns(keyspace, column_parent.column_family, column_parent.super_column, column_names);
+ }
+
+ public static void validateRange(String keyspace, ColumnParent column_parent, SliceRange range) throws InvalidRequestException
+ {
+ AbstractType comparator = ColumnFamily.getComparatorFor(keyspace, column_parent.column_family, column_parent.super_column);
+ try
+ {
+ comparator.validate(range.start);
+ comparator.validate(range.finish);
+ }
+ catch (MarshalException e)
+ {
+ throw new InvalidRequestException(e.getMessage());
+ }
+
+ if (range.count < 0)
+ throw new InvalidRequestException("get_slice requires non-negative count");
+
+ Comparator<byte[]> orderedComparator = range.isReversed() ? comparator.getReverseComparator() : comparator;
+ if (range.start.length > 0
+ && range.finish.length > 0
+ && orderedComparator.compare(range.start, range.finish) > 0)
+ {
+ throw new InvalidRequestException("range finish must come after start in the order of traversal");
+ }
}
}
Modified: incubator/cassandra/trunk/test/system/test_server.py
URL: http://svn.apache.org/viewvc/incubator/cassandra/trunk/test/system/test_server.py?rev=806245&r1=806244&r2=806245&view=diff
==============================================================================
--- incubator/cassandra/trunk/test/system/test_server.py (original)
+++ incubator/cassandra/trunk/test/system/test_server.py Thu Aug 20 16:37:14 2009
@@ -231,12 +231,38 @@
_verify_batch()
def test_bad_calls(self):
+ # supercolumn in a non-super CF
_expect_exception(lambda: client.insert('Keyspace1', 'key1', ColumnPath('Standard1', 'x', 'y'), 'value', 0, ConsistencyLevel.ONE), InvalidRequestException)
-
+ # get doesn't specify column name
_expect_exception(lambda: client.get('Keyspace1', 'key1', ColumnPath('Standard1'), ConsistencyLevel.ONE), InvalidRequestException)
+ # supercolumn in a non-super CF
_expect_exception(lambda: client.get('Keyspace1', 'key1', ColumnPath('Standard1', 'x', 'y'), ConsistencyLevel.ONE), InvalidRequestException)
+ # get doesn't specify supercolumn name
_expect_exception(lambda: client.get('Keyspace1', 'key1', ColumnPath('Super1'), ConsistencyLevel.ONE), InvalidRequestException)
+ # invalid CF
_expect_exception(lambda: client.get_key_range('Keyspace1', 'S', '', '', 1000), InvalidRequestException)
+ # 'x' is not a valid Long
+ _expect_exception(lambda: client.insert('Keyspace1', 'key1', ColumnPath('Super1', 'sc1', 'x'), 'value', 0, ConsistencyLevel.ONE), InvalidRequestException)
+ # start is not a valid Long
+ p = SlicePredicate(slice_range=SliceRange('x', '', False, 1))
+ column_parent = ColumnParent('StandardLong1')
+ _expect_exception(lambda: client.get_slice('Keyspace1', 'key1', column_parent, p, ConsistencyLevel.ONE),
+ InvalidRequestException)
+ # start > finish
+ p = SlicePredicate(slice_range=SliceRange(_i64(10), _i64(0), False, 1))
+ column_parent = ColumnParent('StandardLong1')
+ _expect_exception(lambda: client.get_slice('Keyspace1', 'key1', column_parent, p, ConsistencyLevel.ONE),
+ InvalidRequestException)
+ # start is not a valid Long, supercolumn version
+ p = SlicePredicate(slice_range=SliceRange('x', '', False, 1))
+ column_parent = ColumnParent('Super1', 'sc1')
+ _expect_exception(lambda: client.get_slice('Keyspace1', 'key1', column_parent, p, ConsistencyLevel.ONE),
+ InvalidRequestException)
+ # start > finish, supercolumn version
+ p = SlicePredicate(slice_range=SliceRange(_i64(10), _i64(0), False, 1))
+ column_parent = ColumnParent('Super1', 'sc1')
+ _expect_exception(lambda: client.get_slice('Keyspace1', 'key1', column_parent, p, ConsistencyLevel.ONE),
+ InvalidRequestException)
def test_batch_insert_super(self):
cfmap = {'Super1': _SUPER_COLUMNS,