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/trunk Commit: 0521f8dc5d5e05c0530726e9549fa2481726a818 Parents: 5050aa3 Author: Alex Petrov <oleksandr.pet...@gmail.com> Authored: Mon Dec 11 18:56:40 2017 +0100 Committer: Alex Petrov <oleksandr.pet...@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-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org