From 5ffae4f45a0b8ca7d8eef7d5fc08c32905c67dae Mon Sep 17 00:00:00 2001 From: Sam Tunnicliffe Date: Fri, 2 Oct 2015 13:27:45 +0100 Subject: [PATCH] Custom index expression in SELECT forces index selection Patch by Sam Tunnicliffe; reviewed by Sylvain Lebresne for CASSANDRA-10436 --- NEWS.txt | 4 +- .../restrictions/CustomIndexExpression.java | 6 +- .../restrictions/StatementRestrictions.java | 7 +- .../index/SecondaryIndexManager.java | 6 +- .../cassandra/index/CustomIndexTest.java | 75 +++++++++++++++++++ 5 files changed, 91 insertions(+), 7 deletions(-) diff --git a/NEWS.txt b/NEWS.txt index e4b86630ee..a7e56eca70 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 diff --git a/src/java/org/apache/cassandra/cql3/restrictions/CustomIndexExpression.java b/src/java/org/apache/cassandra/cql3/restrictions/CustomIndexExpression.java index 65c1bb36ea..eb91928547 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); } diff --git a/src/java/org/apache/cassandra/cql3/restrictions/StatementRestrictions.java b/src/java/org/apache/cassandra/cql3/restrictions/StatementRestrictions.java index 1bd42183c2..1c7db4e0fa 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); } diff --git a/src/java/org/apache/cassandra/index/SecondaryIndexManager.java b/src/java/org/apache/cassandra/index/SecondaryIndexManager.java index b04de2ad1c..9b15a3e38c 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 { diff --git a/test/unit/org/apache/cassandra/index/CustomIndexTest.java b/test/unit/org/apache/cassandra/index/CustomIndexTest.java index ceff839003..b2c9257896 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;