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 2015/11/06 15:03:40 UTC
cassandra git commit: Don't use -1 as position for partition key in
schema
Repository: cassandra
Updated Branches:
refs/heads/cassandra-3.0 0b83c188b -> 5343a75bf
Don't use -1 as position for partition key in schema
patch by slebresne; reviewed by beobal for CASSANDRA-10491
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/5343a75b
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/5343a75b
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/5343a75b
Branch: refs/heads/cassandra-3.0
Commit: 5343a75bf56632c169ee8165f281ee5eb521188a
Parents: 0b83c18
Author: Sylvain Lebresne <sy...@datastax.com>
Authored: Mon Nov 2 17:58:31 2015 +0100
Committer: Sylvain Lebresne <sy...@datastax.com>
Committed: Fri Nov 6 15:00:20 2015 +0100
----------------------------------------------------------------------
CHANGES.txt | 1 +
.../org/apache/cassandra/config/CFMetaData.java | 3 +-
.../cassandra/config/ColumnDefinition.java | 25 ++++++---------
.../cql3/statements/CreateIndexStatement.java | 2 +-
.../apache/cassandra/db/filter/RowFilter.java | 32 +++++++++++---------
.../apache/cassandra/db/view/TemporalRow.java | 8 +++--
.../cassandra/schema/LegacySchemaMigrator.java | 7 +++--
.../apache/cassandra/schema/SchemaKeyspace.java | 2 +-
.../cassandra/thrift/ThriftConversion.java | 4 +--
.../org/apache/cassandra/db/ColumnsTest.java | 2 +-
.../schema/LegacySchemaMigratorTest.java | 2 +-
11 files changed, 43 insertions(+), 45 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cassandra/blob/5343a75b/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index a72cab2..146ee73 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
3.0
+ * Don't use -1 for the position of partition key in schema (CASSANDRA-10491)
* Fix distinct queries in mixed version cluster (CASSANDRA-10573)
* Skip sstable on clustering in names query (CASSANDRA-10571)
* Remove value skipping as it breaks read-repair (CASSANDRA-10655)
http://git-wip-us.apache.org/repos/asf/cassandra/blob/5343a75b/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 86f78eb..4d5a176 100644
--- a/src/java/org/apache/cassandra/config/CFMetaData.java
+++ b/src/java/org/apache/cassandra/config/CFMetaData.java
@@ -1266,8 +1266,7 @@ public final class CFMetaData
for (int i = 0; i < partitionKeys.size(); i++)
{
Pair<ColumnIdentifier, AbstractType> p = partitionKeys.get(i);
- int position = partitionKeys.size() == 1 ? ColumnDefinition.NO_POSITION : i;
- partitions.add(new ColumnDefinition(keyspace, table, p.left, p.right, position, ColumnDefinition.Kind.PARTITION_KEY));
+ partitions.add(new ColumnDefinition(keyspace, table, p.left, p.right, i, ColumnDefinition.Kind.PARTITION_KEY));
}
for (int i = 0; i < clusteringColumns.size(); i++)
http://git-wip-us.apache.org/repos/asf/cassandra/blob/5343a75b/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 96513c2..6bcc2e0 100644
--- a/src/java/org/apache/cassandra/config/ColumnDefinition.java
+++ b/src/java/org/apache/cassandra/config/ColumnDefinition.java
@@ -69,9 +69,11 @@ public class ColumnDefinition extends ColumnSpecification implements Comparable<
public final Kind kind;
/*
- * If the column comparator is a composite type, indicates to which
- * component this definition refers to. If NO_POSITION (-1), the definition refers to
- * the full column name.
+ * If the column is a partition key or clustering column, its position relative to
+ * other columns of the same kind. Otherwise, NO_POSITION (-1).
+ *
+ * Note that partition key and clustering columns are numbered separately so
+ * the first clustering column is 0.
*/
private final int position;
@@ -150,14 +152,14 @@ public class ColumnDefinition extends ColumnSpecification implements Comparable<
super(ksName, cfName, name, type);
assert name != null && type != null && kind != null;
assert name.isInterned();
- assert position == NO_POSITION || kind.isPrimaryKeyKind(); // The position really only make sense for partition and clustering columns,
- // so make sure we don't sneak it for something else since it'd breaks equals()
+ assert (position == NO_POSITION) == !kind.isPrimaryKeyKind(); // The position really only make sense for partition and clustering columns (and those must have one),
+ // so make sure we don't sneak it for something else since it'd breaks equals()
this.kind = kind;
this.position = position;
this.cellPathComparator = makeCellPathComparator(kind, type);
this.cellComparator = cellPathComparator == null ? ColumnData.comparator : (a, b) -> cellPathComparator.compare(a.path(), b.path());
this.asymmetricCellPathComparator = cellPathComparator == null ? null : (a, b) -> cellPathComparator.compare(((Cell)a).path(), (CellPath) b);
- this.comparisonOrder = comparisonOrder(kind, isComplex(), position(), name);
+ this.comparisonOrder = comparisonOrder(kind, isComplex(), Math.max(0, position), name);
}
private static Comparator<CellPath> makeCellPathComparator(Kind kind, AbstractType<?> type)
@@ -202,11 +204,6 @@ public class ColumnDefinition extends ColumnSpecification implements Comparable<
return new ColumnDefinition(ksName, cfName, name, newType, position, kind);
}
- public boolean isOnAllComponents()
- {
- return position == NO_POSITION;
- }
-
public boolean isPartitionKey()
{
return kind == Kind.PARTITION_KEY;
@@ -235,13 +232,9 @@ public class ColumnDefinition extends ColumnSpecification implements Comparable<
return type.isReversed() ? ClusteringOrder.DESC : ClusteringOrder.ASC;
}
- /**
- * For convenience sake, if position == NO_POSITION, this method will return 0. The callers should first check
- * isOnAllComponents() to distinguish between proper 0 position and NO_POSITION.
- */
public int position()
{
- return Math.max(0, position);
+ return position;
}
@Override
http://git-wip-us.apache.org/repos/asf/cassandra/blob/5343a75b/src/java/org/apache/cassandra/cql3/statements/CreateIndexStatement.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/statements/CreateIndexStatement.java b/src/java/org/apache/cassandra/cql3/statements/CreateIndexStatement.java
index d11d2c5..e26a1eb 100644
--- a/src/java/org/apache/cassandra/cql3/statements/CreateIndexStatement.java
+++ b/src/java/org/apache/cassandra/cql3/statements/CreateIndexStatement.java
@@ -114,7 +114,7 @@ public class CreateIndexStatement extends SchemaAlteringStatement
if (!cfm.isCompactTable() && cd.isStatic())
throw new InvalidRequestException("Secondary indexes are not allowed on static columns");
- if (cd.kind == ColumnDefinition.Kind.PARTITION_KEY && cd.isOnAllComponents())
+ if (cd.kind == ColumnDefinition.Kind.PARTITION_KEY && cfm.getKeyValidatorAsClusteringComparator().size() == 1)
throw new InvalidRequestException(String.format("Cannot create secondary index on partition key column %s", target.column));
boolean isMap = cd.type instanceof MapType;
http://git-wip-us.apache.org/repos/asf/cassandra/blob/5343a75b/src/java/org/apache/cassandra/db/filter/RowFilter.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/filter/RowFilter.java b/src/java/org/apache/cassandra/db/filter/RowFilter.java
index 09dc342..17db323 100644
--- a/src/java/org/apache/cassandra/db/filter/RowFilter.java
+++ b/src/java/org/apache/cassandra/db/filter/RowFilter.java
@@ -222,6 +222,8 @@ public abstract class RowFilter implements Iterable<RowFilter.Expression>
if (expressions.isEmpty())
return iter;
+ final CFMetaData metadata = iter.metadata();
+
class IsSatisfiedFilter extends Transformation<UnfilteredRowIterator>
{
DecoratedKey pk;
@@ -238,7 +240,7 @@ public abstract class RowFilter implements Iterable<RowFilter.Expression>
return null;
for (Expression e : expressions)
- if (!e.isSatisfiedBy(pk, purged))
+ if (!e.isSatisfiedBy(metadata, pk, purged))
return null;
return row;
}
@@ -282,7 +284,7 @@ public abstract class RowFilter implements Iterable<RowFilter.Expression>
{
assert expr instanceof ThriftExpression;
Row row = result.getRow(makeCompactClustering(iter.metadata(), expr.column().name.bytes));
- if (row == null || !expr.isSatisfiedBy(iter.partitionKey(), row))
+ if (row == null || !expr.isSatisfiedBy(iter.metadata(), iter.partitionKey(), row))
return null;
}
// If we get there, it means all expressions where satisfied, so return the original result
@@ -378,16 +380,16 @@ public abstract class RowFilter implements Iterable<RowFilter.Expression>
* (i.e. it should come from a RowIterator).
* @return whether the row is satisfied by this expression.
*/
- public abstract boolean isSatisfiedBy(DecoratedKey partitionKey, Row row);
+ public abstract boolean isSatisfiedBy(CFMetaData metadata, DecoratedKey partitionKey, Row row);
- protected ByteBuffer getValue(DecoratedKey partitionKey, Row row)
+ protected ByteBuffer getValue(CFMetaData metadata, DecoratedKey partitionKey, Row row)
{
switch (column.kind)
{
case PARTITION_KEY:
- return column.isOnAllComponents()
- ? partitionKey.getKey()
- : CompositeType.extractComponent(partitionKey.getKey(), column.position());
+ return metadata.getKeyValidator() instanceof CompositeType
+ ? CompositeType.extractComponent(partitionKey.getKey(), column.position())
+ : partitionKey.getKey();
case CLUSTERING:
return row.clustering().get(column.position());
default:
@@ -572,7 +574,7 @@ public abstract class RowFilter implements Iterable<RowFilter.Expression>
super(column, operator, value);
}
- public boolean isSatisfiedBy(DecoratedKey partitionKey, Row row)
+ public boolean isSatisfiedBy(CFMetaData metadata, DecoratedKey partitionKey, Row row)
{
// We support null conditions for LWT (in ColumnCondition) but not for RowFilter.
// TODO: we should try to merge both code someday.
@@ -591,7 +593,7 @@ public abstract class RowFilter implements Iterable<RowFilter.Expression>
case NEQ:
{
assert !column.isComplex() : "Only CONTAINS and CONTAINS_KEY are supported for 'complex' types";
- ByteBuffer foundValue = getValue(partitionKey, row);
+ ByteBuffer foundValue = getValue(metadata, partitionKey, row);
// Note that CQL expression are always of the form 'x < 4', i.e. the tested value is on the left.
return foundValue != null && operator.isSatisfiedBy(column.type, foundValue, value);
}
@@ -618,7 +620,7 @@ public abstract class RowFilter implements Iterable<RowFilter.Expression>
}
else
{
- ByteBuffer foundValue = getValue(partitionKey, row);
+ ByteBuffer foundValue = getValue(metadata, partitionKey, row);
if (foundValue == null)
return false;
@@ -645,7 +647,7 @@ public abstract class RowFilter implements Iterable<RowFilter.Expression>
}
else
{
- ByteBuffer foundValue = getValue(partitionKey, row);
+ ByteBuffer foundValue = getValue(metadata, partitionKey, row);
return foundValue != null && mapType.getSerializer().getSerializedValue(foundValue, value, mapType.getKeysType()) != null;
}
@@ -717,7 +719,7 @@ public abstract class RowFilter implements Iterable<RowFilter.Expression>
return CompositeType.build(key, value);
}
- public boolean isSatisfiedBy(DecoratedKey partitionKey, Row row)
+ public boolean isSatisfiedBy(CFMetaData metadata, DecoratedKey partitionKey, Row row)
{
assert key != null;
// We support null conditions for LWT (in ColumnCondition) but not for RowFilter.
@@ -735,7 +737,7 @@ public abstract class RowFilter implements Iterable<RowFilter.Expression>
}
else
{
- ByteBuffer serializedMap = getValue(partitionKey, row);
+ ByteBuffer serializedMap = getValue(metadata, partitionKey, row);
if (serializedMap == null)
return false;
@@ -807,7 +809,7 @@ public abstract class RowFilter implements Iterable<RowFilter.Expression>
return ColumnDefinition.regularDef(metadata, name, metadata.compactValueColumn().type);
}
- public boolean isSatisfiedBy(DecoratedKey partitionKey, Row row)
+ public boolean isSatisfiedBy(CFMetaData metadata, DecoratedKey partitionKey, Row row)
{
assert value != null;
@@ -882,7 +884,7 @@ public abstract class RowFilter implements Iterable<RowFilter.Expression>
}
// Filtering by custom expressions isn't supported yet, so just accept any row
- public boolean isSatisfiedBy(DecoratedKey partitionKey, Row row)
+ public boolean isSatisfiedBy(CFMetaData metadata, DecoratedKey partitionKey, Row row)
{
return true;
}
http://git-wip-us.apache.org/repos/asf/cassandra/blob/5343a75b/src/java/org/apache/cassandra/db/view/TemporalRow.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/view/TemporalRow.java b/src/java/org/apache/cassandra/db/view/TemporalRow.java
index 46dc3fa..d876b00 100644
--- a/src/java/org/apache/cassandra/db/view/TemporalRow.java
+++ b/src/java/org/apache/cassandra/db/view/TemporalRow.java
@@ -380,14 +380,16 @@ public class TemporalRow
if (baseDefinition.isPartitionKey())
{
- if (baseDefinition.isOnAllComponents())
- return basePartitionKey;
- else
+ if (baseCfs.metadata.getKeyValidator() instanceof CompositeType)
{
CompositeType keyComparator = (CompositeType) baseCfs.metadata.getKeyValidator();
ByteBuffer[] components = keyComparator.split(basePartitionKey);
return components[baseDefinition.position()];
}
+ else
+ {
+ return basePartitionKey;
+ }
}
else
{
http://git-wip-us.apache.org/repos/asf/cassandra/blob/5343a75b/src/java/org/apache/cassandra/schema/LegacySchemaMigrator.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/schema/LegacySchemaMigrator.java b/src/java/org/apache/cassandra/schema/LegacySchemaMigrator.java
index 79f9e99..9008cc8 100644
--- a/src/java/org/apache/cassandra/schema/LegacySchemaMigrator.java
+++ b/src/java/org/apache/cassandra/schema/LegacySchemaMigrator.java
@@ -486,7 +486,7 @@ public final class LegacySchemaMigrator
}
else if (isStaticCompactTable)
{
- defs.add(ColumnDefinition.clusteringDef(ksName, cfName, names.defaultClusteringName(), rawComparator, ColumnDefinition.NO_POSITION));
+ defs.add(ColumnDefinition.clusteringDef(ksName, cfName, names.defaultClusteringName(), rawComparator, 0));
defs.add(ColumnDefinition.regularDef(ksName, cfName, names.defaultCompactValueName(), defaultValidator));
}
else
@@ -586,8 +586,9 @@ public final class LegacySchemaMigrator
// Note that the component_index is not useful for non-primary key parts (it never really in fact since there is
// no particular ordering of non-PK columns, we only used to use it as a simplification but that's not needed
// anymore)
- if (kind.isPrimaryKeyKind() && row.has("component_index"))
- componentIndex = row.getInt("component_index");
+ if (kind.isPrimaryKeyKind())
+ // We use to not have a component index when there was a single partition key, we don't anymore (#10491)
+ componentIndex = row.has("component_index") ? row.getInt("component_index") : 0;
// Note: we save the column name as string, but we should not assume that it is an UTF8 name, we
// we need to use the comparator fromString method
http://git-wip-us.apache.org/repos/asf/cassandra/blob/5343a75b/src/java/org/apache/cassandra/schema/SchemaKeyspace.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/schema/SchemaKeyspace.java b/src/java/org/apache/cassandra/schema/SchemaKeyspace.java
index e4e50a0..c715a2a 100644
--- a/src/java/org/apache/cassandra/schema/SchemaKeyspace.java
+++ b/src/java/org/apache/cassandra/schema/SchemaKeyspace.java
@@ -629,7 +629,7 @@ public final class SchemaKeyspace
adder.add("column_name_bytes", column.name.bytes)
.add("kind", column.kind.toString().toLowerCase())
- .add("position", column.isOnAllComponents() ? ColumnDefinition.NO_POSITION : column.position())
+ .add("position", column.position())
.add("clustering_order", column.clusteringOrder().toString().toLowerCase())
.add("type", type.asCQL3Type().toString())
.build();
http://git-wip-us.apache.org/repos/asf/cassandra/blob/5343a75b/src/java/org/apache/cassandra/thrift/ThriftConversion.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/thrift/ThriftConversion.java b/src/java/org/apache/cassandra/thrift/ThriftConversion.java
index 4a1ff76..3443b6e 100644
--- a/src/java/org/apache/cassandra/thrift/ThriftConversion.java
+++ b/src/java/org/apache/cassandra/thrift/ThriftConversion.java
@@ -242,7 +242,7 @@ public class ThriftConversion
// historical reasons)
boolean hasKeyAlias = cf_def.isSetKey_alias() && keyValidator != null && !(keyValidator instanceof CompositeType);
if (hasKeyAlias)
- defs.add(ColumnDefinition.partitionKeyDef(cf_def.keyspace, cf_def.name, UTF8Type.instance.getString(cf_def.key_alias), keyValidator, ColumnDefinition.NO_POSITION));
+ defs.add(ColumnDefinition.partitionKeyDef(cf_def.keyspace, cf_def.name, UTF8Type.instance.getString(cf_def.key_alias), keyValidator, 0));
// Now add any CQL metadata that we want to copy, skipping the keyAlias if there was one
for (ColumnDefinition def : previousCQLMetadata)
@@ -381,7 +381,7 @@ public class ThriftConversion
}
else
{
- defs.add(ColumnDefinition.partitionKeyDef(ks, cf, names.defaultPartitionKeyName(), keyValidator, ColumnDefinition.NO_POSITION));
+ defs.add(ColumnDefinition.partitionKeyDef(ks, cf, names.defaultPartitionKeyName(), keyValidator, 0));
}
}
http://git-wip-us.apache.org/repos/asf/cassandra/blob/5343a75b/test/unit/org/apache/cassandra/db/ColumnsTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/db/ColumnsTest.java b/test/unit/org/apache/cassandra/db/ColumnsTest.java
index 921209e..4e3df80 100644
--- a/test/unit/org/apache/cassandra/db/ColumnsTest.java
+++ b/test/unit/org/apache/cassandra/db/ColumnsTest.java
@@ -381,7 +381,7 @@ public class ColumnsTest
private static void addPartition(List<String> names, List<ColumnDefinition> results)
{
for (String name : names)
- results.add(ColumnDefinition.partitionKeyDef(cfMetaData, bytes(name), UTF8Type.instance, ColumnDefinition.NO_POSITION));
+ results.add(ColumnDefinition.partitionKeyDef(cfMetaData, bytes(name), UTF8Type.instance, 0));
}
private static void addClustering(List<String> names, List<ColumnDefinition> results)
http://git-wip-us.apache.org/repos/asf/cassandra/blob/5343a75b/test/unit/org/apache/cassandra/schema/LegacySchemaMigratorTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/schema/LegacySchemaMigratorTest.java b/test/unit/org/apache/cassandra/schema/LegacySchemaMigratorTest.java
index bc382fb..ad75fd0 100644
--- a/test/unit/org/apache/cassandra/schema/LegacySchemaMigratorTest.java
+++ b/test/unit/org/apache/cassandra/schema/LegacySchemaMigratorTest.java
@@ -639,7 +639,7 @@ public class LegacySchemaMigratorTest
adder.add("validator", column.type.toString())
.add("type", serializeKind(column.kind, table.isDense()))
- .add("component_index", column.isOnAllComponents() ? null : column.position());
+ .add("component_index", column.position());
Optional<IndexMetadata> index = findIndexForColumn(table.getIndexes(), table, column);
if (index.isPresent())