dcapwell commented on code in PR #4024:
URL: https://github.com/apache/cassandra/pull/4024#discussion_r2025165752
##########
test/harry/main/org/apache/cassandra/harry/model/ASTSingleTableModel.java:
##########
@@ -341,61 +380,142 @@ else if (!clusterings.isEmpty())
}
}
+ private static void maybeUpdateColumns(Set<Symbol> columns,
+ @Nullable BytesPartitionState.Row
row,
+ long nowTs, Map<Symbol, Expression>
set,
+ ColumnUpdate update)
+ {
+ if (columns.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, Update> write = new HashMap<>();
+ for (Symbol col : columns)
+ {
+ ByteBuffer current = row == null ? null : row.get(col);
+ Update result = eval(col, current, set.get(col));
+ if (result.kind == Update.Kind.SKIP) continue;
+ write.put(col, result);
+ }
+ if (!write.isEmpty())
+ update.update(nowTs, write);
+ }
+
+ public boolean shouldApply(Batch batch)
+ {
+ if (!batch.isCas()) return true;
+ //TODO (correctness): what happens with CAS on multiple partitions?
+ boolean shouldApply =
batch.mutations.stream().allMatch(this::shouldApply);
+ if (shouldApply
+ &&
ignoredIssues.contains(KnownIssue.BATCH_CAS_DELETE_STATIC_WITH_UPDATE_IF_NOT_EXISTS)
+ && !factory.staticColumns.isEmpty())
+ {
+ // No really, should it?
+ // Let's say you have 2 mutations
+ // DELETE s0 FROM tbl WHERE pk=0 IF EXISTS
+ // INSERT INTO tbl (pk, ck, r) VALUES (0, 0, 0) IF NOT EXISTS
+ // There are 2 cases this query can be in: static columns have
values, static columns are null
+ // When the static columns are not null, then this applies as
expected, but when the static columns are null then the CAS read query returns
empty!
+ // CAS does "SELECT s0, r FROM tbl WHERE pk=0 AND ck=0" and since
the row and the static columns are null, the empty result is generated; so the
DELETE's condition of "IF EXISTS" can not apply!
+ // Now, it doesn't mean that the partition doesn't exist, the
DELETE outside a batch would do "SELECT s0 FROM tbl WHERE pk=0 LIMIT 1"
+ boolean partitionExistsButCasConditionWillFail = false;
+ boolean rowDoesNotExist = false;
+ for (var m : batch.mutations)
+ {
+ if (m.casCondition().isEmpty()) continue;
+ var condition = m.casCondition().get();
+
+ var row = referenceRow(m);
+ if (m.kind == Mutation.Kind.DELETE)
+ {
+ var delete = (Mutation.Delete) m;
+ if (!delete.columns.isEmpty()
+ && row.clustering == null // delete is partition
level, so deletes static columns
+ && existsOrColumnConditionsEqNull(condition))
+ {
+ // are the columns deleted currently null?
+ var partition = partitions.get(row.partition);
+ if (partition != null)
+ {
+ if (partition.staticRow().isEmpty())
+ partitionExistsButCasConditionWillFail = true;
+ }
+ }
+ }
+ if (row.clustering != null && condition ==
CasCondition.Simple.NotExists)
+ rowDoesNotExist = true;
+ }
+ if (rowDoesNotExist && partitionExistsButCasConditionWillFail)
+ return false;
+ }
+ return shouldApply;
+ }
+
+ private static boolean existsOrColumnConditionsEqNull(CasCondition
condition)
+ {
+ if (condition == CasCondition.Simple.Exists) return true;
+ var conditions = ((CasCondition.IfCondition)
condition).conditional.simplify();
+ return conditions.stream().allMatch(c -> {
+ if (!(c instanceof Conditional.Where)) return false;
+ var where = (Conditional.Where) c;
+ if (where.kind != Inequality.EQUAL) return false;
+ return ExpressionEvaluator.eval(where.rhs) == null;
+ });
+ }
+
public boolean shouldApply(Mutation mutation)
{
if (!mutation.isCas()) return true;
return shouldApply(mutation, selectPartitionForCAS(mutation));
}
- private SelectResult selectPartitionForCAS(Mutation mutation)
+ private CasContext selectPartitionForCAS(Mutation mutation)
{
- var partition = partitions.get(factory.createRef(pd(mutation)));
- if (partition == null) return
SelectResult.ordered(factory.selectionOrder, NO_ROWS);
-
- var cd = cdOrNull(mutation);
- var row = cd == null ? null : partition.get(cd);
- ImmutableUniqueList<Symbol> columns = cd != null ?
factory.selectionOrder : factory.partitionAndStaticColumns;
- return SelectResult.ordered(columns, new ByteBuffer[][] {
getRowAsByteBuffer(columns, partition, row)});
+ BytesPartitionState.Ref ref = referencePartition(mutation);
+ Clustering<ByteBuffer> cd = cdOrNull(mutation);
+ BytesPartitionState partition = partitions.get(ref);
+ return new CasContext(ref, cd, partition);
}
- private boolean shouldApply(Mutation mutation, SelectResult current)
+ private boolean shouldApply(Mutation mutation, CasContext ctx)
{
Preconditions.checkArgument(mutation.isCas());
// process condition
- CasCondition condition;
- switch (mutation.kind)
- {
- case INSERT:
- condition = CasCondition.Simple.NotExists;
- break;
- case UPDATE:
- condition = ((Mutation.Update) mutation).casCondition.get();
- break;
- case DELETE:
- condition = ((Mutation.Delete) mutation).casCondition.get();
- break;
- default:
- throw new UnsupportedOperationException(mutation.kind.name());
- }
+ CasCondition condition = mutation.casCondition().get();
+ boolean partitionOrRow = ctx.clustering == null;
+ boolean partitionKnown = ctx.partition != null;
+ BytesPartitionState.Row row = partitionKnown && !partitionOrRow
+ ? ctx.partition.get(ctx.clustering)
+ : null;
if (condition instanceof CasCondition.Simple)
{
- boolean hasPartition = current.rows.length > 0;
- boolean partitionOrRow =
current.columns.equals(factory.partitionAndStaticColumns);
- boolean hasRow = partitionOrRow ? hasPartition :
current.isAllDefined(factory.clusteringColumns);
+ if (partitionOrRow && factory.staticColumns.isEmpty())
+ throw new AssertionError("Attempted to create a EXISTS
condition on partition without static columns; " + mutation.toCQL());
+ // CAS's definition of partition EXISTS isn't based off the
partition existing, its based off the static row
+ // existing (aka at least 1 static column exists and is not null).
+ boolean hasPartition = partitionKnown &&
!ctx.partition.staticRow().isEmpty();
+ boolean hasRow = row != null && !row.isEmpty();
var simple = (CasCondition.Simple) condition;
switch (simple)
{
case Exists:
- return hasRow;
+ return partitionOrRow ? hasPartition : hasRow;
Review Comment:
turns out that CAS defines partition `EXISTS` as
1) partition is defined
2) static row is not empty
The previous logic defined it only as #1 so had to make this change to
support the empty static row check
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For additional commands, e-mail: pr-h...@cassandra.apache.org
