This is an automated email from the ASF dual-hosted git repository.
dcapwell pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/cassandra.git
The following commit(s) were added to refs/heads/trunk by this push:
new c08666567d When updating a multi cell collection element, if the
update is rejected then the shared Row.Builder is not freed causing all future
mutations to be rejected
c08666567d is described below
commit c08666567d95562eeb28f3c49469e564af9a70b8
Author: David Capwell <[email protected]>
AuthorDate: Fri Dec 5 09:41:28 2025 -0800
When updating a multi cell collection element, if the update is rejected
then the shared Row.Builder is not freed causing all future mutations to be
rejected
patch by David Capwell; reviewed by Bernardo Botella Corbi, Caleb
Rackliffe, Dmitry Konstantinov for CASSANDRA-21055
---
CHANGES.txt | 1 +
.../apache/cassandra/cql3/UpdateParameters.java | 19 +-
.../utils/caching/TinyThreadLocalPool.java | 15 ++
.../cassandra/distributed/test/TestBaseImpl.java | 5 +
.../cql3/AccordInteropMultiNodeTableWalkBase.java | 7 +
.../test/cql3/CasMultiNodeTableWalkBase.java | 7 +
.../test/cql3/SingleNodeTableWalkTest.java | 1 +
.../test/cql3/SingleNodeTokenConflictTest.java | 1 +
.../distributed/test/cql3/StatefulASTBase.java | 54 +++-
.../fuzz/topology/AccordTopologyMixupTest.java | 6 +-
.../cassandra/harry/model/ASTSingleTableModel.java | 277 ++++++++++++++++++---
.../simulator/test/SingleTableASTSimulation.java | 4 +-
test/unit/org/apache/cassandra/cql3/CQLTester.java | 6 +
.../cassandra/cql3/ast/CollectionAccess.java | 17 +-
.../org/apache/cassandra/cql3/ast/Conditional.java | 6 +-
.../apache/cassandra/cql3/ast/CreateIndexDDL.java | 9 +-
.../org/apache/cassandra/cql3/ast/Mutation.java | 32 +--
.../org/apache/cassandra/cql3/ast/Reference.java | 9 +
.../cassandra/cql3/ast/ReferenceExpression.java | 9 +
.../unit/org/apache/cassandra/cql3/ast/Symbol.java | 11 +
.../cql3/validation/entities/CollectionsTest.java | 16 ++
.../org/apache/cassandra/utils/ASTGenerators.java | 55 +++-
.../org/apache/cassandra/utils/AssertionUtils.java | 2 +
23 files changed, 476 insertions(+), 93 deletions(-)
diff --git a/CHANGES.txt b/CHANGES.txt
index 787b6f83aa..6cd56b9ed2 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
5.1
+ * When updating a multi cell collection element, if the update is rejected
then the shared Row.Builder is not freed causing all future mutations to be
rejected (CASSANDRA-21055)
* Schema annotations escape validation on CREATE and ALTER DDL statements
(CASSANDRA-21046)
* Calculate once and cache the result of ModificationStatement#requiresRead
as a perf optimization (CASSANDRA-21040)
* Update system schema tables with new distributed keyspace on upgrade
(CASSANDRA-20872)
diff --git a/src/java/org/apache/cassandra/cql3/UpdateParameters.java
b/src/java/org/apache/cassandra/cql3/UpdateParameters.java
index e6ad075974..85a97ccc3d 100644
--- a/src/java/org/apache/cassandra/cql3/UpdateParameters.java
+++ b/src/java/org/apache/cassandra/cql3/UpdateParameters.java
@@ -62,9 +62,6 @@ public class UpdateParameters
// Holds data for operations that require a read-before-write. Will be
null otherwise.
private final Map<DecoratedKey, Partition> prefetchedRows;
- private Row.Builder staticBuilder;
- private Row.Builder regularBuilder;
-
// The builder currently in use. Will alias either staticBuilder or
regularBuilder, which are themselves built lazily.
private Row.Builder builder;
@@ -108,20 +105,8 @@ public class UpdateParameters
throw new InvalidRequestException("Invalid empty or null
value for column " + metadata.clusteringColumns().get(0).name);
}
}
-
- if (clustering == Clustering.STATIC_CLUSTERING)
- {
- if (staticBuilder == null)
- staticBuilder = BTreeRow.pooledUnsortedBuilder();
- builder = staticBuilder;
- }
- else
- {
- if (regularBuilder == null)
- regularBuilder = BTreeRow.pooledUnsortedBuilder();
- builder = regularBuilder;
- }
-
+ assert builder == null : "newRow called without building the previous
row";
+ builder = BTreeRow.pooledUnsortedBuilder();
builder.newRow(clustering);
}
diff --git
a/src/java/org/apache/cassandra/utils/caching/TinyThreadLocalPool.java
b/src/java/org/apache/cassandra/utils/caching/TinyThreadLocalPool.java
index 7712c09258..78ce8d0f26 100644
--- a/src/java/org/apache/cassandra/utils/caching/TinyThreadLocalPool.java
+++ b/src/java/org/apache/cassandra/utils/caching/TinyThreadLocalPool.java
@@ -18,10 +18,12 @@
package org.apache.cassandra.utils.caching;
+import accord.utils.Invariants;
import io.netty.util.concurrent.FastThreadLocal;
public class TinyThreadLocalPool<V> extends
FastThreadLocal<TinyThreadLocalPool.TinyPool<V>>
{
+ private static final boolean DEBUG = Invariants.debug();
protected TinyPool<V> initialValue()
{
return new TinyPool<>();
@@ -46,10 +48,23 @@ public class TinyThreadLocalPool<V> extends
FastThreadLocal<TinyThreadLocalPool.
}
private void offerSafe(V value)
{
+ if (DEBUG)
+ checkOfferSafe(value);
if (val1 == null) val1 = value;
else if (val2 == null) val2 = value;
else if (val3 == null) val3 = value;
}
+
+ private void checkOfferSafe(V value)
+ {
+ if (val1 == value)
+ throw new IllegalStateException("Double offer");
+ if (val2 == value)
+ throw new IllegalStateException("Double offer");
+ if (val3 == value)
+ throw new IllegalStateException("Double offer");
+ }
+
public V poll()
{
Object result;
diff --git
a/test/distributed/org/apache/cassandra/distributed/test/TestBaseImpl.java
b/test/distributed/org/apache/cassandra/distributed/test/TestBaseImpl.java
index 6d91a59237..1f76eda33e 100644
--- a/test/distributed/org/apache/cassandra/distributed/test/TestBaseImpl.java
+++ b/test/distributed/org/apache/cassandra/distributed/test/TestBaseImpl.java
@@ -94,6 +94,11 @@ import static org.assertj.core.api.Assertions.fail;
// checkstyle: suppress below 'blockSystemPropertyUsage'
public class TestBaseImpl extends DistributedTestBase
{
+ static
+ {
+ System.setProperty("accord.debug", "true"); // checkstyle: suppress
nearby 'blockSystemPropertyUsage'
+ }
+
private static final Logger logger =
LoggerFactory.getLogger(TestBaseImpl.class);
public static final Object[][] EMPTY_ROWS = new Object[0][];
diff --git
a/test/distributed/org/apache/cassandra/distributed/test/cql3/AccordInteropMultiNodeTableWalkBase.java
b/test/distributed/org/apache/cassandra/distributed/test/cql3/AccordInteropMultiNodeTableWalkBase.java
index d1a5e0f32e..170d9ea8f6 100644
---
a/test/distributed/org/apache/cassandra/distributed/test/cql3/AccordInteropMultiNodeTableWalkBase.java
+++
b/test/distributed/org/apache/cassandra/distributed/test/cql3/AccordInteropMultiNodeTableWalkBase.java
@@ -117,6 +117,13 @@ Suppressed: java.lang.AssertionError: Unknown keyspace ks12
return super.command(rs, mutation, annotate);
}
+ @Override
+ protected boolean allowListElementAccessForUpdateSet()
+ {
+ // See CASSANDRA-20828
+ return false;
+ }
+
@Override
protected boolean allowUsingTimestamp()
{
diff --git
a/test/distributed/org/apache/cassandra/distributed/test/cql3/CasMultiNodeTableWalkBase.java
b/test/distributed/org/apache/cassandra/distributed/test/cql3/CasMultiNodeTableWalkBase.java
index 9a466b9493..d812eadec1 100644
---
a/test/distributed/org/apache/cassandra/distributed/test/cql3/CasMultiNodeTableWalkBase.java
+++
b/test/distributed/org/apache/cassandra/distributed/test/cql3/CasMultiNodeTableWalkBase.java
@@ -75,6 +75,13 @@ public abstract class CasMultiNodeTableWalkBase extends
MultiNodeTableWalkBase
super(rs, cluster);
}
+ @Override
+ protected boolean allowUsingTimestamp()
+ {
+ // Paxos doesn't allow USING TIMESTAMP
+ return false;
+ }
+
@Override
protected Gen<Mutation> toMutationGen(ASTGenerators.MutationGenBuilder
mutationGenBuilder)
{
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 a2a526121b..a463e06bdb 100644
---
a/test/distributed/org/apache/cassandra/distributed/test/cql3/SingleNodeTableWalkTest.java
+++
b/test/distributed/org/apache/cassandra/distributed/test/cql3/SingleNodeTableWalkTest.java
@@ -447,6 +447,7 @@ public class SingleNodeTableWalkTest extends StatefulASTBase
ASTGenerators.MutationGenBuilder mutationGenBuilder = new
ASTGenerators.MutationGenBuilder(metadata)
.withTxnSafe()
.withColumnExpressions(e ->
e.withOperators(Generators.fromGen(BOOLEAN_DISTRIBUTION.next(rs))))
+
.withListElementAccessForUpdateSet(allowListElementAccessForUpdateSet())
.withIgnoreIssues(IGNORED_ISSUES);
// Run the test with and without bound partitions
diff --git
a/test/distributed/org/apache/cassandra/distributed/test/cql3/SingleNodeTokenConflictTest.java
b/test/distributed/org/apache/cassandra/distributed/test/cql3/SingleNodeTokenConflictTest.java
index 703c4e3c93..72da2a8d6c 100644
---
a/test/distributed/org/apache/cassandra/distributed/test/cql3/SingleNodeTokenConflictTest.java
+++
b/test/distributed/org/apache/cassandra/distributed/test/cql3/SingleNodeTokenConflictTest.java
@@ -386,6 +386,7 @@ public class SingleNodeTokenConflictTest extends
StatefulASTBase
this.mutationGen = toGen(new
ASTGenerators.MutationGenBuilder(metadata)
.withTxnSafe()
.withPartitions(SourceDSL.arbitrary().pick(uniquePartitions))
+
.withListElementAccessForUpdateSet(allowListElementAccessForUpdateSet())
.withIgnoreIssues(IGNORED_ISSUES)
.build());
}
diff --git
a/test/distributed/org/apache/cassandra/distributed/test/cql3/StatefulASTBase.java
b/test/distributed/org/apache/cassandra/distributed/test/cql3/StatefulASTBase.java
index 756095673e..ce43be6373 100644
---
a/test/distributed/org/apache/cassandra/distributed/test/cql3/StatefulASTBase.java
+++
b/test/distributed/org/apache/cassandra/distributed/test/cql3/StatefulASTBase.java
@@ -27,11 +27,13 @@ import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.BiConsumer;
import java.util.function.Consumer;
+import java.util.function.Supplier;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import javax.annotation.Nullable;
import com.google.common.collect.ImmutableList;
+import org.assertj.core.api.ThrowableAssert;
import org.slf4j.Logger;
import accord.utils.Gen;
@@ -105,6 +107,11 @@ public class StatefulASTBase extends TestBaseImpl
*/
protected static boolean CQL_DEBUG_APPLY_OPERATOR = false;
+ /**
+ * Allows for overriding the CQL format logic, default is none (single
line), but can create custom ones to help with the history
+ */
+ protected static Supplier<CQLFormatter> CQL_FORMATTER = () ->
CQLFormatter.None.instance;
+
protected static final Gen<Gen<Boolean>> BOOL_DISTRIBUTION =
Gens.bools().mixedDistribution();
protected static final Gen<Gen<Conditional.Where.Inequality>>
LESS_THAN_DISTRO =
Gens.mixedDistribution(Stream.of(Conditional.Where.Inequality.values())
.filter(i -> i ==
Conditional.Where.Inequality.LESS_THAN || i ==
Conditional.Where.Inequality.LESS_THAN_EQ)
@@ -196,7 +203,7 @@ public class StatefulASTBase extends TestBaseImpl
protected static <S extends BaseState> Property.Command<S, Void, ?>
compactTable(RandomSource rs, S state)
{
return new Property.SimpleCommand<>("nodetool compact " +
state.metadata.keyspace + ' ' + state.metadata.name, s2 -> {
- state.cluster.forEach(i -> i.nodetoolResult("compact",
s2.metadata.keyspace, s2.metadata.name).asserts().success());
+ s2.cluster.forEach(i -> i.nodetoolResult("compact",
s2.metadata.keyspace, s2.metadata.name).asserts().success());
s2.compact();
});
}
@@ -475,6 +482,12 @@ public class StatefulASTBase extends TestBaseImpl
return command(rs, select, null);
}
+ protected boolean allowListElementAccessForUpdateSet()
+ {
+ // this requires a read at the same CL, but the model is global
level and not per-node level, so can't handle
+ return mutationCl() != ConsistencyLevel.NODE_LOCAL;
+ }
+
protected boolean allowRepair()
{
return false;
@@ -586,18 +599,24 @@ public class StatefulASTBase extends TestBaseImpl
{
var inst = selectInstance(rs);
String postfix = "on " + inst;
- if (mutation.isCas())
- {
+ @Nullable
+ Consumer<ThrowableAssert.ThrowingCallable> shouldRaiseThrowable =
model.shouldReject(mutation);
+ if (shouldRaiseThrowable != null)
+ postfix += ", should reject";
+ else if (mutation.isCas())
postfix += ", would apply " + model.shouldApply(mutation);
- // CAS doesn't allow timestamps
- mutation = mutation.withoutTimestamp();
- }
+
if (annotate == null) annotate = postfix;
else annotate += ", " + postfix;
- Mutation finalMutation = mutation;
return new Property.SimpleCommand<>(humanReadable(mutation,
annotate), s -> {
- var result = s.executeQuery(inst, Integer.MAX_VALUE,
s.mutationCl(), finalMutation);
- s.model.updateAndValidate(result, finalMutation);
+ if (shouldRaiseThrowable != null)
+ {
+ shouldRaiseThrowable.accept(() -> s.executeQuery(inst,
Integer.MAX_VALUE, s.mutationCl(), mutation));
+ s.mutation();
+ return;
+ }
+ var result = s.executeQuery(inst, Integer.MAX_VALUE,
s.mutationCl(), mutation);
+ s.model.updateAndValidate(result, mutation);
s.mutation();
});
}
@@ -611,7 +630,11 @@ public class StatefulASTBase extends TestBaseImpl
{
var inst = selectInstance(rs);
String postfix = "on " + inst;
- if (model.isConditional(txn))
+ @Nullable
+ Consumer<ThrowableAssert.ThrowingCallable> shouldRaiseThrowable =
model.shouldReject(txn);
+ if (shouldRaiseThrowable != null)
+ postfix += ", should reject";
+ else if (model.isConditional(txn))
postfix += ", would apply " + model.shouldApply(txn);
if (annotate == null) annotate = postfix;
else annotate += ", " + postfix;
@@ -619,7 +642,14 @@ public class StatefulASTBase extends TestBaseImpl
return new Property.SimpleCommand<>(humanReadable(txn, annotate),
s -> {
boolean hasMutation = txn.ifBlock.isPresent() ||
!txn.mutations.isEmpty();
ConsistencyLevel cl = hasMutation ? s.mutationCl() :
s.selectCl();
- s.model.updateAndValidate(s.executeQuery(inst,
Integer.MAX_VALUE, cl, txn), txn);
+ if (shouldRaiseThrowable != null)
+ {
+ shouldRaiseThrowable.accept(() -> s.executeQuery(inst,
Integer.MAX_VALUE, cl, txn));
+ }
+ else
+ {
+ s.model.updateAndValidate(s.executeQuery(inst,
Integer.MAX_VALUE, cl, txn), txn);
+ }
if (hasMutation)
s.mutation();
});
@@ -706,7 +736,7 @@ public class StatefulASTBase extends TestBaseImpl
{
// With UTF-8 some chars can cause printing issues leading to
error messages that don't reproduce the original issue.
// To avoid this problem, always escape the CQL so nothing gets
lost
- String cql =
StringUtils.escapeControlChars(stmt.visit(debug).toCQL(CQLFormatter.None.instance));
+ String cql =
StringUtils.escapeControlChars(stmt.visit(debug).toCQL(CQL_FORMATTER.get()));
if (annotate != null)
cql += " -- " + annotate;
return cql;
diff --git
a/test/distributed/org/apache/cassandra/fuzz/topology/AccordTopologyMixupTest.java
b/test/distributed/org/apache/cassandra/fuzz/topology/AccordTopologyMixupTest.java
index 82a444c37d..e12b4860dc 100644
---
a/test/distributed/org/apache/cassandra/fuzz/topology/AccordTopologyMixupTest.java
+++
b/test/distributed/org/apache/cassandra/fuzz/topology/AccordTopologyMixupTest.java
@@ -148,7 +148,11 @@ public class AccordTopologyMixupTest extends
TopologyMixupTestBase<AccordTopolog
private static CommandGen<Spec> cqlOperations(Spec spec)
{
Gen<Statement> select = (Gen<Statement>) (Gen<?>) fromQT(new
ASTGenerators.SelectGenBuilder(spec.metadata).withLimit1().build());
- Gen<Statement> mutation = (Gen<Statement>) (Gen<?>) fromQT(new
ASTGenerators.MutationGenBuilder(spec.metadata).withTxnSafe().disallowUpdateMultiplePartitionKeys().build());
+ Gen<Statement> mutation = (Gen<Statement>) (Gen<?>) fromQT(new
ASTGenerators.MutationGenBuilder(spec.metadata)
+
.withTxnSafe()
+
.disallowUpdateMultiplePartitionKeys() //TODO (coverage): this is something
Accord should support, so should remove and make sure accord is updated
+
.disallowListElementAccessForUpdateSet() //TODO (coverage): CASSANDRA-20828
found an issue with multi cell list type timestamp handling, so make sure
accord doesn't hit this
+ .build());
Gen<Statement> txn = (Gen<Statement>) (Gen<?>) fromQT(new
ASTGenerators.TxnGenBuilder(spec.metadata).build());
Map<Gen<Statement>, Integer> operations = new LinkedHashMap<>();
operations.put(select, 1);
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 37f4585332..805857ea98 100644
--- a/test/harry/main/org/apache/cassandra/harry/model/ASTSingleTableModel.java
+++ b/test/harry/main/org/apache/cassandra/harry/model/ASTSingleTableModel.java
@@ -35,6 +35,7 @@ import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.TreeMap;
+import java.util.function.Consumer;
import java.util.function.Function;
import java.util.function.IntFunction;
import java.util.stream.Collectors;
@@ -50,6 +51,7 @@ import accord.utils.Invariants;
import org.apache.cassandra.cql3.KnownIssue;
import org.apache.cassandra.cql3.ast.AssignmentOperator;
import org.apache.cassandra.cql3.ast.CasCondition;
+import org.apache.cassandra.cql3.ast.CollectionAccess;
import org.apache.cassandra.cql3.ast.Conditional.Where.Inequality;
import org.apache.cassandra.cql3.ast.Conditional;
import org.apache.cassandra.cql3.ast.Element;
@@ -71,17 +73,24 @@ import org.apache.cassandra.db.BufferClustering;
import org.apache.cassandra.db.Clustering;
import org.apache.cassandra.db.marshal.AbstractType;
import org.apache.cassandra.db.marshal.BooleanType;
+import org.apache.cassandra.db.marshal.CollectionType;
import org.apache.cassandra.db.marshal.Int32Type;
+import org.apache.cassandra.db.marshal.ListType;
import org.apache.cassandra.db.marshal.LongType;
+import org.apache.cassandra.db.marshal.MapType;
import org.apache.cassandra.dht.Murmur3Partitioner;
import org.apache.cassandra.dht.Token;
+import org.apache.cassandra.exceptions.InvalidRequestException;
import org.apache.cassandra.harry.model.BytesPartitionState.PrimaryKey;
import org.apache.cassandra.harry.util.StringUtils;
import org.apache.cassandra.schema.TableMetadata;
import org.apache.cassandra.tools.nodetool.formatter.TableBuilder;
+import org.apache.cassandra.utils.AssertionUtils;
import org.apache.cassandra.utils.ByteBufferUtil;
import org.apache.cassandra.utils.ImmutableUniqueList;
import org.apache.cassandra.utils.Pair;
+import org.assertj.core.api.Assertions;
+import org.assertj.core.api.ThrowableAssert;
import static org.apache.cassandra.cql3.ast.Elements.symbols;
import static org.apache.cassandra.harry.MagicConstants.NO_TIMESTAMP;
@@ -268,6 +277,8 @@ public class ASTSingleTableModel
if (!mutations.isEmpty())
numMutations++; // bump here to make sure the last mutation
doesn't have the same ts as the mutations here
long nowTs = numMutations;
+ for (var m : mutations)
+ Invariants.require(shouldReject(m) == null, "Mutation should have
been rejected");
for (var m : mutations)
{
if (m.timestampOrDefault(NO_TIMESTAMP) == NO_TIMESTAMP)
@@ -315,6 +326,7 @@ public class ASTSingleTableModel
public void update(Mutation mutation)
{
if (!shouldApply(mutation)) return;
+ Invariants.require(shouldReject(mutation) == null, "Mutation should
have been rejected");
updateInternal(mutation);
}
@@ -326,6 +338,7 @@ public class ASTSingleTableModel
validateCasNotApplied(actual, mutation);
return;
}
+ Invariants.require(shouldReject(mutation) == null, "Mutation should
have been rejected");
if (mutation.isCas() && validateCass.validate())
validate(CAS_APPLIED_COLUMNS, actual, CAS_SUCCESS_RESULT);
updateInternal(mutation);
@@ -553,12 +566,11 @@ public class ASTSingleTableModel
partition = factory.create(pd);
partitions.put(partition.ref(), partition);
}
- Map<Symbol, Expression> values = insert.values;
+ Map<ReferenceExpression, Expression> values = insert.values;
if (!factory.staticColumns.isEmpty() &&
!Sets.intersection(factory.staticColumns, values.keySet()).isEmpty())
{
- maybeUpdateColumns(Sets.intersection(factory.staticColumns,
values.keySet()),
- partition.staticRow(),
- nowTs, values,
+ maybeUpdateColumns(partition.staticRow(),
+ nowTs, filter(values, factory.staticColumns),
partition::setStaticColumns);
}
// table has clustering but non are in the write, so only pk/static
can be updated
@@ -566,9 +578,8 @@ public class ASTSingleTableModel
return;
BytesPartitionState finalPartition = partition;
var cd = key(insert.values, factory.clusteringColumns);
- maybeUpdateColumns(Sets.intersection(factory.regularColumns,
values.keySet()),
- partition.get(cd),
- nowTs, values,
+ maybeUpdateColumns(partition.get(cd),
+ nowTs, filter(values, factory.regularColumns),
(ts, write) -> finalPartition.setColumns(cd, ts,
write, true));
}
@@ -586,12 +597,11 @@ public class ASTSingleTableModel
partition = factory.create(pd);
partitions.put(partition.ref(), partition);
}
- Map<Symbol, Expression> set = update.set;
- if (!factory.staticColumns.isEmpty() &&
!Sets.intersection(factory.staticColumns, set.keySet()).isEmpty())
+ Map<ReferenceExpression, Expression> set = update.set;
+ if (!factory.staticColumns.isEmpty() && !filter(set,
factory.staticColumns).isEmpty())
{
- maybeUpdateColumns(Sets.intersection(factory.staticColumns,
set.keySet()),
- partition.staticRow(),
- nowTs, set,
+ maybeUpdateColumns(partition.staticRow(),
+ nowTs, filter(set, factory.staticColumns),
partition::setStaticColumns);
}
// table has clustering but non are in the write, so only
pk/static can be updated
@@ -600,9 +610,8 @@ public class ASTSingleTableModel
BytesPartitionState finalPartition = partition;
for (Clustering<ByteBuffer> cd : clustering(remaining))
{
- maybeUpdateColumns(Sets.intersection(factory.regularColumns,
set.keySet()),
- partition.get(cd),
- nowTs, set,
+ maybeUpdateColumns(partition.get(cd),
+ nowTs, filter(set, factory.regularColumns),
(ts, write) ->
finalPartition.setColumns(cd, ts, write, false));
}
}
@@ -665,24 +674,35 @@ public class ASTSingleTableModel
}
}
- private static void maybeUpdateColumns(Set<Symbol> columns,
- @Nullable BytesPartitionState.Row
row,
- long nowTs, Map<Symbol, Expression>
set,
+ private static Map<ReferenceExpression, Expression>
filter(Map<ReferenceExpression, Expression> map, Set<Symbol> columns)
+ {
+ Map<ReferenceExpression, Expression> update = new HashMap<>();
+ for (var e : map.entrySet())
+ {
+ if (columns.contains(e.getKey().column()))
+ update.put(e.getKey(), e.getValue());
+ }
+ return update;
+ }
+
+ private static void maybeUpdateColumns(@Nullable BytesPartitionState.Row
row,
+ long nowTs,
Map<ReferenceExpression, Expression> set,
ColumnUpdate update)
{
- if (columns.isEmpty())
+ if (set.isEmpty())
{
update.update(nowTs, Collections.emptyMap());
return;
}
// static columns to add in. If we are doing something like += to a
row that doesn't exist, we still update statics...
Map<Symbol, ByteBuffer> write = new HashMap<>();
- for (Symbol col : columns)
+ for (var e : set.entrySet())
{
- ByteBuffer current = row == null ? null : row.get(col);
- EvalResult result = eval(col, current, set.get(col));
+ var col = e.getKey();
+ ByteBuffer current = row == null ? null : row.get(col.column());
+ EvalResult result = eval(col, current, e.getValue());
if (result.kind == EvalResult.Kind.SKIP) continue;
- write.put(col, result.value);
+ write.put(col.column(), result.value);
}
if (!write.isEmpty())
update.update(nowTs, write);
@@ -753,11 +773,124 @@ public class ASTSingleTableModel
return process(Who.cas, updatedCondition, lets);
}
+ public Consumer<ThrowableAssert.ThrowingCallable> shouldReject(Txn txn)
+ {
+ if (!shouldApply(txn)) return null;
+ List<Mutation> mutations = null;
+ if (txn.ifBlock.isPresent()) mutations = txn.ifBlock.get().mutations;
+ else if (!txn.mutations.isEmpty()) mutations = txn.mutations;
+
+ if (mutations == null) return null;
+ Set<Consumer<Throwable>> checks = new HashSet<>();
+ for (var m : mutations)
+ {
+ var failures = shouldReject0(m);
+ if (failures != null)
+ checks.addAll(failures);
+ }
+ return toShouldRejectConsumer(checks);
+ }
+
+ public Consumer<ThrowableAssert.ThrowingCallable> shouldReject(Mutation
mutation)
+ {
+ return toShouldRejectConsumer(shouldReject0(mutation));
+ }
+
+ private static Consumer<ThrowableAssert.ThrowingCallable>
toShouldRejectConsumer(@Nullable Set<Consumer<Throwable>> checks)
+ {
+ return checks == null || checks.isEmpty() ? null
+ : t ->
Assertions.assertThatThrownBy(t)
+
.satisfiesAnyOf(checks.toArray(Consumer[]::new));
+ }
+
+ private Set<Consumer<Throwable>> shouldReject0(Mutation mutation)
+ {
+ if (!shouldApply(mutation)) return null;
+ Set<Consumer<Throwable>> checks = null;
+ if (mutation.kind == Mutation.Kind.UPDATE)
+ {
+ Consumer<Throwable> notFound = t ->
+ Assertions.assertThat(t)
+
.is(AssertionUtils.anyOfThrowable(com.datastax.driver.core.exceptions.InvalidQueryException.class,
// result from java driver
+
InvalidRequestException.class)) // result from jvm-dtest execute api
+ .hasMessage("Attempted to
set an element on a list which is null");
+ Mutation.Update update = mutation.asUpdate();
+ for (var e : update.set.entrySet())
+ {
+ if (e.getKey() instanceof CollectionAccess)
+ {
+ CollectionAccess access = (CollectionAccess) e.getKey();
+ if (access.column().type().getClass() == ListType.class)
+ {
+ // if the column doesn't have data reject
+ for (BytesPartitionState.Ref ref :
referencePartitions(mutation))
+ {
+ BytesPartitionState partition =
partitions.get(ref);
+ if (partition == null)
+ {
+ if (checks == null) checks = new HashSet<>();
+ checks.add(notFound);
+ continue;
+ }
+ List<BytesPartitionState.Row> rows;
+ if
(factory.staticColumns.contains(access.column()))
+ {
+ rows = List.of(partition.staticRow());
+ }
+ else
+ {
+ rows =
cds(mutation).stream().map(partition::get).collect(Collectors.toList());
+ }
+ if (rows.isEmpty())
+ {
+ if (checks == null) checks = new HashSet<>();
+ checks.add(notFound);
+ continue;
+ }
+ for (var row : rows)
+ {
+ if (row == null)
+ {
+ if (checks == null) checks = new
HashSet<>();
+ checks.add(notFound);
+ continue;
+ }
+ if (row.get(access.column()) == null)
+ {
+ if (checks == null) checks = new
HashSet<>();
+ checks.add(notFound);
+ continue;
+ }
+ int offset =
Int32Type.instance.compose(access.element.valueEncoded());
+ ByteBuffer columnValue =
row.get(access.column());
+ var values = ((ListType<?>)
access.column().type()).unpack(columnValue);
+ if (offset < 0 || offset >= values.size())
+ {
+ if (checks == null) checks = new
HashSet<>();
+ checks.add(t -> Assertions.assertThat(t)
+
.is(AssertionUtils.anyOfThrowable(com.datastax.driver.core.exceptions.InvalidQueryException.class,
// result from java driver
+
InvalidRequestException.class)) // result from jvm-dtest
execute api
+
.hasMessage(String.format("List index %s out of bound, list has size %s",
offset, values.size())));
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ return checks;
+ }
+
public BytesPartitionState.Ref referencePartition(Mutation mutation)
{
return factory.createRef(pd(mutation));
}
+ public List<BytesPartitionState.Ref> referencePartitions(Mutation mutation)
+ {
+ return
pds(mutation).stream().map(factory::createRef).collect(Collectors.toList());
+ }
+
private enum Who { cas, accord }
private boolean process(Who who, Conditional condition, Map<String,
SelectResult> lets)
@@ -952,18 +1085,18 @@ public class ASTSingleTableModel
return Pair.create(partitionKeys, other);
}
- private static ImmutableUniqueList<Clustering<ByteBuffer>>
keys(Collection<Symbol> columns, Map<Symbol, List<ByteBuffer>> columnValues)
+ private static ImmutableUniqueList<Clustering<ByteBuffer>>
keys(Collection<Symbol> columns, Map<? extends ReferenceExpression,
List<ByteBuffer>> columnValues)
{
return keys(columns, columnValues, Function.identity());
}
- private static ImmutableUniqueList<Clustering<ByteBuffer>>
keys(Map<Symbol, List<? extends Expression>> values, Collection<Symbol> columns)
+ private static ImmutableUniqueList<Clustering<ByteBuffer>> keys(Map<?
extends ReferenceExpression, List<? extends Expression>> values,
Collection<Symbol> columns)
{
return keys(columns, values, ASTSingleTableModel::eval);
}
private static <T> ImmutableUniqueList<Clustering<ByteBuffer>>
keys(Collection<Symbol> columns,
-
Map<Symbol, ? extends List<? extends T>> columnValues,
+ Map<?
extends ReferenceExpression, ? extends List<? extends T>> columnValues,
Function<T, ByteBuffer> eval)
{
if (columns.isEmpty()) return ImmutableUniqueList.empty();
@@ -1037,16 +1170,51 @@ public class ASTSingleTableModel
return pks.get(0);
}
+ private List<Clustering<ByteBuffer>> pds(Mutation mutation)
+ {
+ switch (mutation.kind)
+ {
+ case INSERT:
+ return pds((Mutation.Insert) mutation);
+ case UPDATE:
+ return pds((Mutation.Update) mutation);
+ case DELETE:
+ return pds((Mutation.Delete) mutation);
+ default:
+ throw new UnsupportedOperationException(mutation.kind.name());
+ }
+ }
+
+ private List<Clustering<ByteBuffer>> pds(Mutation.Insert mutation)
+ {
+ return List.of(key(mutation.values, factory.partitionColumns));
+ }
+
+ private List<Clustering<ByteBuffer>> pds(Mutation.Update mutation)
+ {
+ return pds(mutation.where.simplify());
+ }
+
+ private List<Clustering<ByteBuffer>> pds(Mutation.Delete mutation)
+ {
+ return pds(mutation.where.simplify());
+ }
+
+ private List<Clustering<ByteBuffer>> pds(List<Conditional> conditionals)
+ {
+ return splitOnPartition(conditionals).left;
+ }
+
@Nullable
- private Clustering<ByteBuffer> cdOrNull(Mutation mutation)
+ private List<Clustering<ByteBuffer>> cds(Mutation mutation)
{
- if (factory.clusteringColumns.isEmpty()) return Clustering.EMPTY;
+ if (factory.clusteringColumns.isEmpty()) return
List.of(Clustering.EMPTY);
if (mutation.kind == Mutation.Kind.INSERT)
{
var insert = (Mutation.Insert) mutation;
return
!insert.values.keySet().containsAll(factory.clusteringColumns)
- ? null
- : key(insert.values, factory.clusteringColumns);
+ ? null
+ : List.of(key(insert.values, factory.clusteringColumns));
}
Conditional where;
switch (mutation.kind)
@@ -1056,13 +1224,20 @@ public class ASTSingleTableModel
break;
case DELETE:
where = ((Mutation.Delete) mutation).where;
- break;
+ break;
default:
throw new UnsupportedOperationException("Unexpected mutation:
" + mutation.kind);
}
var partitions = splitOnPartition(where.simplify());
if (partitions.right.isEmpty()) return null;
- var matches = clustering(partitions.right);
+ return clustering(partitions.right);
+ }
+
+ @Nullable
+ private Clustering<ByteBuffer> cdOrNull(Mutation mutation)
+ {
+ var matches = cds(mutation);
+ if (matches == null) return null;
Preconditions.checkArgument(matches.size() == 1);
return matches.get(0);
}
@@ -1654,7 +1829,7 @@ public class ASTSingleTableModel
private List<PrimaryKey> filter(LookupContext ctx, BytesPartitionState
partition)
{
- Map<Symbol, List<? extends Expression>> values = ctx.eq;
+ Map<ReferenceExpression, List<? extends Expression>> values = ctx.eq;
List<PrimaryKey> rows = new ArrayList<>(partition.size());
if (!factory.clusteringColumns.isEmpty() &&
values.keySet().containsAll(factory.clusteringColumns))
{
@@ -1699,10 +1874,11 @@ public class ASTSingleTableModel
return matches;
}
- private Clustering<ByteBuffer> key(Map<Symbol, Expression> values,
ImmutableUniqueList<Symbol> columns)
+ private Clustering<ByteBuffer> key(Map<? extends ReferenceExpression,
Expression> input, ImmutableUniqueList<Symbol> columns)
{
if (columns.isEmpty()) return Clustering.EMPTY;
// same as keys, but only one possible value can happen
+ Map<Symbol, Expression> values = input.entrySet().stream().filter(e ->
e.getKey() instanceof Symbol).map(e -> (Map.Entry<Symbol, Expression>)
e).collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
List<Clustering<ByteBuffer>> keys = keys(Maps.transformValues(values,
Collections::singletonList), columns);
Preconditions.checkState(keys.size() == 1, "Expected 1 key, but found
%s", keys.size());
return keys.get(0);
@@ -1729,8 +1905,35 @@ public class ASTSingleTableModel
}
}
- private static EvalResult eval(Symbol col, @Nullable ByteBuffer current,
Expression e)
+ private static EvalResult eval(ReferenceExpression col, @Nullable
ByteBuffer current, Expression e)
{
+ if (col instanceof Reference)
+ throw new IllegalArgumentException("References are not supported
yet; foo.bar.baz style eval is not yet handled; given " + col.toCQL());
+ if (col instanceof CollectionAccess)
+ {
+ CollectionAccess access = (CollectionAccess) col;
+ CollectionType<?> ct = (CollectionType<?>) access.column().type();
+ switch (ct.kind)
+ {
+ case LIST:
+ {
+ int offset =
Int32Type.instance.compose(eval(access.element));
+ var values = ct.unpack(current);
+ values.set(offset, eval(e));
+ return EvalResult.accept(ct.pack(values));
+ }
+ case MAP:
+ {
+ @SuppressWarnings("unchecked") MapType<Object, Object> mt
= (MapType<Object, Object>) ct;
+ Object key =
mt.nameComparator().compose(eval(access.element));
+ Map<Object, Object> values = current == null ? new
HashMap<>() : mt.compose(current);
+ values.put(key, mt.valueComparator().compose(eval(e)));
+ return EvalResult.accept(mt.decompose(values));
+ }
+ case SET:
+ throw new UnsupportedOperationException("Set collection
access not supported");
+ }
+ }
if (!(e instanceof AssignmentOperator)) return
EvalResult.accept(eval(e));
current = col.type().sanitize(current);
// multi cell collections have the property that they do update even
if the current value is null
@@ -1880,8 +2083,8 @@ public class ASTSingleTableModel
private class LookupContext
{
- private final Map<Symbol, List<? extends Expression>> eq = new
HashMap<>();
- private final Map<Symbol, List<ColumnCondition>> ltOrGt = new
HashMap<>();
+ private final Map<ReferenceExpression, List<? extends Expression>> eq
= new HashMap<>();
+ private final Map<ReferenceExpression, List<ColumnCondition>> ltOrGt =
new HashMap<>();
@Nullable
private Token token = null;
@Nullable
diff --git
a/test/simulator/test/org/apache/cassandra/simulator/test/SingleTableASTSimulation.java
b/test/simulator/test/org/apache/cassandra/simulator/test/SingleTableASTSimulation.java
index ef1f60b470..3b963aed88 100644
---
a/test/simulator/test/org/apache/cassandra/simulator/test/SingleTableASTSimulation.java
+++
b/test/simulator/test/org/apache/cassandra/simulator/test/SingleTableASTSimulation.java
@@ -242,7 +242,9 @@ public class SingleTableASTSimulation extends
SimulationTestBase.SimpleSimulatio
.uniqueBestEffort()
.ofSize(rs.nextInt(1, 20))
.next(rs);
- Gen<Action> mutationGen = toGen(ASTGenerators.mutationBuilder(rs,
model, uniquePartitions, i -> null).build())
+ Gen<Action> mutationGen = toGen(ASTGenerators.mutationBuilder(rs,
model, uniquePartitions, i -> null)
+
.disallowListElementAccessForUpdateSet() //TODO (coverage): CASSANDRA-20828
found an issue with multi cell list type timestamp handling, so make sure
accord doesn't hit this
+ .build())
.map(mutation -> query(mutation));
Gen<Action> selectPartitionGen = Gens.pick(uniquePartitions)
diff --git a/test/unit/org/apache/cassandra/cql3/CQLTester.java
b/test/unit/org/apache/cassandra/cql3/CQLTester.java
index 413b1553fc..4ac41a608b 100644
--- a/test/unit/org/apache/cassandra/cql3/CQLTester.java
+++ b/test/unit/org/apache/cassandra/cql3/CQLTester.java
@@ -232,6 +232,12 @@ import static
org.apache.cassandra.utils.LocalizeString.toLowerCaseLocalized;
*/
public abstract class CQLTester
{
+
+ static
+ {
+ System.setProperty("accord.debug", "true"); // checkstyle: suppress
nearby 'blockSystemPropertyUsage'
+ }
+
/**
* The super user
*/
diff --git a/test/unit/org/apache/cassandra/cql3/ast/CollectionAccess.java
b/test/unit/org/apache/cassandra/cql3/ast/CollectionAccess.java
index 19c8176f6e..d43d7bf34f 100644
--- a/test/unit/org/apache/cassandra/cql3/ast/CollectionAccess.java
+++ b/test/unit/org/apache/cassandra/cql3/ast/CollectionAccess.java
@@ -25,10 +25,10 @@ import org.apache.cassandra.db.marshal.AbstractType;
public class CollectionAccess implements ReferenceExpression
{
private final ReferenceExpression column;
- private final ReferenceExpression element;
+ public final Value element;
private final AbstractType<?> type;
- public CollectionAccess(ReferenceExpression column, ReferenceExpression
element, AbstractType<?> type)
+ public CollectionAccess(ReferenceExpression column, Value element,
AbstractType<?> type)
{
this.column = column;
this.element = element;
@@ -55,4 +55,17 @@ public class CollectionAccess implements ReferenceExpression
{
return Stream.of(column, element);
}
+
+ @Override
+ public Symbol column()
+ {
+ // could be recursive... foo.bar.baz[42]
+ return column.column();
+ }
+
+ @Override
+ public String toString()
+ {
+ return toCQL();
+ }
}
diff --git a/test/unit/org/apache/cassandra/cql3/ast/Conditional.java
b/test/unit/org/apache/cassandra/cql3/ast/Conditional.java
index d2a8e2b125..c7323127f6 100644
--- a/test/unit/org/apache/cassandra/cql3/ast/Conditional.java
+++ b/test/unit/org/apache/cassandra/cql3/ast/Conditional.java
@@ -326,8 +326,8 @@ public interface Conditional extends Expression
interface EqBuilder<T extends EqBuilder<T>>
{
- T value(Symbol symbol, Expression e);
- default T value(Symbol symbol, Object value)
+ T value(ReferenceExpression symbol, Expression e);
+ default T value(ReferenceExpression symbol, Object value)
{
return value(symbol, new Bind(value, symbol.type()));
}
@@ -413,7 +413,7 @@ public interface Conditional extends Expression
T is(ReferenceExpression ref, Is.Kind kind);
@Override
- default T value(Symbol symbol, Expression e)
+ default T value(ReferenceExpression symbol, Expression e)
{
return where(symbol, Where.Inequality.EQUAL, e);
}
diff --git a/test/unit/org/apache/cassandra/cql3/ast/CreateIndexDDL.java
b/test/unit/org/apache/cassandra/cql3/ast/CreateIndexDDL.java
index 91f6167ba5..3e75b9c56b 100644
--- a/test/unit/org/apache/cassandra/cql3/ast/CreateIndexDDL.java
+++ b/test/unit/org/apache/cassandra/cql3/ast/CreateIndexDDL.java
@@ -256,16 +256,21 @@ public class CreateIndexDDL implements Element
public static class CollectionReference implements ReferenceExpression
{
public enum Kind { FULL, KEYS, ENTRIES }
-
public final Kind kind;
- public final ReferenceExpression column;
+ public final ReferenceExpression column;
public CollectionReference(Kind kind, ReferenceExpression column)
{
this.kind = kind;
this.column = column;
}
+ @Override
+ public Symbol column()
+ {
+ return column.column();
+ }
+
@Override
public AbstractType<?> type()
{
diff --git a/test/unit/org/apache/cassandra/cql3/ast/Mutation.java
b/test/unit/org/apache/cassandra/cql3/ast/Mutation.java
index 1031fa0fd2..1a598b6555 100644
--- a/test/unit/org/apache/cassandra/cql3/ast/Mutation.java
+++ b/test/unit/org/apache/cassandra/cql3/ast/Mutation.java
@@ -261,11 +261,11 @@ public abstract class Mutation implements Statement
public static class Insert extends Mutation
{
- public final LinkedHashMap<Symbol, Expression> values;
+ public final LinkedHashMap<ReferenceExpression, Expression> values;
public final boolean ifNotExists;
public final Optional<Using> using;
- public Insert(TableReference table, LinkedHashMap<Symbol, Expression>
values, boolean ifNotExists, Optional<Using> using)
+ public Insert(TableReference table, LinkedHashMap<ReferenceExpression,
Expression> values, boolean ifNotExists, Optional<Using> using)
{
super(Mutation.Kind.INSERT, table);
this.values = values;
@@ -290,7 +290,7 @@ public abstract class Mutation implements Statement
sb.append("INSERT INTO ");
table.toCQL(sb, formatter);
sb.append(" (");
- for (Symbol col : values.keySet())
+ for (ReferenceExpression col : values.keySet())
{
col.toCQL(sb, formatter);
sb.append(", ");
@@ -339,10 +339,10 @@ public abstract class Mutation implements Statement
var u = v.visit(this);
if (u != this) return u;
boolean updated = false;
- LinkedHashMap<Symbol, Expression> copied = new
LinkedHashMap<>(values.size());
+ LinkedHashMap<ReferenceExpression, Expression> copied = new
LinkedHashMap<>(values.size());
for (var e : values.entrySet())
{
- Symbol s = e.getKey();
+ Symbol s = e.getKey().asSymbol();
Symbol s2 = s.visit("INSERT", v);
if (s != s2)
updated = true;
@@ -388,11 +388,11 @@ public abstract class Mutation implements Statement
public static class Update extends Mutation
{
public final Optional<Using> using;
- public final LinkedHashMap<Symbol, Expression> set;
+ public final LinkedHashMap<ReferenceExpression, Expression> set;
public final Conditional where;
public final Optional<? extends CasCondition> casCondition;
- public Update(TableReference table, Optional<Using> using,
LinkedHashMap<Symbol, Expression> set, Conditional where, Optional<? extends
CasCondition> casCondition)
+ public Update(TableReference table, Optional<Using> using,
LinkedHashMap<ReferenceExpression, Expression> set, Conditional where,
Optional<? extends CasCondition> casCondition)
{
super(Mutation.Kind.UPDATE, table);
this.using = using;
@@ -477,10 +477,10 @@ public abstract class Mutation implements Statement
var u = v.visit(this);
if (u != this) return u;
boolean updated = false;
- LinkedHashMap<Symbol, Expression> copied = new
LinkedHashMap<>(set.size());
+ LinkedHashMap<ReferenceExpression, Expression> copied = new
LinkedHashMap<>(set.size());
for (var e : set.entrySet())
{
- Symbol s = e.getKey().visit("UPDATE", v);
+ ReferenceExpression s = e.getKey().visit(v);
if (s != e.getKey())
updated = true;
Expression ex = e.getValue().visit(v);
@@ -736,7 +736,7 @@ WHERE PK_column_conditions
public static class InsertBuilder extends TableBasedBuilder<Insert,
InsertBuilder>
{
- private final LinkedHashMap<Symbol, Expression> values = new
LinkedHashMap<>();
+ private final LinkedHashMap<ReferenceExpression, Expression> values =
new LinkedHashMap<>();
private boolean ifNotExists = false;
private @Nullable TTL ttl;
private @Nullable Timestamp timestamp;
@@ -775,7 +775,7 @@ WHERE PK_column_conditions
}
@Override
- public InsertBuilder value(Symbol ref, Expression e)
+ public InsertBuilder value(ReferenceExpression ref, Expression e)
{
maybePkEq(ref);
values.put(ref, e);
@@ -816,7 +816,7 @@ WHERE PK_column_conditions
Conditional.EqBuilder<B>,
WithTTl<B>, WithTimestamp<B>
{
- B set(Symbol column, Expression value);
+ B set(ReferenceExpression column, Expression value);
default B set(String column, Object value, AbstractType<?> type)
{
return set(new Symbol(column, type), new Literal(value, type));
@@ -837,7 +837,7 @@ WHERE PK_column_conditions
private TableReference table;
private @Nullable TTL ttl;
private @Nullable Timestamp timestamp;
- private final LinkedHashMap<Symbol, Expression> set = new
LinkedHashMap<>();
+ private final LinkedHashMap<ReferenceExpression, Expression> set = new
LinkedHashMap<>();
private final Conditional.Builder where = new Conditional.Builder();
private @Nullable CasCondition casCondition;
@@ -868,7 +868,7 @@ WHERE PK_column_conditions
}
@Override
- public B set(Symbol column, Expression value)
+ public B set(ReferenceExpression column, Expression value)
{
set.put(column, value);
return (B) this;
@@ -967,9 +967,9 @@ WHERE PK_column_conditions
}
@Override
- public TableBasedUpdateBuilder set(Symbol column, Expression value)
+ public TableBasedUpdateBuilder set(ReferenceExpression column,
Expression value)
{
- if (!metadata.regularAndStaticColumns.contains(column))
+ if (!metadata.regularAndStaticColumns.contains(column.column()))
throw new IllegalArgumentException("Attempted to set a non
regular or static column " + column + "; expected " +
metadata.regularAndStaticColumns);
return super.set(column, value);
}
diff --git a/test/unit/org/apache/cassandra/cql3/ast/Reference.java
b/test/unit/org/apache/cassandra/cql3/ast/Reference.java
index ee3b73d6fd..905a9b94ce 100644
--- a/test/unit/org/apache/cassandra/cql3/ast/Reference.java
+++ b/test/unit/org/apache/cassandra/cql3/ast/Reference.java
@@ -30,6 +30,9 @@ import com.google.common.collect.ImmutableList;
import org.apache.cassandra.db.marshal.AbstractType;
import org.apache.cassandra.db.marshal.BytesType;
+/**
+ * Represents {@code foo.bar.baz} syntax. Mainly used for UDT field access
({@code col_udt.foo}) and Accord LET access ({@code row1.foo}).
+ */
public class Reference implements ReferenceExpression
{
public final List<ReferenceExpression> path;
@@ -136,6 +139,12 @@ public class Reference implements ReferenceExpression
return toCQL();
}
+ @Override
+ public Symbol column()
+ {
+ return path.get(0).column();
+ }
+
public static class Builder
{
private final List<ReferenceExpression> path;
diff --git a/test/unit/org/apache/cassandra/cql3/ast/ReferenceExpression.java
b/test/unit/org/apache/cassandra/cql3/ast/ReferenceExpression.java
index d1727fccdb..5e4c471540 100644
--- a/test/unit/org/apache/cassandra/cql3/ast/ReferenceExpression.java
+++ b/test/unit/org/apache/cassandra/cql3/ast/ReferenceExpression.java
@@ -28,4 +28,13 @@ public interface ReferenceExpression extends Expression
{
return v.visit(this);
}
+
+ default Symbol asSymbol()
+ {
+ if (!(this instanceof Symbol))
+ throw new IllegalStateException("Unable to convert type " +
getClass() + " to Symbol");
+ return (Symbol) this;
+ }
+
+ Symbol column();
}
diff --git a/test/unit/org/apache/cassandra/cql3/ast/Symbol.java
b/test/unit/org/apache/cassandra/cql3/ast/Symbol.java
index e492293691..431ce6878a 100644
--- a/test/unit/org/apache/cassandra/cql3/ast/Symbol.java
+++ b/test/unit/org/apache/cassandra/cql3/ast/Symbol.java
@@ -151,6 +151,12 @@ public class Symbol implements ReferenceExpression,
Comparable<Symbol>
return toCQL().compareTo(o.toCQL());
}
+ @Override
+ public Symbol column()
+ {
+ return this;
+ }
+
public static class UnquotedSymbol extends Symbol
{
public UnquotedSymbol(String symbol, AbstractType<?> type)
@@ -158,6 +164,11 @@ public class Symbol implements ReferenceExpression,
Comparable<Symbol>
super(symbol, type);
}
+ public static UnquotedSymbol unknownType(String name)
+ {
+ return new UnquotedSymbol(name, BytesType.instance);
+ }
+
@Override
public void toCQL(StringBuilder sb, CQLFormatter formatter)
{
diff --git
a/test/unit/org/apache/cassandra/cql3/validation/entities/CollectionsTest.java
b/test/unit/org/apache/cassandra/cql3/validation/entities/CollectionsTest.java
index 5af7255516..ed9c837bd9 100644
---
a/test/unit/org/apache/cassandra/cql3/validation/entities/CollectionsTest.java
+++
b/test/unit/org/apache/cassandra/cql3/validation/entities/CollectionsTest.java
@@ -2107,4 +2107,20 @@ public class CollectionsTest extends CQLTester
assertRows(execute("SELECT c['t3'..] FROM %s"), row(set("t3", "t4")));
assertRows(execute("SELECT c[..'t5'] FROM %s"), row(set("t1", "t2",
"t3", "t4")));
}
+
+ /**
+ * {@link org.apache.cassandra.db.rows.Row.Builder} is cached, and there
was logic to reuse the same builder cross
+ * rows, even if the builder was returned to the pool! This is "fine" in
the happy path, but if the second row fails
+ * for any reason, then the builder is in a partial state and also
referenced by the pool!
+ */
+ @Test
+ public void CASSANDRA_21055()
+ {
+ createTable("CREATE TABLE %s (pk int, ck int, l list<int>, PRIMARY KEY
(pk, ck))");
+
+ execute("INSERT INTO %s (pk, ck, l) VALUES (1, 1, [10, 20, 30])");
+ assertInvalidMessage("Attempted to set an element on a list which is
null",
+ "UPDATE %s SET l[0] = 0 WHERE pk=1 AND ck IN (1,
2)");
+ execute("INSERT INTO %s (pk, ck, l) VALUES (1, 2, [40, 50, 60])");
+ }
}
diff --git a/test/unit/org/apache/cassandra/utils/ASTGenerators.java
b/test/unit/org/apache/cassandra/utils/ASTGenerators.java
index 9cd39abba7..d433cc73fa 100644
--- a/test/unit/org/apache/cassandra/utils/ASTGenerators.java
+++ b/test/unit/org/apache/cassandra/utils/ASTGenerators.java
@@ -50,6 +50,7 @@ import org.apache.cassandra.cql3.KnownIssue;
import org.apache.cassandra.cql3.ast.AssignmentOperator;
import org.apache.cassandra.cql3.ast.Bind;
import org.apache.cassandra.cql3.ast.CasCondition;
+import org.apache.cassandra.cql3.ast.CollectionAccess;
import org.apache.cassandra.cql3.ast.Conditional;
import org.apache.cassandra.cql3.ast.CreateIndexDDL;
import org.apache.cassandra.cql3.ast.Expression;
@@ -64,6 +65,7 @@ import org.apache.cassandra.cql3.ast.Txn;
import org.apache.cassandra.cql3.ast.TypeHint;
import org.apache.cassandra.cql3.ast.Value;
import org.apache.cassandra.db.marshal.AbstractType;
+import org.apache.cassandra.db.marshal.CollectionType;
import org.apache.cassandra.db.marshal.Int32Type;
import org.apache.cassandra.db.marshal.IntegerType;
import org.apache.cassandra.db.marshal.ListType;
@@ -392,6 +394,11 @@ public class ASTGenerators
}
}
+ public interface ValueGen
+ {
+ Value generate(RandomnessSource rnd, Object value, AbstractType<?>
type);
+ }
+
public static class MutationGenBuilder
{
public enum DeleteKind { Partition, Row, Column }
@@ -414,6 +421,9 @@ public class ASTGenerators
private boolean allowUpdateMultiplePartitionKeys = true;
private boolean allowUpdateMultipleClusteringKeys = true;
private EnumSet<KnownIssue> ignoreIssues = IGNORED_ISSUES;
+ private EnumSet<CollectionType.Kind>
allowedCollectionElementAccessForUpdateSet =
EnumSet.of(CollectionType.Kind.LIST, CollectionType.Kind.MAP);
+ private Gen<Boolean> collectionElementAccessForUpdateSet =
SourceDSL.booleans().all();
+ private ValueGen valueGen = (rnd, obj, type) ->
SourceDSL.booleans().all().generate(rnd) ? new Bind(obj, type) : new
Literal(obj, type);
public MutationGenBuilder(TableMetadata metadata)
{
@@ -432,6 +442,18 @@ public class ASTGenerators
columnExpressions.put(symbol, new
ExpressionBuilder(symbol.type()));
}
+ public MutationGenBuilder disallowListElementAccessForUpdateSet()
+ {
+ return withListElementAccessForUpdateSet(false);
+ }
+
+ public MutationGenBuilder withListElementAccessForUpdateSet(boolean
allow)
+ {
+ if (allow)
allowedCollectionElementAccessForUpdateSet.add(CollectionType.Kind.LIST);
+ else
allowedCollectionElementAccessForUpdateSet.remove(CollectionType.Kind.LIST);
+ return this;
+ }
+
public MutationGenBuilder withIgnoreIssues(EnumSet<KnownIssue>
ignoreIssues)
{
this.ignoreIssues = Objects.requireNonNull(ignoreIssues);
@@ -986,6 +1008,33 @@ public class ASTGenerators
{
for (Symbol c : new ArrayList<>(columnsToGenerate))
{
+ if (c.type().isMultiCell()
+ && c.type().isCollection()
+ &&
!allowedCollectionElementAccessForUpdateSet.isEmpty())
+ {
+ CollectionType<?> ct = (CollectionType<?>) c.type();
+ if
(allowedCollectionElementAccessForUpdateSet.contains(ct.kind)
+ &&
collectionElementAccessForUpdateSet.generate(rnd))
+ {
+ //TODO (coverage): nothing stops the following:
c[0] = 42, c[1] = 72, just not impl in generator yet
+ Value key;
+ switch (ct.kind)
+ {
+ case LIST:
+ key = valueGen.generate(rnd, (int)
rnd.next(Constraint.between(0, 10)), Int32Type.instance);
+ break;
+ case MAP:
+ key = valueGen.generate(rnd,
getTypeSupport(ct.nameComparator()).bytesGen().generate(rnd),
ct.nameComparator());
+ break;
+ default:
+ throw new
UnsupportedOperationException(ct.kind.name());
+ }
+ CollectionAccess ref = new CollectionAccess(c,
key, ct.valueComparator());
+ builder.value(ref,
getTypeSupport(ct.valueComparator()).bytesGen().generate(rnd));
+ columnsToGenerate.remove(c);
+ continue;
+ }
+ }
var useOperator = columnExpressions.get(c).useOperator;
EnumSet<AssignmentOperator.Kind> additionOperatorAllowed =
AssignmentOperator.supportsOperators(c.type(), isTransaction);
if (!additionOperatorAllowed.isEmpty() &&
useOperator.generate(rnd))
@@ -1094,7 +1143,8 @@ public class ASTGenerators
}
MutationGenBuilder mutationBuilder = new
MutationGenBuilder(metadata)
.withTxnSafe()
-
.disallowUpdateMultiplePartitionKeys()
+
.disallowUpdateMultiplePartitionKeys() //TODO (coverage): this is something
Accord should support, so should remove and make sure accord is updated
+
.disallowListElementAccessForUpdateSet() //TODO (coverage): CASSANDRA-20828
found an issue with multi cell list type timestamp handling, so make sure
accord doesn't hit this
.withReferences(new
ArrayList<>(builder.allowedReferences()));
if (!allowReferences)
mutationBuilder.withReferences(Collections.emptyList());
@@ -1223,7 +1273,8 @@ public class ASTGenerators
{
MutationGenBuilder builder = mutationBuilder(IGNORED_ISSUES, rs,
model, List.of(pk), indexes)
.withTxnSafe()
-
.disallowUpdateMultiplePartitionKeys();
+
.disallowUpdateMultiplePartitionKeys() //TODO (coverage): this is something
Accord should support, so should remove and make sure accord is updated
+
.disallowListElementAccessForUpdateSet(); //TODO (coverage): CASSANDRA-20828
found an issue with multi cell list type timestamp handling, so make sure
accord doesn't hit this
if (!allowEmpty)
builder.disallowEmpty();
return builder.build();
diff --git a/test/unit/org/apache/cassandra/utils/AssertionUtils.java
b/test/unit/org/apache/cassandra/utils/AssertionUtils.java
index 6a0a1f5fc6..d44fe7f84e 100644
--- a/test/unit/org/apache/cassandra/utils/AssertionUtils.java
+++ b/test/unit/org/apache/cassandra/utils/AssertionUtils.java
@@ -38,6 +38,7 @@ public class AssertionUtils
return Assertions.anyOf(it);
}
+ @SafeVarargs
public static Condition<Throwable> anyOfThrowable(Class<? extends
Throwable>... klasses)
{
return anyOf(Stream.of(klasses).map(AssertionUtils::isThrowable));
@@ -147,6 +148,7 @@ public class AssertionUtils
return hasCause(isThrowable(klass));
}
+ @SafeVarargs
public static Condition<Throwable> hasCauseAnyOf(Class<? extends
Throwable>... matchers)
{
return hasCause(anyOfThrowable(matchers));
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
