Custom index expression in SELECT forces index selection Patch by Sam Tunnicliffe; reviewed by Sylvain Lebresne for CASSANDRA-10436
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/5ffae4f4 Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/5ffae4f4 Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/5ffae4f4 Branch: refs/heads/trunk Commit: 5ffae4f45a0b8ca7d8eef7d5fc08c32905c67dae Parents: 2d47237 Author: Sam Tunnicliffe <[email protected]> Authored: Fri Oct 2 13:27:45 2015 +0100 Committer: Sam Tunnicliffe <[email protected]> Committed: Fri Oct 2 18:05:41 2015 +0100 ---------------------------------------------------------------------- NEWS.txt | 4 +- .../restrictions/CustomIndexExpression.java | 6 +- .../restrictions/StatementRestrictions.java | 7 +- .../cassandra/index/SecondaryIndexManager.java | 6 +- .../apache/cassandra/index/CustomIndexTest.java | 75 ++++++++++++++++++++ 5 files changed, 91 insertions(+), 7 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/5ffae4f4/NEWS.txt ---------------------------------------------------------------------- diff --git a/NEWS.txt b/NEWS.txt index e4b8663..a7e56ec 100644 --- a/NEWS.txt +++ b/NEWS.txt @@ -111,7 +111,9 @@ Upgrading - SizeTieredCompactionStrategy parameter cold_reads_to_omit has been removed. - The secondary index API has been comprehensively reworked. This will be a breaking change for any custom index implementations, which should now look to implement - the new org.apache.cassandra.index.Index interface. + the new org.apache.cassandra.index.Index interface. New syntax has been added to create + and query row-based indexes, which are not explicitly linked to a single column in the + base table. 2.2.2 http://git-wip-us.apache.org/repos/asf/cassandra/blob/5ffae4f4/src/java/org/apache/cassandra/cql3/restrictions/CustomIndexExpression.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql3/restrictions/CustomIndexExpression.java b/src/java/org/apache/cassandra/cql3/restrictions/CustomIndexExpression.java index 65c1bb3..eb91928 100644 --- a/src/java/org/apache/cassandra/cql3/restrictions/CustomIndexExpression.java +++ b/src/java/org/apache/cassandra/cql3/restrictions/CustomIndexExpression.java @@ -21,7 +21,7 @@ package org.apache.cassandra.cql3.restrictions; import org.apache.cassandra.config.CFMetaData; import org.apache.cassandra.cql3.*; import org.apache.cassandra.db.filter.RowFilter; -import org.apache.cassandra.db.marshal.UTF8Type; +import org.apache.cassandra.db.marshal.AbstractType; public class CustomIndexExpression { @@ -38,9 +38,9 @@ public class CustomIndexExpression this.valueRaw = value; } - public void prepareValue(CFMetaData cfm, VariableSpecifications boundNames) + public void prepareValue(CFMetaData cfm, AbstractType<?> expressionType, VariableSpecifications boundNames) { - ColumnSpecification spec = new ColumnSpecification(cfm.ksName, cfm.ksName, valueColId, UTF8Type.instance); + ColumnSpecification spec = new ColumnSpecification(cfm.ksName, cfm.ksName, valueColId, expressionType); value = valueRaw.prepare(cfm.ksName, spec); value.collectMarkerSpecification(boundNames); } http://git-wip-us.apache.org/repos/asf/cassandra/blob/5ffae4f4/src/java/org/apache/cassandra/cql3/restrictions/StatementRestrictions.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql3/restrictions/StatementRestrictions.java b/src/java/org/apache/cassandra/cql3/restrictions/StatementRestrictions.java index 1bd4218..1c7db4e 100644 --- a/src/java/org/apache/cassandra/cql3/restrictions/StatementRestrictions.java +++ b/src/java/org/apache/cassandra/cql3/restrictions/StatementRestrictions.java @@ -31,6 +31,7 @@ import org.apache.cassandra.cql3.statements.Bound; import org.apache.cassandra.cql3.statements.StatementType; import org.apache.cassandra.db.*; import org.apache.cassandra.db.filter.RowFilter; +import org.apache.cassandra.db.marshal.AbstractType; import org.apache.cassandra.dht.*; import org.apache.cassandra.exceptions.InvalidRequestException; import org.apache.cassandra.index.Index; @@ -503,7 +504,6 @@ public final class StatementRestrictions throw new InvalidRequestException(IndexRestrictions.MULTIPLE_EXPRESSIONS); CustomIndexExpression expression = expressions.get(0); - expression.prepareValue(cfm, boundNames); CFName cfName = expression.targetIndex.getCfName(); if (cfName.hasKeyspace() @@ -521,9 +521,12 @@ public final class StatementRestrictions if (!index.getIndexMetadata().isCustom()) throw IndexRestrictions.nonCustomIndexInExpression(expression.targetIndex); - if (index.customExpressionValueType() == null) + AbstractType<?> expressionType = index.customExpressionValueType(); + if (expressionType == null) throw IndexRestrictions.customExpressionNotSupported(expression.targetIndex); + expression.prepareValue(cfm, expressionType, boundNames); + indexRestrictions.add(expression); } http://git-wip-us.apache.org/repos/asf/cassandra/blob/5ffae4f4/src/java/org/apache/cassandra/index/SecondaryIndexManager.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/index/SecondaryIndexManager.java b/src/java/org/apache/cassandra/index/SecondaryIndexManager.java index b04de2a..9b15a3e 100644 --- a/src/java/org/apache/cassandra/index/SecondaryIndexManager.java +++ b/src/java/org/apache/cassandra/index/SecondaryIndexManager.java @@ -585,8 +585,12 @@ public class SecondaryIndexManager implements IndexRegistry { if (expression.isCustom()) { + // Only a single custom expression is allowed per query and, if present, + // we want to always favour the index specified in such an expression RowFilter.CustomExpression customExpression = (RowFilter.CustomExpression)expression; - searchableIndexes.add(indexes.get(customExpression.getTargetIndex().name)); + logger.trace("Command contains a custom index expression, using target index {}", customExpression.getTargetIndex().name); + Tracing.trace("Command contains a custom index expression, using target index {}", customExpression.getTargetIndex().name); + return indexes.get(customExpression.getTargetIndex().name); } else { http://git-wip-us.apache.org/repos/asf/cassandra/blob/5ffae4f4/test/unit/org/apache/cassandra/index/CustomIndexTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/index/CustomIndexTest.java b/test/unit/org/apache/cassandra/index/CustomIndexTest.java index ceff839..b2c9257 100644 --- a/test/unit/org/apache/cassandra/index/CustomIndexTest.java +++ b/test/unit/org/apache/cassandra/index/CustomIndexTest.java @@ -17,6 +17,8 @@ import org.apache.cassandra.cql3.statements.SelectStatement; import org.apache.cassandra.db.ColumnFamilyStore; import org.apache.cassandra.db.ReadCommand; import org.apache.cassandra.db.marshal.AbstractType; +import org.apache.cassandra.db.marshal.Int32Type; +import org.apache.cassandra.db.marshal.UTF8Type; import org.apache.cassandra.exceptions.InvalidRequestException; import org.apache.cassandra.schema.IndexMetadata; import org.apache.cassandra.schema.Indexes; @@ -403,6 +405,53 @@ public class CustomIndexTest extends CQLTester assertEquals(1, lessSelective.searchersProvided); } + @Test + public void customExpressionForcesIndexSelection() throws Throwable + { + createTable("CREATE TABLE %s(a int, b int, c int, PRIMARY KEY (a))"); + createIndex(String.format("CREATE CUSTOM INDEX more_selective ON %%s(b) USING '%s'", + SettableSelectivityIndex.class.getName())); + createIndex(String.format("CREATE CUSTOM INDEX less_selective ON %%s(c) USING '%s'", + SettableSelectivityIndex.class.getName())); + SettableSelectivityIndex moreSelective = + (SettableSelectivityIndex)getCurrentColumnFamilyStore().indexManager.getIndexByName("more_selective"); + SettableSelectivityIndex lessSelective = + (SettableSelectivityIndex)getCurrentColumnFamilyStore().indexManager.getIndexByName("less_selective"); + assertEquals(0, moreSelective.searchersProvided); + assertEquals(0, lessSelective.searchersProvided); + + // without a custom expression, the more selective index should be chosen + moreSelective.setEstimatedResultRows(1); + lessSelective.setEstimatedResultRows(1000); + execute("SELECT * FROM %s WHERE b=0 AND c=0 ALLOW FILTERING"); + assertEquals(1, moreSelective.searchersProvided); + assertEquals(0, lessSelective.searchersProvided); + + // when a custom expression is present, its target index should be preferred + execute("SELECT * FROM %s WHERE b=0 AND expr(less_selective, 'expression') ALLOW FILTERING"); + assertEquals(1, moreSelective.searchersProvided); + assertEquals(1, lessSelective.searchersProvided); + } + + @Test + public void testCustomExpressionValueType() throws Throwable + { + // verify that the type of the expression value is determined by Index::customExpressionValueType + createTable("CREATE TABLE %s (k int, v1 uuid, v2 blob, PRIMARY KEY(k))"); + createIndex(String.format("CREATE CUSTOM INDEX int_index ON %%s() USING '%s'", + Int32ExpressionIndex.class.getName())); + createIndex(String.format("CREATE CUSTOM INDEX text_index ON %%s() USING '%s'", + UTF8ExpressionIndex.class.getName())); + + execute("SELECT * FROM %s WHERE expr(text_index, 'foo')"); + assertInvalidMessage("Invalid INTEGER constant (99) for \"custom index expression\" of type text", + "SELECT * FROM %s WHERE expr(text_index, 99)"); + + execute("SELECT * FROM %s WHERE expr(int_index, 99)"); + assertInvalidMessage("Invalid STRING constant (foo) for \"custom index expression\" of type int", + "SELECT * FROM %s WHERE expr(int_index, 'foo')"); + } + private void testCreateIndex(String indexName, String... targetColumnNames) throws Throwable { createIndex(String.format("CREATE CUSTOM INDEX %s ON %%s(%s) USING '%s'", @@ -459,6 +508,32 @@ public class CustomIndexTest extends CQLTester } } + public static final class UTF8ExpressionIndex extends StubIndex + { + public UTF8ExpressionIndex(ColumnFamilyStore baseCfs, IndexMetadata metadata) + { + super(baseCfs, metadata); + } + + public AbstractType<?> customExpressionValueType() + { + return UTF8Type.instance; + } + } + + public static final class Int32ExpressionIndex extends StubIndex + { + public Int32ExpressionIndex(ColumnFamilyStore baseCfs, IndexMetadata metadata) + { + super(baseCfs, metadata); + } + + public AbstractType<?> customExpressionValueType() + { + return Int32Type.instance; + } + } + public static final class SettableSelectivityIndex extends StubIndex { private int searchersProvided = 0;
