cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From slebre...@apache.org
Subject cassandra git commit: Don't use -1 as position for partition key in schema
Date Fri, 06 Nov 2015 14:03:40 GMT
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 <sylvain@datastax.com>
Authored: Mon Nov 2 17:58:31 2015 +0100
Committer: Sylvain Lebresne <sylvain@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())


Mime
View raw message