Merge branch 'cassandra-2.0' into cassandra-2.1
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/9c7a601b Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/9c7a601b Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/9c7a601b Branch: refs/heads/trunk Commit: 9c7a601bbd6ea0df72f6acea8d4cda2fd13f949c Parents: ecf48dd 90a012a Author: Tyler Hobbs <[email protected]> Authored: Thu Mar 5 12:29:26 2015 -0600 Committer: Tyler Hobbs <[email protected]> Committed: Thu Mar 5 12:29:26 2015 -0600 ---------------------------------------------------------------------- CHANGES.txt | 2 + .../cql3/statements/MultiColumnRestriction.java | 2 +- .../cassandra/cql3/statements/Restriction.java | 2 +- .../cql3/statements/SelectStatement.java | 426 ++++---- .../statements/SingleColumnRestriction.java | 11 +- .../cassandra/cql3/MultiColumnRelationTest.java | 198 +++- .../cql3/statements/SelectStatementTest.java | 965 +++++++++++++++++++ 7 files changed, 1386 insertions(+), 220 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/9c7a601b/CHANGES.txt ---------------------------------------------------------------------- diff --cc CHANGES.txt index ea79e22,462a8d1..57dd97e --- a/CHANGES.txt +++ b/CHANGES.txt @@@ -1,36 -1,6 +1,38 @@@ -2.0.13: +2.1.4 + * Make SSTableRewriter.abort() more robust to failure (CASSANDRA-8832) + * Remove cold_reads_to_omit from STCS (CASSANDRA-8860) + * Make EstimatedHistogram#percentile() use ceil instead of floor (CASSANDRA-8883) + * Fix top partitions reporting wrong cardinality (CASSANDRA-8834) + * Fix rare NPE in KeyCacheSerializer (CASSANDRA-8067) + * Pick sstables for validation as late as possible inc repairs (CASSANDRA-8366) + * Fix commitlog getPendingTasks to not increment (CASSANDRA-8856) + * Fix parallelism adjustment in range and secondary index queries + when the first fetch does not satisfy the limit (CASSANDRA-8856) + * Check if the filtered sstables is non-empty in STCS (CASSANDRA-8843) + * Upgrade java-driver used for cassandra-stress (CASSANDRA-8842) + * Fix CommitLog.forceRecycleAllSegments() memory access error (CASSANDRA-8812) + * Improve assertions in Memory (CASSANDRA-8792) + * Fix SSTableRewriter cleanup (CASSANDRA-8802) + * Introduce SafeMemory for CompressionMetadata.Writer (CASSANDRA-8758) + * 'nodetool info' prints exception against older node (CASSANDRA-8796) + * Ensure SSTableReader.last corresponds exactly with the file end (CASSANDRA-8750) + * Make SSTableWriter.openEarly more robust and obvious (CASSANDRA-8747) + * Enforce SSTableReader.first/last (CASSANDRA-8744) + * Cleanup SegmentedFile API (CASSANDRA-8749) + * Avoid overlap with early compaction replacement (CASSANDRA-8683) + * Safer Resource Management++ (CASSANDRA-8707) + * Write partition size estimates into a system table (CASSANDRA-7688) + * cqlsh: Fix keys() and full() collection indexes in DESCRIBE output + (CASSANDRA-8154) + * Show progress of streaming in nodetool netstats (CASSANDRA-8886) + * IndexSummaryBuilder utilises offheap memory, and shares data between + each IndexSummary opened from it (CASSANDRA-8757) + * markCompacting only succeeds if the exact SSTableReader instances being + marked are in the live set (CASSANDRA-8689) + * cassandra-stress support for varint (CASSANDRA-8882) +Merged from 2.0: + * Fix regression in mixed single and multi-column relation support for + SELECT statements (CASSANDRA-8613) * Add ability to limit number of native connections (CASSANDRA-8086) * Add offline tool to relevel sstables (CASSANDRA-8301) * Preserve stream ID for more protocol errors (CASSANDRA-8848) http://git-wip-us.apache.org/repos/asf/cassandra/blob/9c7a601b/src/java/org/apache/cassandra/cql3/statements/MultiColumnRestriction.java ---------------------------------------------------------------------- diff --cc src/java/org/apache/cassandra/cql3/statements/MultiColumnRestriction.java index 96cb905,f643684..6946c98 --- a/src/java/org/apache/cassandra/cql3/statements/MultiColumnRestriction.java +++ b/src/java/org/apache/cassandra/cql3/statements/MultiColumnRestriction.java @@@ -59,7 -58,7 +59,7 @@@ public interface MultiColumnRestrictio */ public static class InWithValues extends SingleColumnRestriction.InWithValues implements MultiColumnRestriction.IN { -- public InWithValues(List<Term> values) ++ public InWithValues(List<? extends Term> values) { super(values); } http://git-wip-us.apache.org/repos/asf/cassandra/blob/9c7a601b/src/java/org/apache/cassandra/cql3/statements/Restriction.java ---------------------------------------------------------------------- diff --cc src/java/org/apache/cassandra/cql3/statements/Restriction.java index 659ed95,3d33bde..485fd22 --- a/src/java/org/apache/cassandra/cql3/statements/Restriction.java +++ b/src/java/org/apache/cassandra/cql3/statements/Restriction.java @@@ -68,10 -59,10 +68,10 @@@ public interface Restrictio /** Returns true if the start or end bound (depending on the argument) is inclusive, false otherwise */ public boolean isInclusive(Bound b); - public Relation.Type getRelation(Bound eocBound, Bound inclusiveBound); + public Operator getRelation(Bound eocBound, Bound inclusiveBound); - public IndexOperator getIndexOperator(Bound b); + public Operator getIndexOperator(Bound b); - public void setBound(ColumnIdentifier name, Operator type, Term t) throws InvalidRequestException; - public void setBound(Relation.Type type, Term t) throws InvalidRequestException; ++ public void setBound(Operator type, Term t) throws InvalidRequestException; } } http://git-wip-us.apache.org/repos/asf/cassandra/blob/9c7a601b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java ---------------------------------------------------------------------- diff --cc src/java/org/apache/cassandra/cql3/statements/SelectStatement.java index 9099ba7,6b3c781..fc5e4f6 --- a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java @@@ -715,19 -717,37 +716,39 @@@ public class SelectStatement implement // we always do a slice for CQL3 tables, so it's ok to ignore them here assert !isColumnRange(); - ColumnNameBuilder builder = cfDef.getColumnNameBuilder(); - Iterator<CFDefinition.Name> idIter = cfDef.clusteringColumns().iterator(); - while (idIter.hasNext()) + CBuilder builder = cfm.comparator.prefixBuilder(); - Iterator<ColumnDefinition> idIter = cfm.clusteringColumns().iterator(); - for (Restriction r : columnRestrictions) ++ ++ Iterator<ColumnDefinition> columnIter = cfm.clusteringColumns().iterator(); ++ while (columnIter.hasNext()) { - ColumnDefinition def = idIter.next(); - CFDefinition.Name name = idIter.next(); - Restriction r = columnRestrictions[name.position]; ++ ColumnDefinition def = columnIter.next(); ++ Restriction r = columnRestrictions[def.position()]; assert r != null && !r.isSlice(); if (r.isEQ()) { - ByteBuffer val = r.values(options).get(0); - if (val == null) - throw new InvalidRequestException(String.format("Invalid null value for clustering key part %s", def.name)); - builder.add(val); - List<ByteBuffer> values = r.values(variables); ++ List<ByteBuffer> values = r.values(options); + if (r.isMultiColumn()) + { + for (int i = 0, m = values.size(); i < m; i++) + { ++ ByteBuffer val = values.get(i); ++ + if (i != 0) - name = idIter.next(); ++ columnIter.next(); + - ByteBuffer val = values.get(i); + if (val == null) - throw new InvalidRequestException(String.format("Invalid null value in condition for column %s", name)); ++ throw new InvalidRequestException(String.format("Invalid null value for clustering key part %s", def.name)); + builder.add(val); + } + } + else + { - ByteBuffer val = values.get(0); ++ ByteBuffer val = r.values(options).get(0); + if (val == null) - throw new InvalidRequestException(String.format("Invalid null value for clustering key part %s", name.name)); ++ throw new InvalidRequestException(String.format("Invalid null value for clustering key part %s", def.name)); + builder.add(val); + } } else { @@@ -752,22 -775,32 +773,21 @@@ } return columns; } -- else ++ ++ // we have a multi-column IN restriction ++ List<List<ByteBuffer>> values = ((MultiColumnRestriction.IN) r).splitValues(options); ++ TreeSet<CellName> inValues = new TreeSet<>(cfm.comparator); ++ for (List<ByteBuffer> components : values) { -- // we have a multi-column IN restriction - List<List<ByteBuffer>> values = ((MultiColumnRestriction.IN) r).splitValues(options); - TreeSet<CellName> inValues = new TreeSet<>(cfm.comparator); - List<List<ByteBuffer>> values = ((MultiColumnRestriction.IN) r).splitValues(variables); - if (values.isEmpty()) - return null; - TreeSet<ByteBuffer> inValues = new TreeSet<>(cfDef.cfm.comparator); -- for (List<ByteBuffer> components : values) -- { - ColumnNameBuilder b = builder.copy(); -- for (int i = 0; i < components.size(); i++) - { -- if (components.get(i) == null) - throw new InvalidRequestException("Invalid null value in condition for column " + cfm.clusteringColumns().get(i + def.position())); - { - List<CFDefinition.Name> clusteringCols = new ArrayList<>(cfDef.clusteringColumns()); - throw new InvalidRequestException("Invalid null value in condition for clustering column " + clusteringCols.get(i + name.position)); - } - b.add(components.get(i)); - } - if (cfDef.isCompact) - inValues.add(b.build()); - else - inValues.addAll(addSelectedColumns(b)); - } - return inValues; ++ for (int i = 0; i < components.size(); i++) ++ if (components.get(i) == null) ++ throw new InvalidRequestException("Invalid null value in condition for column " ++ + cfm.clusteringColumns().get(i + def.position()).name); + - Composite prefix = builder.buildWith(components); - inValues.addAll(addSelectedColumns(prefix)); - } - return inValues; ++ Composite prefix = builder.buildWith(components); ++ inValues.addAll(addSelectedColumns(prefix)); } ++ return inValues; } } @@@ -827,38 -864,23 +847,24 @@@ return false; } - private static List<Composite> buildBound(Bound bound, - List<ColumnDefinition> defs, - Restriction[] restrictions, - boolean isReversed, - CType type, - QueryOptions options) throws InvalidRequestException + @VisibleForTesting - static List<ByteBuffer> buildBound(Bound bound, - List<CFDefinition.Name> names, - Restriction[] restrictions, - boolean isReversed, - CFDefinition cfDef, - ColumnNameBuilder builder, - List<ByteBuffer> variables) throws InvalidRequestException ++ static List<Composite> buildBound(Bound bound, ++ List<ColumnDefinition> defs, ++ Restriction[] restrictions, ++ boolean isReversed, ++ CType type, ++ QueryOptions options) throws InvalidRequestException { + CBuilder builder = type.builder(); + - // check the first restriction to see if we're dealing with a multi-column restriction - if (!defs.isEmpty()) - { - Restriction firstRestriction = restrictions[0]; - if (firstRestriction != null && firstRestriction.isMultiColumn()) - { - if (firstRestriction.isSlice()) - return buildMultiColumnSliceBound(bound, defs, (MultiColumnRestriction.Slice) firstRestriction, isReversed, builder, options); - else if (firstRestriction.isIN()) - return buildMultiColumnInBound(bound, defs, (MultiColumnRestriction.IN) firstRestriction, isReversed, builder, type, options); - else - return buildMultiColumnEQBound(bound, defs, (MultiColumnRestriction.EQ) firstRestriction, isReversed, builder, options); - } - } - // The end-of-component of composite doesn't depend on whether the // component type is reversed or not (i.e. the ReversedType is applied // to the component comparator but not to the end-of-component itself), // it only depends on whether the slice is reversed Bound eocBound = isReversed ? Bound.reverse(bound) : bound; - for (Iterator<ColumnDefinition> iter = defs.iterator(); iter.hasNext();) - for (int i = 0; i < names.size(); i++) ++ for (int i = 0, m = defs.size(); i < m; i++) { - ColumnDefinition def = iter.next(); - CFDefinition.Name name = names.get(i); ++ ColumnDefinition def = defs.get(i); // In a restriction, we always have Bound.START < Bound.END for the "base" comparator. // So if we're doing a reverse slice, we must inverse the bounds when giving them as start and end of the slice filter. @@@ -875,165 -898,132 +881,140 @@@ } if (r.isSlice()) { - builder.add(getSliceValue(r, b, options)); + if (r.isMultiColumn()) + { - List<ByteBuffer> values = ((MultiColumnRestriction.Slice) r).componentBounds(b, variables); - List<Name> columns = subList(names, i, values.size()); - addComponents(builder, columns, values); ++ MultiColumnRestriction.Slice slice = (MultiColumnRestriction.Slice) r; ++ ++ if (!slice.hasBound(b)) ++ { ++ Composite prefix = builder.build(); ++ return Collections.singletonList(builder.remainingCount() > 0 && eocBound == Bound.END ++ ? prefix.end() ++ : prefix); ++ } ++ ++ List<ByteBuffer> vals = slice.componentBounds(b, options); ++ ++ for (int j = 0, n = vals.size(); j < n; j++) ++ addValue(builder, defs.get(i + j), vals.get(j)) ; + } + else + { - addComponent(builder, name, getSliceValue(r, b, variables)); ++ builder.add(getSliceValue(r, b, options)); + } - - Relation.Type relType = ((Restriction.Slice)r).getRelation(eocBound, b); - return Collections.singletonList(builder.buildForRelation(relType)); + Operator relType = ((Restriction.Slice)r).getRelation(eocBound, b); + return Collections.singletonList(builder.build().withEOC(eocForRelation(relType))); } - else + + if (r.isIN()) { - // IN or EQ + // The IN query might not have listed the values in comparator order, so we need to re-sort + // the bounds lists to make sure the slices works correctly (also, to avoid duplicates). - TreeSet<ByteBuffer> inValues = new TreeSet<>(isReversed ? cfDef.cfm.comparator.reverseComparator : cfDef.cfm.comparator); ++ TreeSet<Composite> inValues = new TreeSet<>(isReversed ? type.reverseComparator() : type); + + if (r.isMultiColumn()) + { - List<List<ByteBuffer>> splitInValues = ((MultiColumnRestriction.IN) r).splitValues(variables); ++ List<List<ByteBuffer>> splitInValues = ((MultiColumnRestriction.IN) r).splitValues(options); ++ + for (List<ByteBuffer> components : splitInValues) + { - ColumnNameBuilder copy = builder.copy(); - List<Name> columns = subList(names, i, components.size()); - addComponents(copy, columns, components); ++ for (int j = 0; j < components.size(); j++) ++ if (components.get(j) == null) ++ throw new InvalidRequestException("Invalid null value in condition for column " + defs.get(i + j).name); + - inValues.add(buildColumnName(copy, eocBound)); ++ Composite prefix = builder.buildWith(components); ++ inValues.add(builder.remainingCount() == 0 ? prefix : addEOC(prefix, eocBound)); + } + return new ArrayList<>(inValues); + } + - List<ByteBuffer> values = r.values(variables); + List<ByteBuffer> values = r.values(options); if (values.size() != 1) { // IN query, we only support it on the clustering columns - assert name.position == names.size() - 1; - + assert def.position() == defs.size() - 1; - // The IN query might not have listed the values in comparator order, so we need to re-sort - // the bounds lists to make sure the slices works correctly (also, to avoid duplicates). - TreeSet<Composite> s = new TreeSet<>(isReversed ? type.reverseComparator() : type); for (ByteBuffer val : values) { - ColumnNameBuilder copy = builder.copy(); - addComponent(copy, name, val); - inValues.add(buildColumnName(copy, eocBound)); + if (val == null) - throw new InvalidRequestException(String.format("Invalid null clustering key part %s", def.name)); ++ throw new InvalidRequestException(String.format("Invalid null value in condition for column %s", ++ def.name)); + Composite prefix = builder.buildWith(val); + // See below for why this - s.add(builder.remainingCount() == 0 ? prefix : (eocBound == Bound.END ? prefix.end() : prefix.start())); ++ inValues.add(builder.remainingCount() == 0 ? prefix : addEOC(prefix, eocBound)); } - return new ArrayList<>(s); + return new ArrayList<>(inValues); } + } - ByteBuffer val = values.get(0); - if (val == null) - throw new InvalidRequestException(String.format("Invalid null clustering key part %s", def.name)); - builder.add(val); - List<ByteBuffer> values = r.values(variables); ++ List<ByteBuffer> values = r.values(options); ++ + if (r.isMultiColumn()) + { - List<Name> columns = subList(names, i, values.size()); - addComponents(builder, columns, values); - i += values.size() - 1; ++ for (int j = 0; j < values.size(); j++) ++ addValue(builder, defs.get(i + j), values.get(j)); ++ i += values.size() - 1; // skips the processed columns + } + else + { - addComponent(builder, name, values.get(0)); ++ addValue(builder, def, values.get(0)); } } - return Collections.singletonList(buildColumnName(builder, eocBound)); - } - - /** - * Returns a view of the portion of the list starting at the index offset and containing the number of elements - * specified. - * - * @param list the original list - * @param offset the index offset - * @param length the number of elements - * @return a view of the specified range within this list - */ - private static <T> List<T> subList(List<T> list, int offset, int length) - { - return list.subList(offset, offset + length); - } - - /** - * Builds the column name when there was not slice. - * - * @param builder the column name builder - * @param eocBound the End of Component bound - * @return the column name - */ - private static ByteBuffer buildColumnName(ColumnNameBuilder builder, Bound eocBound) - { + // Means no relation at all or everything was an equal // Note: if the builder is "full", there is no need to use the end-of-component bit. For columns selection, // it would be harmless to do it. However, we use this method got the partition key too. And when a query // with 2ndary index is done, and with the the partition provided with an EQ, we'll end up here, and in that // case using the eoc would be bad, since for the random partitioner we have no guarantee that - // builder.buildAsEndOfRange() will sort after builder.build() (see #5240). - return (eocBound == Bound.END && builder.remainingCount() > 0) ? builder.buildAsEndOfRange() : builder.build(); + // prefix.end() will sort after prefix (see #5240). + Composite prefix = builder.build(); - return Collections.singletonList(builder.remainingCount() == 0 ? prefix : (eocBound == Bound.END ? prefix.end() : prefix.start())); ++ return Collections.singletonList(builder.remainingCount() == 0 ? prefix : addEOC(prefix, eocBound)); + } + + /** - * Adds the specified component values to the specified builder. ++ * Adds an EOC to the specified Composite. + * - * @param builder the ColumnNameBuilder to which the values must be added - * @param names the column names - * @param values the values to add - * @throws InvalidRequestException if one of the values is null ++ * @param composite the composite ++ * @param eocBound the EOC bound ++ * @return a new <code>Composite</code> with the EOC corresponding to the eocBound + */ - private static void addComponents(ColumnNameBuilder builder, List<Name> names, List<ByteBuffer> values) throws InvalidRequestException ++ private static Composite addEOC(Composite composite, Bound eocBound) + { - for (int i = 0, m = values.size(); i < m; i++) - addComponent(builder, names.get(i), values.get(i)); ++ return eocBound == Bound.END ? composite.end() : composite.start(); + } + + /** - * Adds the specified component value to the specified builder ++ * Adds the specified value to the specified builder + * - * @param builder the ColumnNameBuilder to which the value must be added - * @param name the column associated to the value ++ * @param builder the CBuilder to which the value must be added ++ * @param def the column associated to the value + * @param value the value to add + * @throws InvalidRequestException if the value is null + */ - private static void addComponent(ColumnNameBuilder builder, Name name, ByteBuffer value) throws InvalidRequestException ++ private static void addValue(CBuilder builder, ColumnDefinition def, ByteBuffer value) throws InvalidRequestException + { + if (value == null) - throw new InvalidRequestException(String.format("Invalid null value in condition for column %s", name)); ++ throw new InvalidRequestException(String.format("Invalid null value in condition for column %s", def.name)); + builder.add(value); } + private static Composite.EOC eocForRelation(Operator op) + { + switch (op) + { + case LT: + // < X => using startOf(X) as finish bound + return Composite.EOC.START; + case GT: + case LTE: + // > X => using endOf(X) as start bound + // <= X => using endOf(X) as finish bound + return Composite.EOC.END; + default: + // >= X => using X as start bound (could use START_OF too) + // = X => using X + return Composite.EOC.NONE; + } + } + - private static List<Composite> buildMultiColumnSliceBound(Bound bound, - List<ColumnDefinition> defs, - MultiColumnRestriction.Slice slice, - boolean isReversed, - CBuilder builder, - QueryOptions options) throws InvalidRequestException - { - Bound eocBound = isReversed ? Bound.reverse(bound) : bound; - - Iterator<ColumnDefinition> iter = defs.iterator(); - ColumnDefinition firstName = iter.next(); - // A hack to preserve pre-6875 behavior for tuple-notation slices where the comparator mixes ASCENDING - // and DESCENDING orders. This stores the bound for the first component; we will re-use it for all following - // components, even if they don't match the first component's reversal/non-reversal. Note that this does *not* - // guarantee correct query results, it just preserves the previous behavior. - Bound firstComponentBound = isReversed == isReversedType(firstName) ? bound : Bound.reverse(bound); - - if (!slice.hasBound(firstComponentBound)) - { - Composite prefix = builder.build(); - return Collections.singletonList(builder.remainingCount() > 0 && eocBound == Bound.END - ? prefix.end() - : prefix); - } - - List<ByteBuffer> vals = slice.componentBounds(firstComponentBound, options); - - ByteBuffer v = vals.get(firstName.position()); - if (v == null) - throw new InvalidRequestException("Invalid null value in condition for column " + firstName.name); - builder.add(v); - - while (iter.hasNext()) - { - ColumnDefinition def = iter.next(); - if (def.position() >= vals.size()) - break; - - v = vals.get(def.position()); - if (v == null) - throw new InvalidRequestException("Invalid null value in condition for column " + def.name); - builder.add(v); - } - Operator relType = slice.getRelation(eocBound, firstComponentBound); - return Collections.singletonList(builder.build().withEOC(eocForRelation(relType))); - } - - private static List<Composite> buildMultiColumnInBound(Bound bound, - List<ColumnDefinition> defs, - MultiColumnRestriction.IN restriction, - boolean isReversed, - CBuilder builder, - CType type, - QueryOptions options) throws InvalidRequestException - { - List<List<ByteBuffer>> splitInValues = restriction.splitValues(options); - Bound eocBound = isReversed ? Bound.reverse(bound) : bound; - - // The IN query might not have listed the values in comparator order, so we need to re-sort - // the bounds lists to make sure the slices works correctly (also, to avoid duplicates). - TreeSet<Composite> inValues = new TreeSet<>(isReversed ? type.reverseComparator() : type); - for (List<ByteBuffer> components : splitInValues) - { - for (int i = 0; i < components.size(); i++) - if (components.get(i) == null) - throw new InvalidRequestException("Invalid null value in condition for column " + defs.get(i)); - - Composite prefix = builder.buildWith(components); - inValues.add(eocBound == Bound.END && builder.remainingCount() - components.size() > 0 - ? prefix.end() - : prefix); - } - return new ArrayList<>(inValues); - } - - private static List<Composite> buildMultiColumnEQBound(Bound bound, - List<ColumnDefinition> defs, - MultiColumnRestriction.EQ restriction, - boolean isReversed, - CBuilder builder, - QueryOptions options) throws InvalidRequestException - { - Bound eocBound = isReversed ? Bound.reverse(bound) : bound; - List<ByteBuffer> values = restriction.values(options); - for (int i = 0; i < values.size(); i++) - { - ByteBuffer component = values.get(i); - if (component == null) - throw new InvalidRequestException("Invalid null value in condition for column " + defs.get(i)); - builder.add(component); - } - - Composite prefix = builder.build(); - return Collections.singletonList(builder.remainingCount() > 0 && eocBound == Bound.END - ? prefix.end() - : prefix); - } - private static boolean isNullRestriction(Restriction r, Bound b) { return r == null || (r.isSlice() && !((Restriction.Slice)r).hasBound(b)); @@@ -1470,12 -1497,6 +1451,10 @@@ */ boolean hasQueriableIndex = false; boolean hasQueriableClusteringColumnIndex = false; - boolean hasSingleColumnRelations = false; - boolean hasMultiColumnRelations = false; + + ColumnFamilyStore cfs = Keyspace.open(keyspace()).getColumnFamilyStore(columnFamily()); + SecondaryIndexManager indexManager = cfs.indexManager; + for (Relation relation : whereClause) { if (relation.isMultiColumn()) @@@ -1485,12 -1506,11 +1464,11 @@@ for (ColumnIdentifier.Raw rawEntity : rel.getEntities()) { ColumnIdentifier entity = rawEntity.prepare(cfm); - boolean[] queriable = processRelationEntity(stmt, relation, entity, cfDef); + ColumnDefinition def = cfm.getColumnDefinition(entity); + boolean[] queriable = processRelationEntity(stmt, indexManager, relation, entity, def); hasQueriableIndex |= queriable[0]; hasQueriableClusteringColumnIndex |= queriable[1]; - Name name = cfDef.get(entity); - names.add(name); + names.add(def); - hasMultiColumnRelations |= ColumnDefinition.Kind.CLUSTERING_COLUMN.equals(def.kind); } updateRestrictionsForRelation(stmt, names, rel, boundNames); } @@@ -1498,19 -1518,16 +1476,16 @@@ { SingleColumnRelation rel = (SingleColumnRelation) relation; ColumnIdentifier entity = rel.getEntity().prepare(cfm); - boolean[] queriable = processRelationEntity(stmt, relation, entity, cfDef); + ColumnDefinition def = cfm.getColumnDefinition(entity); + boolean[] queriable = processRelationEntity(stmt, indexManager, relation, entity, def); hasQueriableIndex |= queriable[0]; hasQueriableClusteringColumnIndex |= queriable[1]; - hasSingleColumnRelations |= ColumnDefinition.Kind.CLUSTERING_COLUMN.equals(def.kind); - Name name = cfDef.get(entity); - updateRestrictionsForRelation(stmt, name, rel, boundNames); + updateRestrictionsForRelation(stmt, def, rel, boundNames); } } - if (hasSingleColumnRelations && hasMultiColumnRelations) - throw new InvalidRequestException("Mixing single column relations and multi column relations on clustering columns is not allowed"); // At this point, the select statement if fully constructed, but we still have a few things to validate - processPartitionKeyRestrictions(stmt, cfDef, hasQueriableIndex); + processPartitionKeyRestrictions(stmt, hasQueriableIndex, cfm); // All (or none) of the partition key columns have been specified; // hence there is no need to turn these restrictions into index expressions. @@@ -1597,47 -1614,82 +1572,83 @@@ return prepLimit; } - private void updateRestrictionsForRelation(SelectStatement stmt, List<CFDefinition.Name> names, MultiColumnRelation relation, VariableSpecifications boundNames) throws InvalidRequestException + private void updateRestrictionsForRelation(SelectStatement stmt, List<ColumnDefinition> defs, MultiColumnRelation relation, VariableSpecifications boundNames) throws InvalidRequestException { - List<CFDefinition.Name> restrictedColumns = new ArrayList<>(); - Set<CFDefinition.Name> seen = new HashSet<>(); + List<ColumnDefinition> restrictedColumns = new ArrayList<>(); + Set<ColumnDefinition> seen = new HashSet<>(); + Restriction existing = null; - int previousPosition = -1; - for (ColumnDefinition def : defs) - int previousPosition = names.get(0).position - 1; - for (int i = 0, m = names.size(); i < m; i++) ++ int previousPosition = defs.get(0).position() - 1; ++ for (int i = 0, m = defs.size(); i < m; i++) { - Name name = names.get(i); ++ ColumnDefinition def = defs.get(i); ++ // ensure multi-column restriction only applies to clustering columns - if (name.kind != CFDefinition.Name.Kind.COLUMN_ALIAS) - throw new InvalidRequestException(String.format("Multi-column relations can only be applied to clustering columns: %s", name)); + if (def.kind != ColumnDefinition.Kind.CLUSTERING_COLUMN) - throw new InvalidRequestException(String.format("Multi-column relations can only be applied to clustering columns: %s", def)); ++ throw new InvalidRequestException(String.format("Multi-column relations can only be applied to clustering columns: %s", def.name)); - if (seen.contains(name)) - throw new InvalidRequestException(String.format("Column \"%s\" appeared twice in a relation: %s", name, relation)); - seen.add(name); + if (seen.contains(def)) - throw new InvalidRequestException(String.format("Column \"%s\" appeared twice in a relation: %s", def, relation)); ++ throw new InvalidRequestException(String.format("Column \"%s\" appeared twice in a relation: %s", def.name, relation)); + seen.add(def); // check that no clustering columns were skipped - if (name.position != previousPosition + 1) + if (def.position() != previousPosition + 1) { if (previousPosition == -1) throw new InvalidRequestException(String.format( "Clustering columns may not be skipped in multi-column relations. " + "They should appear in the PRIMARY KEY order. Got %s", relation)); - else - throw new InvalidRequestException(String.format( - "Clustering columns must appear in the PRIMARY KEY order in multi-column relations: %s", relation)); + + throw new InvalidRequestException(String.format( - "Clustering columns must appear in the PRIMARY KEY order in multi-column relations: %s", relation)); ++ "Clustering columns must appear in the PRIMARY KEY order in multi-column relations: %s", ++ relation)); } previousPosition++; - Restriction existing = getExistingRestriction(stmt, def); + Restriction previous = existing; - existing = getExistingRestriction(stmt, name); - Relation.Type operator = relation.operator(); ++ existing = getExistingRestriction(stmt, def); + Operator operator = relation.operator(); if (existing != null) { - if (operator == Relation.Type.EQ || operator == Relation.Type.IN) + if (operator == Operator.EQ || operator == Operator.IN) - throw new InvalidRequestException(String.format("Column \"%s\" cannot be restricted by more than one relation if it is in an %s relation", def, relation.operator())); + { + throw new InvalidRequestException(String.format( + "Column \"%s\" cannot be restricted by more than one relation if it is in an %s relation", - name, relation.operator())); ++ def.name, operator)); + } else if (!existing.isSlice()) - throw new InvalidRequestException(String.format("Column \"%s\" cannot be restricted by an equality relation and an inequality relation", def)); + { + throw new InvalidRequestException(String.format( - "Column \"%s\" cannot be restricted by an equality relation and an inequality relation", name)); ++ "Column \"%s\" cannot be restricted by an equality relation and an inequality relation", ++ def.name)); + } + else + { + if (!existing.isMultiColumn()) + { + throw new InvalidRequestException(String.format( + "Column \"%s\" cannot have both tuple-notation inequalities and single-column inequalities: %s", - name, relation)); ++ def.name, relation)); + } + + boolean existingRestrictionStartBefore = - (i == 0 && name.position != 0 && stmt.columnRestrictions[name.position - 1] == existing); ++ (i == 0 && def.position() != 0 && stmt.columnRestrictions[def.position() - 1] == existing); + + boolean existingRestrictionStartAfter = (i != 0 && previous != existing); + + if (existingRestrictionStartBefore || existingRestrictionStartAfter) + { + throw new InvalidRequestException(String.format( + "Column \"%s\" cannot be restricted by two tuple-notation inequalities not starting with the same column: %s", - name, relation)); ++ def.name, relation)); + } + - checkBound(existing, name, operator); ++ checkBound(existing, def, operator); + } } - restrictedColumns.add(name); + restrictedColumns.add(def); } - boolean onToken = false; - switch (relation.operator()) { case EQ: @@@ -1683,38 -1735,55 +1694,58 @@@ case GT: case GTE: { - Term t = relation.getValue().prepare(names); + Term t = relation.getValue().prepare(keyspace(), defs); t.collectMarkerSpecification(boundNames); - - Restriction.Slice restriction = (Restriction.Slice) getExistingRestriction(stmt, names.get(0)); ++ Restriction.Slice restriction = (Restriction.Slice)getExistingRestriction(stmt, defs.get(0)); + if (restriction == null) + restriction = new MultiColumnRestriction.Slice(false); + restriction.setBound(relation.operator(), t); + - for (CFDefinition.Name name : names) + for (ColumnDefinition def : defs) { - Restriction.Slice restriction = (Restriction.Slice)getExistingRestriction(stmt, def); - if (restriction == null) - restriction = new MultiColumnRestriction.Slice(false); - else if (!restriction.isMultiColumn()) - throw new InvalidRequestException(String.format("Column \"%s\" cannot have both tuple-notation inequalities and single-column inequalities: %s", def.name, relation)); - restriction.setBound(def.name, relation.operator(), t); - stmt.columnRestrictions[name.position] = restriction; + stmt.columnRestrictions[def.position()] = restriction; } + break; } + case NEQ: + throw new InvalidRequestException(String.format("Unsupported \"!=\" relation: %s", relation)); } } - private Restriction getExistingRestriction(SelectStatement stmt, ColumnDefinition def) + /** + * Checks that the operator for the specified column is compatible with the bounds of the existing restriction. + * + * @param existing the existing restriction - * @param name the column name - * @param relType the type of relation ++ * @param def the column definition ++ * @param operator the operator + * @throws InvalidRequestException if the operator is not compatible with the bounds of the existing restriction + */ - private static void checkBound(Restriction existing, Name name, Relation.Type relType) throws InvalidRequestException ++ private static void checkBound(Restriction existing, ColumnDefinition def, Operator operator) throws InvalidRequestException + { + Restriction.Slice existingSlice = (Restriction.Slice) existing; + - if (existingSlice.hasBound(Bound.START) && (relType == Relation.Type.GT || relType == Relation.Type.GTE)) ++ if (existingSlice.hasBound(Bound.START) && (operator == Operator.GT || operator == Operator.GTE)) + throw new InvalidRequestException(String.format( - "More than one restriction was found for the start bound on %s", name.name)); ++ "More than one restriction was found for the start bound on %s", def.name)); + - if (existingSlice.hasBound(Bound.END) && (relType == Relation.Type.LT || relType == Relation.Type.LTE)) ++ if (existingSlice.hasBound(Bound.END) && (operator == Operator.LT || operator == Operator.LTE)) + throw new InvalidRequestException(String.format( - "More than one restriction was found for the end bound on %s", name.name)); ++ "More than one restriction was found for the end bound on %s", def.name)); + } + - private Restriction getExistingRestriction(SelectStatement stmt, CFDefinition.Name name) ++ private static Restriction getExistingRestriction(SelectStatement stmt, ColumnDefinition def) { - switch (name.kind) + switch (def.kind) { - case KEY_ALIAS: - return stmt.keyRestrictions[name.position]; - case COLUMN_ALIAS: - return stmt.columnRestrictions[name.position]; - case VALUE_ALIAS: - return null; + case PARTITION_KEY: + return stmt.keyRestrictions[def.position()]; + case CLUSTERING_COLUMN: + return stmt.columnRestrictions[def.position()]; + case REGULAR: + case STATIC: + return stmt.metadataRestrictions.get(def.name); default: - return stmt.metadataRestrictions.get(name); + throw new AssertionError(); } } @@@ -1804,45 -1871,23 +1835,47 @@@ case GTE: case LT: case LTE: + { + if (existingRestriction == null) + existingRestriction = new SingleColumnRestriction.Slice(newRel.onToken); + else if (!existingRestriction.isSlice()) + throw new InvalidRequestException(String.format("Column \"%s\" cannot be restricted by both an equality and an inequality relation", def.name)); + else if (existingRestriction.isMultiColumn()) + throw new InvalidRequestException(String.format("Column \"%s\" cannot be restricted by both a tuple notation inequality and a single column inequality (%s)", def.name, newRel)); + else if (existingRestriction.isOnToken() != newRel.onToken) + // For partition keys, we shouldn't have slice restrictions without token(). And while this is rejected later by + // processPartitionKeysRestrictions, we shouldn't update the existing restriction by the new one if the old one was using token() + // and the new one isn't since that would bypass that later test. + throw new InvalidRequestException("Only EQ and IN relation are supported on the partition key (unless you use the token() function)"); + ++ checkBound(existingRestriction, def, newRel.operator()); ++ + Term t = newRel.getValue().prepare(keyspace(), receiver); + t.collectMarkerSpecification(boundNames); - ((SingleColumnRestriction.Slice)existingRestriction).setBound(def.name, newRel.operator(), t); ++ ((SingleColumnRestriction.Slice)existingRestriction).setBound(newRel.operator(), t); + } + break; + case CONTAINS_KEY: + if (!(receiver.type instanceof MapType)) + throw new InvalidRequestException(String.format("Cannot use CONTAINS KEY on non-map column %s", def.name)); + // Fallthrough on purpose + case CONTAINS: { + if (!receiver.type.isCollection()) + throw new InvalidRequestException(String.format("Cannot use %s relation on non collection column %s", newRel.operator(), def.name)); + if (existingRestriction == null) - existingRestriction = new SingleColumnRestriction.Slice(newRel.onToken); - else if (!existingRestriction.isSlice()) - throw new InvalidRequestException(String.format("Column \"%s\" cannot be restricted by both an equality and an inequality relation", name)); - else if (existingRestriction.isOnToken() != newRel.onToken) - // For partition keys, we shouldn't have slice restrictions without token(). And while this is rejected later by - // processPartitionKeysRestrictions, we shouldn't update the existing restriction by the new one if the old one was using token() - // and the new one isn't since that would bypass that later test. - throw new InvalidRequestException("Only EQ and IN relation are supported on the partition key (unless you use the token() function)"); - else if (existingRestriction.isMultiColumn()) - throw new InvalidRequestException(String.format("Column \"%s\" cannot be restricted by both a tuple notation inequality and a single column inequality (%s)", name, newRel)); - Term t = newRel.getValue().prepare(receiver); + existingRestriction = new SingleColumnRestriction.Contains(); + else if (!existingRestriction.isContains()) + throw new InvalidRequestException(String.format("Collection column %s can only be restricted by CONTAINS or CONTAINS KEY", def.name)); + + boolean isKey = newRel.operator() == Operator.CONTAINS_KEY; + receiver = makeCollectionReceiver(receiver, isKey); + Term t = newRel.getValue().prepare(keyspace(), receiver); t.collectMarkerSpecification(boundNames); - ((SingleColumnRestriction.Slice)existingRestriction).setBound(newRel.operator(), t); + ((SingleColumnRestriction.Contains)existingRestriction).add(t, isKey); + break; } - break; } return existingRestriction; } @@@ -1958,12 -2003,12 +1991,12 @@@ // columns must have a EQ, and all following must have no restriction. Unless // the column is indexed that is. boolean canRestrictFurtherComponents = true; - CFDefinition.Name previous = null; + ColumnDefinition previous = null; - boolean previousIsSlice = false; + Restriction previousRestriction = null; - Iterator<CFDefinition.Name> iter = cfDef.clusteringColumns().iterator(); + Iterator<ColumnDefinition> iter = cfm.clusteringColumns().iterator(); for (int i = 0; i < stmt.columnRestrictions.length; i++) { - CFDefinition.Name cname = iter.next(); + ColumnDefinition cdef = iter.next(); Restriction restriction = stmt.columnRestrictions[i]; if (restriction == null) @@@ -1972,21 -2017,28 +2005,28 @@@ } else if (!canRestrictFurtherComponents) { -- // We're here if the previous clustering column was either not restricted or was a slice. - // We can't restrict the current column unless: - // 1) we're in the special case of the 'tuple' notation from #4851 which we expand as multiple - // consecutive slices: in which case we're good with this restriction and we continue - // 2) we have a 2ndary index, in which case we have to use it but can skip more validation - if (!(previousIsSlice && restriction.isSlice() && restriction.isMultiColumn())) ++ // We're here if the previous clustering column was either not restricted, was a slice or an IN tulpe-notation. + + // we can continue if we are in the special case of a slice 'tuple' notation from #4851 + if (restriction != previousRestriction) { + // if we have a 2ndary index, we need to use it if (hasQueriableIndex) { - stmt.usesSecondaryIndexing = true; // handle gaps and non-keyrange cases. + stmt.usesSecondaryIndexing = true; break; } + + if (previousRestriction == null) + throw new InvalidRequestException(String.format( - "PRIMARY KEY column \"%s\" cannot be restricted (preceding column \"%s\" is not restricted)", cname, previous)); ++ "PRIMARY KEY column \"%s\" cannot be restricted (preceding column \"%s\" is not restricted)", cdef.name, previous.name)); + + if (previousRestriction.isMultiColumn() && previousRestriction.isIN()) + throw new InvalidRequestException(String.format( - "PRIMARY KEY column \"%s\" cannot be restricted (preceding column \"%s\" is restricted by an IN tuple notation)", cname, previous)); ++ "PRIMARY KEY column \"%s\" cannot be restricted (preceding column \"%s\" is restricted by an IN tuple notation)", cdef.name, previous.name)); + throw new InvalidRequestException(String.format( - "PRIMARY KEY column \"%s\" cannot be restricted (preceding column \"%s\" is either not restricted or by a non-EQ relation)", cdef.name, previous.name)); - "PRIMARY KEY column \"%s\" cannot be restricted (preceding column \"%s\" is restricted by a non-EQ relation)", cname, previous)); ++ "PRIMARY KEY column \"%s\" cannot be restricted (preceding column \"%s\" is restricted by a non-EQ relation)", cdef.name, previous.name)); } } else if (restriction.isSlice()) @@@ -2002,18 -2053,16 +2041,23 @@@ else if (restriction.isIN()) { if (!restriction.isMultiColumn() && i != stmt.columnRestrictions.length - 1) - throw new InvalidRequestException(String.format("Clustering column \"%s\" cannot be restricted by an IN relation", cname)); + throw new InvalidRequestException(String.format("Clustering column \"%s\" cannot be restricted by an IN relation", cdef.name)); - else if (stmt.selectACollection()) ++ + if (stmt.selectACollection()) - throw new InvalidRequestException(String.format("Cannot restrict column \"%s\" by IN relation as a collection is selected by the query", cname)); + throw new InvalidRequestException(String.format("Cannot restrict column \"%s\" by IN relation as a collection is selected by the query", cdef.name)); + + if (restriction.isMultiColumn()) + canRestrictFurtherComponents = false; } + else if (restriction.isContains()) + { + if (!hasQueriableIndex) + throw new InvalidRequestException(String.format("Cannot restrict column \"%s\" by a CONTAINS relation without a secondary index", cdef.name)); + stmt.usesSecondaryIndexing = true; + } - previous = cname; + previous = cdef; + previousRestriction = restriction; } } http://git-wip-us.apache.org/repos/asf/cassandra/blob/9c7a601b/src/java/org/apache/cassandra/cql3/statements/SingleColumnRestriction.java ---------------------------------------------------------------------- diff --cc src/java/org/apache/cassandra/cql3/statements/SingleColumnRestriction.java index b1c6ccc,2e63272..34cd175 --- a/src/java/org/apache/cassandra/cql3/statements/SingleColumnRestriction.java +++ b/src/java/org/apache/cassandra/cql3/statements/SingleColumnRestriction.java @@@ -87,9 -78,9 +87,9 @@@ public abstract class SingleColumnRestr public static class InWithValues extends SingleColumnRestriction implements Restriction.IN { -- protected final List<Term> values; ++ protected final List<? extends Term> values; -- public InWithValues(List<Term> values) ++ public InWithValues(List<? extends Term> values) { this.values = values; } @@@ -292,7 -253,7 +292,8 @@@ throw new AssertionError(); } - public void setBound(ColumnIdentifier name, Operator operator, Term t) throws InvalidRequestException - public void setBound(Relation.Type type, Term t) throws InvalidRequestException ++ @Override ++ public final void setBound(Operator operator, Term t) throws InvalidRequestException { Bound b; boolean inclusive; @@@ -318,9 -279,9 +319,7 @@@ throw new AssertionError(); } -- if (bounds[b.idx] != null) -- throw new InvalidRequestException(String.format( - "More than one restriction was found for the %s bound on %s", b.name().toLowerCase(), name)); - "More than one restriction was found for the %s bound", b.name().toLowerCase())); ++ assert bounds[b.idx] == null; bounds[b.idx] = t; boundInclusive[b.idx] = inclusive;
