You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by if...@apache.org on 2017/12/19 10:00:42 UTC

[2/6] cassandra git commit: Fix index target computation for dense composite tables with dropped compact storage

Fix index target computation for dense composite tables with dropped compact storage

Patch by Alex Petrov; reviewed by Zhao Yang for CASSANDRA-14104.

Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/0521f8dc
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/0521f8dc
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/0521f8dc

Branch: refs/heads/cassandra-3.11
Commit: 0521f8dc5d5e05c0530726e9549fa2481726a818
Parents: 5050aa3
Author: Alex Petrov <ol...@gmail.com>
Authored: Mon Dec 11 18:56:40 2017 +0100
Committer: Alex Petrov <ol...@gmail.com>
Committed: Tue Dec 19 09:55:00 2017 +0100

----------------------------------------------------------------------
 CHANGES.txt                                     |   1 +
 .../index/internal/CassandraIndex.java          |  33 ++++--
 .../DropCompactStorageThriftTest.java           | 116 +++++++++++++++++++
 3 files changed, 139 insertions(+), 11 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/0521f8dc/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 0574893..7746c73 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
 3.0.16
+ * Fix index target computation for dense composite tables with dropped compact storage (CASSANDRA-14104)
  * Improve commit log chain marker updating (CASSANDRA-14108)
  * Extra range tombstone bound creates double rows (CASSANDRA-14008)
  * Fix SStable ordering by max timestamp in SinglePartitionReadCommand (CASSANDRA-14010)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/0521f8dc/src/java/org/apache/cassandra/index/internal/CassandraIndex.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/index/internal/CassandraIndex.java b/src/java/org/apache/cassandra/index/internal/CassandraIndex.java
index 5caeefa..3211fe9 100644
--- a/src/java/org/apache/cassandra/index/internal/CassandraIndex.java
+++ b/src/java/org/apache/cassandra/index/internal/CassandraIndex.java
@@ -52,6 +52,7 @@ import org.apache.cassandra.db.partitions.PartitionIterator;
 import org.apache.cassandra.db.partitions.PartitionUpdate;
 import org.apache.cassandra.db.rows.*;
 import org.apache.cassandra.dht.LocalPartitioner;
+import org.apache.cassandra.exceptions.ConfigurationException;
 import org.apache.cassandra.exceptions.InvalidRequestException;
 import org.apache.cassandra.index.Index;
 import org.apache.cassandra.index.IndexRegistry;
@@ -790,13 +791,20 @@ public abstract class CassandraIndex implements Index
         return getFunctions(indexMetadata, parseTarget(baseCfs.metadata, indexMetadata)).newIndexInstance(baseCfs, indexMetadata);
     }
 
-    // Public because it's also used to convert index metadata into a thrift-compatible format
-    public static Pair<ColumnDefinition, IndexTarget.Type> parseTarget(CFMetaData cfm,
-                                                                       IndexMetadata indexDef)
+    public static Pair<ColumnDefinition, IndexTarget.Type> parseTarget(CFMetaData cfm, IndexMetadata indexDef)
     {
         String target = indexDef.options.get("target");
         assert target != null : String.format("No target definition found for index %s", indexDef.name);
+        Pair<ColumnDefinition, IndexTarget.Type> result = parseTarget(cfm, target);
+        if (result == null)
+            throw new ConfigurationException(String.format("Unable to parse targets for index %s (%s)", indexDef.name, target));
+        return result;
+    }
 
+    // Public because it's also used to convert index metadata into a thrift-compatible format
+    public static Pair<ColumnDefinition, IndexTarget.Type> parseTarget(CFMetaData cfm,
+                                                                       String target)
+    {
         // if the regex matches then the target is in the form "keys(foo)", "entries(bar)" etc
         // if not, then it must be a simple column name and implictly its type is VALUES
         Matcher matcher = TARGET_REGEX.matcher(target);
@@ -826,15 +834,18 @@ public abstract class CassandraIndex implements Index
         }
 
         // if it's not a CQL table, we can't assume that the column name is utf8, so
