Allow filtering on clustering columns for queries without secondary indexes
patch by Alex Petrov; reviewed by Benjamin Lerer for CASSANDRA-11310 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/a600920c Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/a600920c Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/a600920c Branch: refs/heads/trunk Commit: a600920cb5ee2866b09ee6c1ebae9518096e5bc4 Parents: 831bebd Author: Alex Petrov <[email protected]> Authored: Fri Apr 15 22:26:02 2016 +0200 Committer: Benjamin Lerer <[email protected]> Committed: Fri Apr 15 22:26:02 2016 +0200 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../ClusteringColumnRestrictions.java | 80 +- .../restrictions/StatementRestrictions.java | 58 +- .../entities/FrozenCollectionsTest.java | 9 +- .../cql3/validation/operations/SelectTest.java | 1067 ++++++++++++------ 5 files changed, 800 insertions(+), 415 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/a600920c/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index acbbfa5..7bb97e5 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 3.6 + * Allow filtering on clustering columns for queries without secondary indexes (CASSANDRA-11310) * Refactor Restriction hierarchy (CASSANDRA-11354) * Eliminate allocations in R/W path (CASSANDRA-11421) * Update Netty to 4.0.36 (CASSANDRA-11567) http://git-wip-us.apache.org/repos/asf/cassandra/blob/a600920c/src/java/org/apache/cassandra/cql3/restrictions/ClusteringColumnRestrictions.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql3/restrictions/ClusteringColumnRestrictions.java b/src/java/org/apache/cassandra/cql3/restrictions/ClusteringColumnRestrictions.java index 47cf76b..ab16ebc 100644 --- a/src/java/org/apache/cassandra/cql3/restrictions/ClusteringColumnRestrictions.java +++ b/src/java/org/apache/cassandra/cql3/restrictions/ClusteringColumnRestrictions.java @@ -42,16 +42,28 @@ final class ClusteringColumnRestrictions extends RestrictionSetWrapper */ protected final ClusteringComparator comparator; + /** + * <code>true</code> if filtering is allowed for this restriction, <code>false</code> otherwise + */ + private final boolean allowFiltering; + public ClusteringColumnRestrictions(CFMetaData cfm) { - super(new RestrictionSet()); - this.comparator = cfm.comparator; + this(cfm, false); } - private ClusteringColumnRestrictions(ClusteringComparator comparator, RestrictionSet restrictionSet) + public ClusteringColumnRestrictions(CFMetaData cfm, boolean allowFiltering) + { + this(cfm.comparator, new RestrictionSet(), allowFiltering); + } + + private ClusteringColumnRestrictions(ClusteringComparator comparator, + RestrictionSet restrictionSet, + boolean allowFiltering) { super(restrictionSet); this.comparator = comparator; + this.allowFiltering = allowFiltering; } public ClusteringColumnRestrictions mergeWith(Restriction restriction) throws InvalidRequestException @@ -59,7 +71,7 @@ final class ClusteringColumnRestrictions extends RestrictionSetWrapper SingleRestriction newRestriction = (SingleRestriction) restriction; RestrictionSet newRestrictionSet = restrictions.addRestriction(newRestriction); - if (!isEmpty()) + if (!isEmpty() && !allowFiltering) { SingleRestriction lastRestriction = restrictions.lastRestriction(); ColumnDefinition lastRestrictionStart = lastRestriction.getFirstColumn(); @@ -76,7 +88,7 @@ final class ClusteringColumnRestrictions extends RestrictionSetWrapper newRestrictionStart.name); } - return new ClusteringColumnRestrictions(this.comparator, newRestrictionSet); + return new ClusteringColumnRestrictions(this.comparator, newRestrictionSet, allowFiltering); } private boolean hasMultiColumnSlice() @@ -105,11 +117,10 @@ final class ClusteringColumnRestrictions extends RestrictionSetWrapper { MultiCBuilder builder = MultiCBuilder.create(comparator, hasIN() || hasMultiColumnSlice()); int keyPosition = 0; + for (SingleRestriction r : restrictions) { - ColumnDefinition def = r.getFirstColumn(); - - if (keyPosition != def.position() || r.isContains() || r.isLIKE()) + if (handleInFilter(r, keyPosition)) break; if (r.isSlice()) @@ -155,6 +166,32 @@ final class ClusteringColumnRestrictions extends RestrictionSetWrapper return restrictions.stream().anyMatch(SingleRestriction::isSlice); } + /** + * Checks if underlying restrictions would require filtering + * + * @return <code>true</code> if any underlying restrictions require filtering, <code>false</code> + * otherwise + */ + public final boolean needFiltering() + { + int position = 0; + SingleRestriction slice = null; + for (SingleRestriction restriction : restrictions) + { + if (handleInFilter(restriction, position)) + return true; + + if (slice != null && !slice.getFirstColumn().equals(restriction.getFirstColumn())) + return true; + + if (slice == null && restriction.isSlice()) + slice = restriction; + else + position = restriction.getLastColumn().position() + 1; + } + return hasContains(); + } + @Override public void addRowFilterTo(RowFilter filter, SecondaryIndexManager indexManager, @@ -162,18 +199,31 @@ final class ClusteringColumnRestrictions extends RestrictionSetWrapper { int position = 0; + SingleRestriction slice = null; for (SingleRestriction restriction : restrictions) { - ColumnDefinition columnDef = restriction.getFirstColumn(); - // We ignore all the clustering columns that can be handled by slices. - if (!(restriction.isContains() || restriction.isLIKE()) && position == columnDef.position()) + if (handleInFilter(restriction, position) || restriction.hasSupportingIndex(indexManager)) { - position = restriction.getLastColumn().position() + 1; - if (!restriction.hasSupportingIndex(indexManager)) - continue; + restriction.addRowFilterTo(filter, indexManager, options); + continue; } - restriction.addRowFilterTo(filter, indexManager, options); + + if (slice != null && !slice.getFirstColumn().equals(restriction.getFirstColumn())) + { + restriction.addRowFilterTo(filter, indexManager, options); + continue; + } + + if (slice == null && restriction.isSlice()) + slice = restriction; + else + position = restriction.getLastColumn().position() + 1; } } + + private boolean handleInFilter(SingleRestriction restriction, int index) { + return restriction.isContains() || restriction.isLIKE() || index != restriction.getFirstColumn().position(); + } + } http://git-wip-us.apache.org/repos/asf/cassandra/blob/a600920c/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 032a622..b146cfd 100644 --- a/src/java/org/apache/cassandra/cql3/restrictions/StatementRestrictions.java +++ b/src/java/org/apache/cassandra/cql3/restrictions/StatementRestrictions.java @@ -85,7 +85,7 @@ public final class StatementRestrictions /** * The restrictions used to build the row filter */ - private final IndexRestrictions indexRestrictions = new IndexRestrictions(); + private final IndexRestrictions filterRestrictions = new IndexRestrictions(); /** * <code>true</code> if the secondary index need to be queried, <code>false</code> otherwise @@ -106,15 +106,15 @@ public final class StatementRestrictions */ public static StatementRestrictions empty(StatementType type, CFMetaData cfm) { - return new StatementRestrictions(type, cfm); + return new StatementRestrictions(type, cfm, false); } - private StatementRestrictions(StatementType type, CFMetaData cfm) + private StatementRestrictions(StatementType type, CFMetaData cfm, boolean allowFiltering) { this.type = type; this.cfm = cfm; this.partitionKeyRestrictions = new PartitionKeySingleRestrictionSet(cfm.getKeyValidatorAsClusteringComparator()); - this.clusteringColumnsRestrictions = new ClusteringColumnRestrictions(cfm); + this.clusteringColumnsRestrictions = new ClusteringColumnRestrictions(cfm, allowFiltering); this.nonPrimaryKeyRestrictions = new RestrictionSet(); this.notNullColumns = new HashSet<>(); } @@ -125,10 +125,10 @@ public final class StatementRestrictions VariableSpecifications boundNames, boolean selectsOnlyStaticColumns, boolean selectsComplexColumn, - boolean useFiltering, - boolean forView) throws InvalidRequestException + boolean allowFiltering, + boolean forView) { - this(type, cfm); + this(type, cfm, allowFiltering); ColumnFamilyStore cfs; @@ -185,7 +185,7 @@ public final class StatementRestrictions processCustomIndexExpressions(whereClause.expressions, boundNames, secondaryIndexManager); hasQueriableClusteringColumnIndex = clusteringColumnsRestrictions.hasSupportingIndex(secondaryIndexManager); - hasQueriableIndex = !indexRestrictions.getCustomIndexExpressions().isEmpty() + hasQueriableIndex = !filterRestrictions.getCustomIndexExpressions().isEmpty() || hasQueriableClusteringColumnIndex || partitionKeyRestrictions.hasSupportingIndex(secondaryIndexManager) || nonPrimaryKeyRestrictions.hasSupportingIndex(secondaryIndexManager); @@ -197,7 +197,7 @@ public final class StatementRestrictions // Some but not all of the partition key columns have been specified; // hence we need turn these restrictions into a row filter. if (usesSecondaryIndexing) - indexRestrictions.add(partitionKeyRestrictions); + filterRestrictions.add(partitionKeyRestrictions); if (selectsOnlyStaticColumns && hasClusteringColumnsRestriction()) { @@ -218,16 +218,18 @@ public final class StatementRestrictions throw invalidRequest("Cannot restrict clustering columns when selecting only static columns"); } - processClusteringColumnsRestrictions(hasQueriableIndex, selectsOnlyStaticColumns, selectsComplexColumn, forView); + processClusteringColumnsRestrictions(hasQueriableIndex, + selectsOnlyStaticColumns, + selectsComplexColumn, + forView, + allowFiltering); // Covers indexes on the first clustering column (among others). if (isKeyRange && hasQueriableClusteringColumnIndex) usesSecondaryIndexing = true; - usesSecondaryIndexing = usesSecondaryIndexing || clusteringColumnsRestrictions.hasContains(); - - if (usesSecondaryIndexing) - indexRestrictions.add(clusteringColumnsRestrictions); + if (usesSecondaryIndexing || clusteringColumnsRestrictions.needFiltering()) + filterRestrictions.add(clusteringColumnsRestrictions); // Even if usesSecondaryIndexing is false at this point, we'll still have to use one if // there is restrictions not covered by the PK. @@ -243,10 +245,10 @@ public final class StatementRestrictions } if (hasQueriableIndex) usesSecondaryIndexing = true; - else if (!useFiltering) + else if (!allowFiltering) throw invalidRequest(StatementRestrictions.REQUIRES_ALLOW_FILTERING_MESSAGE); - indexRestrictions.add(nonPrimaryKeyRestrictions); + filterRestrictions.add(nonPrimaryKeyRestrictions); } if (usesSecondaryIndexing) @@ -274,7 +276,7 @@ public final class StatementRestrictions // may be used by QueryHandler implementations public IndexRestrictions getIndexRestrictions() { - return indexRestrictions; + return filterRestrictions; } /** @@ -285,7 +287,7 @@ public final class StatementRestrictions public Set<ColumnDefinition> nonPKRestrictedColumns(boolean includeNotNullRestrictions) { Set<ColumnDefinition> columns = new HashSet<>(); - for (Restrictions r : indexRestrictions.getRestrictions()) + for (Restrictions r : filterRestrictions.getRestrictions()) { for (ColumnDefinition def : r.getColumnDefs()) if (!def.isPrimaryKeyColumn()) @@ -451,7 +453,8 @@ public final class StatementRestrictions private void processClusteringColumnsRestrictions(boolean hasQueriableIndex, boolean selectsOnlyStaticColumns, boolean selectsComplexColumn, - boolean forView) throws InvalidRequestException + boolean forView, + boolean allowFiltering) { checkFalse(!type.allowClusteringColumnSlices() && clusteringColumnsRestrictions.hasSlice(), "Slice restrictions are not supported on the clustering columns in %s statements", type); @@ -467,9 +470,10 @@ public final class StatementRestrictions { checkFalse(clusteringColumnsRestrictions.hasIN() && selectsComplexColumn, "Cannot restrict clustering columns by IN relations when a collection is selected by the query"); - checkFalse(clusteringColumnsRestrictions.hasContains() && !hasQueriableIndex, + checkFalse(clusteringColumnsRestrictions.hasContains() && !hasQueriableIndex && !allowFiltering, "Cannot restrict clustering columns by a CONTAINS relation without a secondary index"); + if (hasClusteringColumnsRestriction()) { List<ColumnDefinition> clusteringColumns = cfm.clusteringColumns(); @@ -480,7 +484,7 @@ public final class StatementRestrictions ColumnDefinition clusteringColumn = clusteringColumns.get(i); ColumnDefinition restrictedColumn = restrictedColumns.get(i); - if (!clusteringColumn.equals(restrictedColumn)) + if (!clusteringColumn.equals(restrictedColumn) && !allowFiltering) { checkTrue(hasQueriableIndex || forView, "PRIMARY KEY column \"%s\" cannot be restricted as preceding column \"%s\" is not restricted", @@ -552,19 +556,19 @@ public final class StatementRestrictions expression.prepareValue(cfm, expressionType, boundNames); - indexRestrictions.add(expression); + filterRestrictions.add(expression); } public RowFilter getRowFilter(SecondaryIndexManager indexManager, QueryOptions options) { - if (indexRestrictions.isEmpty()) + if (filterRestrictions.isEmpty()) return RowFilter.NONE; RowFilter filter = RowFilter.create(); - for (Restrictions restrictions : indexRestrictions.getRestrictions()) + for (Restrictions restrictions : filterRestrictions.getRestrictions()) restrictions.addRowFilterTo(filter, indexManager, options); - for (CustomIndexExpression expression : indexRestrictions.getCustomIndexExpressions()) + for (CustomIndexExpression expression : filterRestrictions.getCustomIndexExpressions()) expression.addToRowFilter(filter, cfm, options); return filter; @@ -743,8 +747,8 @@ public final class StatementRestrictions */ public boolean needFiltering() { - int numberOfRestrictions = indexRestrictions.getCustomIndexExpressions().size(); - for (Restrictions restrictions : indexRestrictions.getRestrictions()) + int numberOfRestrictions = filterRestrictions.getCustomIndexExpressions().size(); + for (Restrictions restrictions : filterRestrictions.getRestrictions()) numberOfRestrictions += restrictions.size(); return numberOfRestrictions > 1 http://git-wip-us.apache.org/repos/asf/cassandra/blob/a600920c/test/unit/org/apache/cassandra/cql3/validation/entities/FrozenCollectionsTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/cql3/validation/entities/FrozenCollectionsTest.java b/test/unit/org/apache/cassandra/cql3/validation/entities/FrozenCollectionsTest.java index 9df8ea0..4aa178a 100644 --- a/test/unit/org/apache/cassandra/cql3/validation/entities/FrozenCollectionsTest.java +++ b/test/unit/org/apache/cassandra/cql3/validation/entities/FrozenCollectionsTest.java @@ -633,14 +633,15 @@ public class FrozenCollectionsTest extends CQLTester assertInvalidMessage("Cannot restrict clustering columns by a CONTAINS relation without a secondary index", "SELECT * FROM %s WHERE b CONTAINS ?", 1); - assertInvalidMessage("Cannot restrict clustering columns by a CONTAINS relation without a secondary index", - "SELECT * FROM %s WHERE b CONTAINS ? ALLOW FILTERING", 1); + assertRows(execute("SELECT * FROM %s WHERE b CONTAINS ? ALLOW FILTERING", 1), + row(0, list(1, 2, 3), set(1, 2, 3), map(1, "a")), + row(1, list(1, 2, 3), set(4, 5, 6), map(2, "b"))); assertInvalidMessage(StatementRestrictions.REQUIRES_ALLOW_FILTERING_MESSAGE, "SELECT * FROM %s WHERE d CONTAINS KEY ?", 1); - assertInvalidMessage("Cannot restrict clustering columns by a CONTAINS relation without a secondary index", - "SELECT * FROM %s WHERE b CONTAINS ? AND d CONTAINS KEY ? ALLOW FILTERING", 1, 1); + assertRows(execute("SELECT * FROM %s WHERE b CONTAINS ? AND d CONTAINS KEY ? ALLOW FILTERING", 1, 1), + row(0, list(1, 2, 3), set(1, 2, 3), map(1, "a"))); // index lookup on b assertRows(execute("SELECT * FROM %s WHERE b=?", list(1, 2, 3)),
