diff --git a/test/distributed/org/apache/cassandra/distributed/test/cql3/SingleNodeTableWalkTest.java b/test/distributed/org/apache/cassandra/distributed/test/cql3/SingleNodeTableWalkTest.java index 6c7c58936c46..dc71e8f96f8b 100644 --- a/test/distributed/org/apache/cassandra/distributed/test/cql3/SingleNodeTableWalkTest.java +++ b/test/distributed/org/apache/cassandra/distributed/test/cql3/SingleNodeTableWalkTest.java @@ -392,23 +392,15 @@ protected List supportedIndexers() String annotation; - if (rs.nextBoolean()) - { - builder.between(ckSymbol, state.value(rs, low, ckSymbol.type()), state.value(rs, high, ckSymbol.type())); - annotation = "clustering BETWEEN"; - } - else if (rs.nextBoolean()) - { - builder.where(ckSymbol, Inequality.GREATER_THAN_EQ, low); - builder.where(ckSymbol, Inequality.LESS_THAN_EQ, high); - annotation = "clustering >= AND <="; - } - else - { - builder.where(ckSymbol, Inequality.GREATER_THAN, low); - builder.where(ckSymbol, Inequality.LESS_THAN, high); - annotation = "clustering > AND <"; - } + ASTGenerators.RangeType rangeType = rs.pick(ASTGenerators.RangeType.BOUND, ASTGenerators.RangeType.BETWEEN); + + ASTGenerators.applyRangeCondition(builder, rangeType, ckSymbol, + state.greaterThanGen.next(rs), + state.lessThanGen.next(rs), + state.value(rs, low, ckSymbol.type()), + state.value(rs, high, ckSymbol.type())); + + annotation = "clustering " + rangeType.name(); if (rs.nextBoolean()) builder.orderByColumn(ckSymbol, rs.pick(Select.OrderBy.Ordering.values())); diff --git a/test/harry/main/org/apache/cassandra/harry/model/ASTSingleTableModel.java b/test/harry/main/org/apache/cassandra/harry/model/ASTSingleTableModel.java index eb130e6c7731..58cfa4905a3c 100644 --- a/test/harry/main/org/apache/cassandra/harry/model/ASTSingleTableModel.java +++ b/test/harry/main/org/apache/cassandra/harry/model/ASTSingleTableModel.java @@ -76,6 +76,9 @@ import org.apache.cassandra.cql3.ast.Visitor; import org.apache.cassandra.db.BufferClustering; import org.apache.cassandra.db.Clustering; +import org.apache.cassandra.db.ClusteringBound; +import org.apache.cassandra.db.Slice; +import org.apache.cassandra.db.Slices; import org.apache.cassandra.db.marshal.AbstractType; import org.apache.cassandra.db.marshal.BooleanType; import org.apache.cassandra.db.marshal.CollectionType; @@ -621,14 +624,13 @@ nowTs, filter(set, factory.regularColumns), } private enum DeleteKind - {PARTITION, ROW, COLUMN} + {PARTITION, ROW, COLUMN, RANGE} private void update(Mutation.Delete delete) { long nowTs = delete.timestampOrDefault(numMutations); - //TODO (coverage): range deletes var split = splitOnPartition(delete.where.simplify()); List> pks = split.left; - List> clusterings = split.right.isEmpty() ? Collections.emptyList() : clustering(split.right); + List> clusterings = Collections.emptyList(); HashSet columns = delete.columns.isEmpty() ? null : new HashSet<>(delete.columns); for (Clustering pd : pks) { @@ -638,8 +640,15 @@ private void update(Mutation.Delete delete) DeleteKind kind = DeleteKind.PARTITION; if (!delete.columns.isEmpty()) kind = DeleteKind.COLUMN; - else if (!clusterings.isEmpty()) - kind = DeleteKind.ROW; + else if (containsRangeConditionOnClustering(split.right)) + kind = DeleteKind.RANGE; + + if (kind != DeleteKind.RANGE) + { + clusterings = split.right.isEmpty() ? Collections.emptyList() : clustering(split.right); + if (kind == DeleteKind.PARTITION && !clusterings.isEmpty()) + kind = DeleteKind.ROW; + } switch (kind) { @@ -671,6 +680,19 @@ else if (!clusterings.isEmpty()) } } break; + case RANGE: + assert clusterings.isEmpty(); + LookupContext ctx = new LookupContext(delete); + for (BytesPartitionState.Row value : partition.rows()) + { + if (ctx.include(value)) + { + partition.deleteRow(value.clustering, nowTs); + if (partition.shouldDelete()) + partitions.remove(partition.ref()); + } + } + break; default: throw new UnsupportedOperationException(); } @@ -1016,6 +1038,10 @@ private List> clustering(List conditionals) if (factory.clusteringColumns.isEmpty()) return Collections.singletonList(Clustering.EMPTY); throw new IllegalArgumentException("No clustering columns defined in the WHERE clause, but clustering columns exist; expected " + factory.clusteringColumns); } + + if (containsRangeConditionOnClustering(conditionals)) + return extractClusteringsFromSlices(conditionals); + var split = splitOnClustering(conditionals); var clusterings = split.left; var remaining = split.right; @@ -1088,6 +1114,131 @@ else if (c instanceof Conditional.In) return Pair.create(partitionKeys, other); } + private boolean containsRangeConditionOnClustering(List conditionals) + { + for (Conditional cond : conditionals) + { + if (cond instanceof Conditional.Where) + { + Conditional.Where where = (Conditional.Where) cond; + if (factory.clusteringColumns.contains(where.lhs)) + { + switch (where.kind) + { + case GREATER_THAN: + case GREATER_THAN_EQ: + case LESS_THAN: + case LESS_THAN_EQ: + return true; + } + } + } + else if (cond instanceof Conditional.Between) + { + Conditional.Between between = (Conditional.Between) cond; + if (between.ref instanceof Symbol && factory.clusteringColumns.contains((Symbol) between.ref)) + return true; + } + } + return false; + } + + private List> extractClusteringsFromSlices(List conditionals) + { + Map> rangeConditions = new HashMap<>(); + // Build slices + Slices.Builder builder = new Slices.Builder(factory.clusteringComparator); + + for (Conditional cond : conditionals) + { + if (cond instanceof Conditional.Where) + { + Conditional.Where where = (Conditional.Where) cond; + if (factory.clusteringColumns.contains(where.lhs) + && (where.kind != Inequality.EQUAL)) // skip equality + { + Symbol col = (Symbol) where.lhs; + rangeConditions.computeIfAbsent(col, __ -> new ArrayList<>()).add(where); + } + } + else if (cond instanceof Conditional.Between) + { + Conditional.Between between = (Conditional.Between) cond; + ByteBuffer start = eval(between.start); + ByteBuffer end = eval(between.end); + + ClusteringBound first = ClusteringBound.inclusiveStartOf(Clustering.make(start)); + ClusteringBound last = ClusteringBound.inclusiveEndOf(Clustering.make(end)); + Slice slice = Slice.make(first, last); + // To avoid adding empty slices + if (!slice.isEmpty(factory.clusteringComparator)) + builder.add(slice); + } + } + + for (Map.Entry> entry : rangeConditions.entrySet()) + { + ByteBuffer lower = null; + ByteBuffer upper = null; + boolean includeLower = false; + boolean includeUpper = false; + + for (Conditional.Where cond : entry.getValue()) + { + ByteBuffer val = eval(cond.rhs); + switch (cond.kind) + { + case GREATER_THAN: lower = val; includeLower = false; break; + case GREATER_THAN_EQ: lower = val; includeLower = true; break; + case LESS_THAN: upper = val; includeUpper = false; break; + case LESS_THAN_EQ: upper = val; includeUpper = true; break; + } + } + + ClusteringBound start = (lower == null) + ? ClusteringBound.BOTTOM + : (includeLower + ? ClusteringBound.inclusiveStartOf(Clustering.make(lower)) + : ClusteringBound.exclusiveStartOf(Clustering.make(lower))); + + ClusteringBound end = (upper == null) + ? ClusteringBound.TOP + : (includeUpper + ? ClusteringBound.inclusiveEndOf(Clustering.make(upper)) + : ClusteringBound.exclusiveEndOf(Clustering.make(upper))); + + Slice slice = Slice.make(start, end); + // To avoid adding empty slices + if (!slice.isEmpty(factory.clusteringComparator)) + builder.add(slice); + } + List> clusterings = new ArrayList<>(); + for (Slice slice : builder.build()) + { + if (!slice.start().isBottom()) + clusterings.add(createClusteringBound(slice.start())); + if (!slice.end().isTop()) + clusterings.add(createClusteringBound(slice.end())); + } + + return clusterings; + } + + + private static Clustering createClusteringBound(ClusteringBound bound) + { + if (bound == null) + throw new IllegalArgumentException("ClusteringBound should not be null."); + + int size = bound.size(); + ByteBuffer[] values = new ByteBuffer[size]; + + for (int i = 0; i < size; i++) + values[i] = (ByteBuffer) bound.get(i); + + return Clustering.make(values); + } + private static ImmutableUniqueList> keys(Collection columns, Map> columnValues) { return keys(columns, columnValues, Function.identity()); diff --git a/test/harry/main/org/apache/cassandra/harry/model/ASTSingleTableModelTest.java b/test/harry/main/org/apache/cassandra/harry/model/ASTSingleTableModelTest.java index 7115d4eefbb2..bb18430e8edc 100644 --- a/test/harry/main/org/apache/cassandra/harry/model/ASTSingleTableModelTest.java +++ b/test/harry/main/org/apache/cassandra/harry/model/ASTSingleTableModelTest.java @@ -65,6 +65,7 @@ import org.apache.cassandra.schema.TableMetadata; import org.apache.cassandra.utils.ByteBufferUtil; + public class ASTSingleTableModelTest { public static final ByteBuffer ZERO = ByteBufferUtil.bytes(0); @@ -557,6 +558,116 @@ public void deleteRowImpactsSearch() .build()); } + @Test + public void testClusteringRangeDelete() + { + TableMetadata metadata = new Builder().pk(1).ck(1).statics(0).regular(1).build(); + ASTSingleTableModel model = new ASTSingleTableModel(metadata); + + for (int i = 0; i < 10; i++) + { + model.update(Mutation.insert(metadata) + .value("pk", 0) + .value("ck", i) + .value("v", i) + .build()); + } + + model.update(Mutation.delete(metadata) + .value("pk", 0) + .where("ck", Inequality.GREATER_THAN_EQ, 0) + .where("ck", Inequality.LESS_THAN, 7) + .build()); + + model.validate(rows(row(metadata, 0, 7, 7), row(metadata, 0, 8, 8), row(metadata,0,9,9)), + Select.builder(metadata).value("pk", 0).build()); + } + + @Test + public void testRangeDeleteWithMultipleClusteringKeys() + { + TableMetadata metadata = new Builder().pk(1).ck(2).statics(0).regular(1).build(); + ASTSingleTableModel model = new ASTSingleTableModel(metadata); + + for (int i = 0; i < 10; i++) + { + model.update(Mutation.insert(metadata) + .value("pk", 0) + .value("ck0", i) + .value("ck1", i+1) + .value("v", i) + .build()); + } + + model.update(Mutation.delete(metadata) + .value("pk", 0) + .between("ck0", Literal.of(3), Literal.of(7)) + .build()); + + model.validate(rows(row(metadata, 0,0,1,0), + row(metadata, 0,1,2,1), + row(metadata,0,2,3,2), + row(metadata,0,8,9,8), + row(metadata,0,9,10,9)), + Select.builder(metadata).build()); + + + } + + @Test + public void testRangeDeleteWithStaticColumn() + { + TableMetadata metadata = new Builder().pk(1).ck(1).statics(1).regular(1).build(); + ASTSingleTableModel model = new ASTSingleTableModel(metadata); + + for (int i = 0; i < 10; i++) + { + model.update(Mutation.insert(metadata) + .value("pk", 0) + .value("ck", i) + .value("s", 0) + .value("v", i) + .build()); + } + + model.update(Mutation.delete(metadata) + .value("pk", 0) + .between("ck", Literal.of(3), Literal.of(7)) + .build()); + + model.validate(rows(row(metadata, 0,0,0,0), + row(metadata, 0,1,0,1), + row(metadata,0,2,0,2), + row(metadata,0,8,0,8), + row(metadata,0,9,0,9)), + Select.builder(metadata).build()); + + } + + @Test + public void testClusteringRangeDeleteBetween() + { + TableMetadata metadata = new Builder().pk(1).ck(1).statics(0).regular(1).build(); + ASTSingleTableModel model = new ASTSingleTableModel(metadata); + + for (int i = 0; i < 10; i++) + { + model.update(Mutation.insert(metadata) + .value("pk", 0) + .value("ck", i) + .value("v", i) + .build()); + } + + model.update(Mutation.delete(metadata) + .value("pk", 0) + .between("ck", Bind.of(0), Literal.of(6)) + .build()); + + model.validate(rows(row(metadata, 0, 7, 7), row(metadata, 0, 8, 8), row(metadata,0,9,9)), + Select.builder(metadata).value("pk", 0).build()); + } + @Test public void tokenEqIncludesEmptyPartition() { @@ -753,6 +864,35 @@ public void deleteColumnUpdateDoesntHavePartitionState() model.validate(rows(row(metadata, 0, 1, null, List.of(1))), Select.builder(metadata).build()); } + @Test + public void testRangeDeleteGeneratorWithBetweenClause() + { + TableMetadata metadata = new Builder().pk(1).ck(1).statics(0).regular(1).build(); + ASTSingleTableModel model = new ASTSingleTableModel(metadata); + + for (int i = 0; i < 10; i++) + { + model.update(Mutation.insert(metadata) + .value("pk", 0) + .value("ck", i) + .value("v", i) + .build()); + } + + model.update(Mutation.delete(metadata) + .value("pk", 0) + .between("ck", Literal.of(3), Literal.of(7)) + .build()); + + model.validate(rows(row(metadata, 0, 0,0), + row(metadata, 0, 1,1), + row(metadata,0,2,2), + row(metadata,0,8,8), + row(metadata,0,9,9)), + Select.builder(metadata).build()); + + } + private interface SimpleWrite { void write(String name, T value, long ts); diff --git a/test/unit/org/apache/cassandra/utils/ASTGenerators.java b/test/unit/org/apache/cassandra/utils/ASTGenerators.java index aba227b16959..31df22f11ad2 100644 --- a/test/unit/org/apache/cassandra/utils/ASTGenerators.java +++ b/test/unit/org/apache/cassandra/utils/ASTGenerators.java @@ -91,6 +91,36 @@ public class ASTGenerators { public static final EnumSet IGNORED_ISSUES = KnownIssue.ignoreAll(); + public enum RangeType {UNBOUND, BOUND , BETWEEN} + + /** + * Applies a range condition to a builder for a single column. + */ + public static void applyRangeCondition(Conditional.ConditionalBuilder builder, + RangeType rangeType, + Symbol column, + Conditional.Where.Inequality lowerKind, + Conditional.Where.Inequality upperKind, + Expression lowerValue, + Expression upperValue) + { + switch (rangeType) + { + case UNBOUND: + builder.where(column, lowerKind, lowerValue); + break; + case BOUND: + builder.where(column, lowerKind, lowerValue); + builder.where(column, upperKind, upperValue); + break; + case BETWEEN: + builder.between(column, lowerValue, upperValue); + break; + default: + throw new UnsupportedOperationException("Unsupported RangeType: " + rangeType); + } + } + public static Gen> columnValues(List columns) { List> gens = new ArrayList<>(columns.size()); @@ -403,7 +433,7 @@ public interface ValueGen public static class MutationGenBuilder { - public enum DeleteKind { Partition, Row, Column } + public enum DeleteKind { Partition, Row, Column, Range } private final TableMetadata metadata; private final LinkedHashSet allColumns; private final LinkedHashSet partitionColumns, clusteringColumns; @@ -845,6 +875,8 @@ else if (regularAndStaticColumns.size() == 1 || bool.generate(rnd)) // if there are no columns to delete, fallback to row if (deleteKind == DeleteKind.Column && regularAndStaticColumns.isEmpty()) deleteKind = DeleteKind.Row; + if (deleteKind == DeleteKind.Range && clusteringColumns.isEmpty()) + deleteKind = DeleteKind.Partition; if (deleteKind == DeleteKind.Row && clusteringColumns.isEmpty()) deleteKind = DeleteKind.Partition; @@ -907,6 +939,9 @@ else if (regularColumns.isEmpty()) } } break; + case Range: + valueRange(rnd, columnExpressions, builder, clusteringColumns); + break; default: throw new UnsupportedOperationException(); } @@ -958,6 +993,12 @@ else if (regularColumns.isEmpty()) columns = new ArrayList<>(uniq); } break; + case Range: + { + columns = Collections.emptyList(); + existAllowed = false; + } + break; default: throw new UnsupportedOperationException(deleteKind.name()); } @@ -982,6 +1023,40 @@ else if (existAllowed) }; } + private static final Gen RANGE_INEQUALITY_GEN = SourceDSL.arbitrary().pick(Conditional.Where.Inequality.GREATER_THAN_EQ, + Conditional.Where.Inequality.GREATER_THAN, + Conditional.Where.Inequality.LESS_THAN_EQ, + Conditional.Where.Inequality.LESS_THAN); + + private static final Gen LOWER_BOUND_GEN = SourceDSL.arbitrary().pick(Conditional.Where.Inequality.GREATER_THAN, + Conditional.Where.Inequality.GREATER_THAN_EQ); + + private static final Gen UPPER_BOUND_GEN = SourceDSL.arbitrary().pick(Conditional.Where.Inequality.LESS_THAN, + Conditional.Where.Inequality.LESS_THAN_EQ); + + + private void valueRange(RandomnessSource rnd, + Map columnExpressions, + Conditional.ConditionalBuilder builder, + LinkedHashSet columns) + { + List columnList = new ArrayList<>(columns); + + assert !columnList.isEmpty(); + + Symbol s = columnList.get(0); + Gen expressionGen = columnExpressions.get(s).build(); + + RangeType type = SourceDSL.arbitrary().enumValues(RangeType.class).generate(rnd); + applyRangeCondition(builder, type, s, + type == RangeType.UNBOUND ? RANGE_INEQUALITY_GEN.generate(rnd) : LOWER_BOUND_GEN.generate(rnd), + UPPER_BOUND_GEN.generate(rnd), + expressionGen.generate(rnd), + expressionGen.generate(rnd)); + + } + + private void generateRemaining(RandomnessSource rnd, Gen bool, Mutation.Kind kind,