-        // in that case we have to do a linear scan of the cfm's columns to get the matching one
-        if (cfm.isCQLTable())
-            return Pair.create(cfm.getColumnDefinition(new ColumnIdentifier(columnName, true)), targetType);
-        else
-            for (ColumnDefinition column : cfm.allColumns())
-                if (column.name.toString().equals(columnName))
-                    return Pair.create(column, targetType);
+        // in that case we have to do a linear scan of the cfm's columns to get the matching one.
+        // After dropping compact storage (see CASSANDRA-10857), we can't distinguish between the
+        // former compact/thrift table, so we have to fall back to linear scan in both cases.
+        ColumnDefinition cd = cfm.getColumnDefinition(new ColumnIdentifier(columnName, true));
+        if (cd != null)
+            return Pair.create(cd, targetType);
 
-        throw new RuntimeException(String.format("Unable to parse targets for index %s (%s)", indexDef.name, target));
+        for (ColumnDefinition column : cfm.allColumns())
+            if (column.name.toString().equals(columnName))
+                return Pair.create(column, targetType);
+
+        return null;
     }
 
     static CassandraIndexFunctions getFunctions(IndexMetadata indexDef,

http://git-wip-us.apache.org/repos/asf/cassandra/blob/0521f8dc/test/unit/org/apache/cassandra/cql3/validation/operations/DropCompactStorageThriftTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/cql3/validation/operations/DropCompactStorageThriftTest.java b/test/unit/org/apache/cassandra/cql3/validation/operations/DropCompactStorageThriftTest.java
index dde3e7b..7d81018 100644
--- a/test/unit/org/apache/cassandra/cql3/validation/operations/DropCompactStorageThriftTest.java
+++ b/test/unit/org/apache/cassandra/cql3/validation/operations/DropCompactStorageThriftTest.java
@@ -19,15 +19,19 @@
 package org.apache.cassandra.cql3.validation.operations;
 
 import java.nio.ByteBuffer;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.UUID;
 
 import org.junit.Test;
 
+import org.apache.cassandra.config.CFMetaData;
 import org.apache.cassandra.config.ColumnDefinition;
 import org.apache.cassandra.cql3.ColumnSpecification;
 import org.apache.cassandra.cql3.UntypedResultSet;
+import org.apache.cassandra.cql3.statements.IndexTarget;
 import org.apache.cassandra.db.Keyspace;
 import org.apache.cassandra.db.marshal.AbstractType;
 import org.apache.cassandra.db.marshal.AsciiType;
@@ -37,7 +41,9 @@ import org.apache.cassandra.db.marshal.EmptyType;
 import org.apache.cassandra.db.marshal.Int32Type;
 import org.apache.cassandra.db.marshal.MapType;
 import org.apache.cassandra.db.marshal.UTF8Type;
+import org.apache.cassandra.index.internal.CassandraIndex;
 import org.apache.cassandra.locator.SimpleStrategy;
+import org.apache.cassandra.serializers.MarshalException;
 import org.apache.cassandra.thrift.Cassandra;
 import org.apache.cassandra.thrift.CfDef;
 import org.apache.cassandra.thrift.Column;
@@ -49,7 +55,11 @@ import org.apache.cassandra.thrift.KsDef;
 import org.apache.cassandra.thrift.Mutation;
 import org.apache.cassandra.thrift.SuperColumn;
 import org.apache.cassandra.utils.ByteBufferUtil;
+import org.apache.cassandra.utils.Pair;
+import org.apache.cassandra.utils.UUIDGen;
 
+import static junit.framework.Assert.assertFalse;
+import static junit.framework.Assert.assertTrue;
 import static org.apache.cassandra.thrift.ConsistencyLevel.ONE;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.fail;
@@ -491,6 +501,112 @@ public class DropCompactStorageThriftTest extends ThriftCQLTester
                    row("key1", "ckey2", "sval2", ByteBufferUtil.bytes("val2")));
     }
 
+    @Test
+    public void denseCompositeWithIndexesTest() throws Throwable
+    {
+        final String KEYSPACE = "thrift_dense_composite_table_test_ks";
+        final String TABLE = "dense_composite_table";
+
+        ByteBuffer aCol = createDynamicCompositeKey(ByteBufferUtil.bytes("a"));
+        ByteBuffer bCol = createDynamicCompositeKey(ByteBufferUtil.bytes("b"));
+        ByteBuffer cCol = createDynamicCompositeKey(ByteBufferUtil.bytes("c"));
+
+        String compositeType = "DynamicCompositeType(a => BytesType, b => TimeUUIDType, c => UTF8Type)";
+
+        CfDef cfDef = new CfDef();
+        cfDef.setName(TABLE);
+        cfDef.setComparator_type(compositeType);
+        cfDef.setKeyspace(KEYSPACE);
+
+        cfDef.setColumn_metadata(
+        Arrays.asList(new ColumnDef(aCol, "BytesType").setIndex_type(IndexType.KEYS).setIndex_name(KEYSPACE + "_a"),
+                      new ColumnDef(bCol, "BytesType").setIndex_type(IndexType.KEYS).setIndex_name(KEYSPACE + "_b"),
+                      new ColumnDef(cCol, "BytesType").setIndex_type(IndexType.KEYS).setIndex_name(KEYSPACE + "_c")));
+
+
+        KsDef ksDef = new KsDef(KEYSPACE,
+                                SimpleStrategy.class.getName(),
+                                Collections.singletonList(cfDef));
+        ksDef.setStrategy_options(Collections.singletonMap("replication_factor", "1"));
+
+        Cassandra.Client client = getClient();
+        client.system_add_keyspace(ksDef);
+        client.set_keyspace(KEYSPACE);
+
+        CFMetaData cfm = Keyspace.open(KEYSPACE).getColumnFamilyStore(TABLE).metadata;
+        assertFalse(cfm.isCQLTable());
+
+        List<Pair<ColumnDefinition, IndexTarget.Type>> compactTableTargets = new ArrayList<>();
+        compactTableTargets.add(CassandraIndex.parseTarget(cfm, "a"));
+        compactTableTargets.add(CassandraIndex.parseTarget(cfm, "b"));
+        compactTableTargets.add(CassandraIndex.parseTarget(cfm, "c"));
+
+        execute(String.format("ALTER TABLE %s.%s DROP COMPACT STORAGE", KEYSPACE, TABLE));
+        cfm = Keyspace.open(KEYSPACE).getColumnFamilyStore(TABLE).metadata;
+        assertTrue(cfm.isCQLTable());
+
+        List<Pair<ColumnDefinition, IndexTarget.Type>> cqlTableTargets = new ArrayList<>();
+        cqlTableTargets.add(CassandraIndex.parseTarget(cfm, "a"));
+        cqlTableTargets.add(CassandraIndex.parseTarget(cfm, "b"));
+        cqlTableTargets.add(CassandraIndex.parseTarget(cfm, "c"));
+
+        assertEquals(compactTableTargets, cqlTableTargets);
+    }
+
+    private static ByteBuffer createDynamicCompositeKey(Object... objects)
+    {
+        int length = 0;
+
+        for (Object object : objects)
+        {
+            length += 2 * Short.BYTES +  Byte.BYTES;
+            if (object instanceof String)
+                length += ((String) object).length();
+            else if (object instanceof UUID)
+                length += 2 * Long.BYTES;
+            else if (object instanceof ByteBuffer)
+                length += ((ByteBuffer) object).remaining();
+            else
+                throw new MarshalException(object.getClass().getName() + " is not recognized as a valid type for this composite");
+        }
+
+        ByteBuffer out = ByteBuffer.allocate(length);
+
+        for (Object object : objects)
+        {
+            if (object instanceof String)
+            {
+                String cast = (String) object;
+
+                out.putShort((short) (0x8000 | 's'));
+                out.putShort((short) cast.length());
+                out.put(cast.getBytes());
+                out.put((byte) 0);
+            }
+            else if (object instanceof UUID)
+            {
+                out.putShort((short) (0x8000 | 't'));
+                out.putShort((short) 16);
+                out.put(UUIDGen.decompose((UUID) object));
+                out.put((byte) 0);
+            }
+            else if (object instanceof ByteBuffer)
+            {
+                ByteBuffer bytes = ((ByteBuffer) object).duplicate();
+                out.putShort((short) (0x8000 | 'b'));
+                out.putShort((short) bytes.remaining());
+                out.put(bytes);
+                out.put((byte) 0);
+            }
+            else
+            {
+                throw new MarshalException(object.getClass().getName() + " is not recognized as a valid type for this composite");
+            }
+        }
+
+        return out;
+    }
+
     private Column getColumnForInsert(ByteBuffer columnName, ByteBuffer value)
     {
         Column column = new Column();


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org