From aeff0295fc6247806b0641ddeba49c88eb36c392 Mon Sep 17 00:00:00 2001 From: Alan Wang Date: Fri, 12 Jun 2026 12:08:31 -0700 Subject: [PATCH 01/16] added support for LET comparisons --- src/antlr/Parser.g | 10 ++ .../cql3/transactions/ConditionStatement.java | 45 +++++- .../service/accord/txn/TxnCondition.java | 143 ++++++++++++++++-- .../cassandra/utils/NullableSerializer.java | 20 +++ .../test/accord/AccordCQLTestBase.java | 29 ++++ 5 files changed, 226 insertions(+), 21 deletions(-) diff --git a/src/antlr/Parser.g b/src/antlr/Parser.g index 8794d00d02a9..f6ded99a9045 100644 --- a/src/antlr/Parser.g +++ b/src/antlr/Parser.g @@ -836,6 +836,15 @@ txnConditionKind returns [ConditionStatement.Kind op] | '!=' { $op = ConditionStatement.Kind.NEQ; } ; +txnConditionKindRef returns [ConditionStatement.Kind op] + : '=' { $op = ConditionStatement.Kind.EQ_REF; } + | '<' { $op = ConditionStatement.Kind.LT_REF; } + | '<=' { $op = ConditionStatement.Kind.LTE_REF; } + | '>' { $op = ConditionStatement.Kind.GT_REF; } + | '>=' { $op = ConditionStatement.Kind.GTE_REF; } + | '!=' { $op = ConditionStatement.Kind.NEQ_REF; } + ; + txnColumnCondition[List conditions] : lhs=rowDataReference ( @@ -845,6 +854,7 @@ txnColumnCondition[List conditions] | K_NULL { conditions.add(new ConditionStatement.Raw(lhs, ConditionStatement.Kind.IS_NULL, null)); } ) | (txnConditionKind term)=> op=txnConditionKind t=term { conditions.add(new ConditionStatement.Raw(lhs, op, t)); } + | (txnConditionKindRef rowDataReference)=> op=txnConditionKindRef rhs=rowDataReference { conditions.add(new ConditionStatement.Raw(lhs, op, rhs)); } ) | lhs=term op=txnConditionKind rhs=rowDataReference { conditions.add(new ConditionStatement.Raw(lhs, op, rhs)); } ; diff --git a/src/java/org/apache/cassandra/cql3/transactions/ConditionStatement.java b/src/java/org/apache/cassandra/cql3/transactions/ConditionStatement.java index 7f19d834d934..d051353ba510 100644 --- a/src/java/org/apache/cassandra/cql3/transactions/ConditionStatement.java +++ b/src/java/org/apache/cassandra/cql3/transactions/ConditionStatement.java @@ -40,8 +40,14 @@ public enum Kind GT(TxnCondition.Kind.GREATER_THAN, TxnCondition.Kind.LESS_THAN), GTE(TxnCondition.Kind.GREATER_THAN_OR_EQUAL, TxnCondition.Kind.LESS_THAN_OR_EQUAL), LT(TxnCondition.Kind.LESS_THAN, TxnCondition.Kind.GREATER_THAN), - LTE(TxnCondition.Kind.LESS_THAN_OR_EQUAL, TxnCondition.Kind.GREATER_THAN_OR_EQUAL); - + LTE(TxnCondition.Kind.LESS_THAN_OR_EQUAL, TxnCondition.Kind.GREATER_THAN_OR_EQUAL), + EQ_REF(TxnCondition.Kind.EQUAL_REF, TxnCondition.Kind.EQUAL_REF), + NEQ_REF(TxnCondition.Kind.NOT_EQUAL_REF, TxnCondition.Kind.NOT_EQUAL_REF), + GT_REF(TxnCondition.Kind.GREATER_THAN_REF, TxnCondition.Kind.LESS_THAN_REF), + GTE_REF(TxnCondition.Kind.GREATER_THAN_OR_EQUAL_REF, TxnCondition.Kind.LESS_THAN_OR_EQUAL_REF), + LT_REF(TxnCondition.Kind.LESS_THAN_REF, TxnCondition.Kind.GREATER_THAN_REF), + LTE_REF(TxnCondition.Kind.LESS_THAN_OR_EQUAL_REF, TxnCondition.Kind.GREATER_THAN_OR_EQUAL_REF); + // TODO: Support for IN, CONTAINS, CONTAINS KEY private final TxnCondition.Kind kind; @@ -100,8 +106,13 @@ public ConditionStatement prepare(String keyspace, VariableSpecifications bindVa RowDataReference reference; Term value; boolean reversed = false; - - if (lhs instanceof RowDataReference.Raw) + + if (lhs instanceof RowDataReference.Raw && rhs instanceof RowDataReference.Raw) + { + reference = ((RowDataReference.Raw) lhs).prepareAsReceiver(); + value = ((RowDataReference.Raw) rhs).prepareAsReceiver(); + } + else if (lhs instanceof RowDataReference.Raw) { if (((RowDataReference.Raw) lhs).column() == null) throw new IllegalStateException(String.format("Row reference (%s) can only be used with IS NULL/IS NOT NULL conditions", lhs.getText())); @@ -143,13 +154,31 @@ public TxnCondition createCondition(QueryOptions options) case GTE: case LT: case LTE: - // TODO: Support for references on LHS and RHS - TxnReference ref = reference.toTxnReference(options); - checkTrue(ref.kind == TxnReference.Kind.COLUMN, "Condition %s requires COLUMN reference but given %s", kind, ref.kind); - return new TxnCondition.Value(ref.asColumn(), + { + TxnReference refLHS = reference.toTxnReference(options); + checkTrue(refLHS.kind == TxnReference.Kind.COLUMN, "Condition %s requires COLUMN reference but given %s", kind, refLHS.kind); + + return new TxnCondition.Value(refLHS.asColumn(), kind.toTxnKind(reversed), value.bindAndGet(options), options.getProtocolVersion()); + } + case EQ_REF: + case NEQ_REF: + case GT_REF: + case GTE_REF: + case LT_REF: + case LTE_REF: + { + TxnReference refLHS = reference.toTxnReference(options); + checkTrue(refLHS.kind == TxnReference.Kind.COLUMN, "Condition %s requires COLUMN reference but given %s", kind, refLHS.kind); + TxnReference refRHS = ((RowDataReference) value).toTxnReference(options); + checkTrue(refRHS.kind == TxnReference.Kind.COLUMN, "Condition %s requires COLUMN reference but given %s", kind, refRHS.kind); + return new TxnCondition.Reference(refLHS.asColumn(), + kind.toTxnKind(reversed), + refRHS.asColumn(), + options.getProtocolVersion()); + } default: throw new IllegalStateException(); } diff --git a/src/java/org/apache/cassandra/service/accord/txn/TxnCondition.java b/src/java/org/apache/cassandra/service/accord/txn/TxnCondition.java index 380566155ff7..93df1ec5063d 100644 --- a/src/java/org/apache/cassandra/service/accord/txn/TxnCondition.java +++ b/src/java/org/apache/cassandra/service/accord/txn/TxnCondition.java @@ -106,28 +106,37 @@ private interface ConditionSerializer public enum Kind { - NONE("n/a", null), - AND("AND", null), - OR("OR", null), - IS_NOT_NULL("IS NOT NULL", null), - IS_NULL("IS NULL", null), - EQUAL("=", Operator.EQ), - NOT_EQUAL("!=", Operator.NEQ), - GREATER_THAN(">", Operator.GT), - GREATER_THAN_OR_EQUAL(">=", Operator.GTE), - LESS_THAN("<", Operator.LT), - LESS_THAN_OR_EQUAL("<=", Operator.LTE), - COLUMN_CONDITIONS("COLUMN_CONDITIONS", null); + NONE("n/a", null, null), + AND("AND", null, null), + OR("OR", null, null), + IS_NOT_NULL("IS NOT NULL", null, null), + IS_NULL("IS NULL", null, null), + EQUAL("=", Operator.EQ, false), + NOT_EQUAL("!=", Operator.NEQ, false), + GREATER_THAN(">", Operator.GT, false), + GREATER_THAN_OR_EQUAL(">=", Operator.GTE, false), + LESS_THAN("<", Operator.LT, false), + LESS_THAN_OR_EQUAL("<=", Operator.LTE, false), + COLUMN_CONDITIONS("COLUMN_CONDITIONS", null, false), + EQUAL_REF("=", Operator.EQ, true), + NOT_EQUAL_REF("!=", Operator.NEQ, true), + GREATER_THAN_REF(">", Operator.GT, true), + GREATER_THAN_OR_EQUAL_REF(">=", Operator.GTE, true), + LESS_THAN_REF("<", Operator.LT, true), + LESS_THAN_OR_EQUAL_REF("<=", Operator.LTE, true); @Nonnull private final String symbol; @Nullable private final Operator operator; + @Nullable + private final Boolean isReference; - Kind(String symbol, Operator operator) + Kind(String symbol, Operator operator, Boolean isReference) { this.symbol = symbol; this.operator = operator; + this.isReference = isReference; } @SuppressWarnings("rawtypes") @@ -145,6 +154,13 @@ private ConditionSerializer serializer() case GREATER_THAN: case GREATER_THAN_OR_EQUAL: return Value.serializer; + case EQUAL_REF: + case NOT_EQUAL_REF: + case LESS_THAN_REF: + case LESS_THAN_OR_EQUAL_REF: + case GREATER_THAN_REF: + case GREATER_THAN_OR_EQUAL_REF: + return Reference.serializer; case AND: case OR: return BooleanGroup.serializer; @@ -618,6 +634,107 @@ public long serializedSize(Value condition, TableMetadatas tables) }; } + public static class Reference extends TxnCondition + { + private static final EnumSet KINDS = EnumSet.of(Kind.EQUAL_REF, Kind.NOT_EQUAL_REF, + Kind.GREATER_THAN_REF, Kind.GREATER_THAN_OR_EQUAL_REF, + Kind.LESS_THAN_REF, Kind.LESS_THAN_OR_EQUAL_REF); + + private final TxnReference.ColumnReference referenceLHS; + private final TxnReference.ColumnReference referenceRHS; + private final ProtocolVersion version; + + public Reference(TxnReference.ColumnReference referenceLHS, Kind kind, TxnReference.ColumnReference referenceRHS, ProtocolVersion version) + { + super(kind); + Invariants.requireArgument(KINDS.contains(kind), "Kind " + kind + " cannot be used with a value condition"); + this.referenceLHS = referenceLHS; + this.referenceRHS = referenceRHS; + this.version = version; + } + + public static EnumSet supported() + { + return EnumSet.copyOf(KINDS); + } + + @Override + public boolean equals(Object o) + { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + if (!super.equals(o)) return false; + Reference reference1 = (Reference) o; + return referenceLHS.equals(reference1.referenceLHS) && referenceRHS.equals(reference1.referenceRHS); + } + + @Override + public void collect(TableMetadatas.Collector collector) + { + referenceLHS.collect(collector); + referenceRHS.collect(collector); + } + + @Override + public int hashCode() + { + return Objects.hash(super.hashCode(), referenceLHS, referenceRHS); + } + + @Override + public String toString() + { + return referenceLHS.toString() + ' ' + kind.symbol + ' ' + referenceRHS.toString(); + } + + @Override + public boolean applies(TxnData data) + { + ColumnMetadata columnLHS = referenceLHS.column(); + ColumnMetadata columnRHS = referenceRHS.column(); + + Preconditions.checkArgument(columnLHS.type.equals(columnRHS.type), columnLHS.type + " != " + columnRHS.type); + + ByteBuffer lhs = referenceLHS.toByteBuffer(data, columnLHS.type); + ByteBuffer rhs = referenceRHS.toByteBuffer(data, columnRHS.type); + + if (lhs == null || rhs == null) + return false; + + return kind.operator.isSatisfiedBy(columnLHS.type, lhs, rhs); + } + + private static final ConditionSerializer serializer = new ConditionSerializer<>() + { + @Override + public void serialize(Reference condition, TableMetadatas tables, DataOutputPlus out) throws IOException + { + TxnReference.serializer.serialize(condition.referenceLHS, tables, out); + TxnReference.serializer.serialize(condition.referenceRHS, tables, out); + out.writeUTF(condition.version.name()); + } + + @Override + public Reference deserialize(TableMetadatas tables, DataInputPlus in, Kind kind) throws IOException + { + TxnReference.ColumnReference referenceLHS = TxnReference.serializer.deserialize(tables, in).asColumn(); + TxnReference.ColumnReference referenceRHS = TxnReference.serializer.deserialize(tables, in).asColumn(); + ProtocolVersion protocolVersion = ProtocolVersion.valueOf(in.readUTF()); + return new Reference(referenceLHS, kind, referenceRHS, protocolVersion); + } + + @Override + public long serializedSize(Reference condition, TableMetadatas tables) + { + long size = 0; + size += TxnReference.serializer.serializedSize(condition.referenceLHS, tables); + size += TxnReference.serializer.serializedSize(condition.referenceRHS, tables); + size += TypeSizes.sizeof(condition.version.name()); + return size; + } + }; + } + public static class BooleanGroup extends TxnCondition { private static final Set KINDS = ImmutableSet.of(Kind.AND, Kind.OR); diff --git a/src/java/org/apache/cassandra/utils/NullableSerializer.java b/src/java/org/apache/cassandra/utils/NullableSerializer.java index 8392bf19dc85..b2ed0124decd 100644 --- a/src/java/org/apache/cassandra/utils/NullableSerializer.java +++ b/src/java/org/apache/cassandra/utils/NullableSerializer.java @@ -22,6 +22,7 @@ import org.apache.cassandra.db.TypeSizes; import org.apache.cassandra.io.IVersionedSerializer; +import org.apache.cassandra.io.ParameterisedUnversionedSerializer; import org.apache.cassandra.io.UnversionedSerializer; import org.apache.cassandra.io.VersionedSerializer; import org.apache.cassandra.io.util.DataInputPlus; @@ -36,6 +37,13 @@ public static void serializeNullable(T value, DataOutputPlus out, Unversione serializer.serialize(value, out); } + public static void serializeNullable(T value, P param, DataOutputPlus out, ParameterisedUnversionedSerializer serializer) throws IOException + { + out.writeBoolean(value != null); + if (value != null) + serializer.serialize(value, param, out); + } + public static void serializeNullable(T value, DataOutputPlus out, int version, IVersionedSerializer serializer) throws IOException { out.writeBoolean(value != null); @@ -55,6 +63,11 @@ public static T deserializeNullable(DataInputPlus in, UnversionedSerializer< return in.readBoolean() ? serializer.deserialize(in) : null; } + public static T deserializeNullable(DataInputPlus in, P param, ParameterisedUnversionedSerializer serializer) throws IOException + { + return in.readBoolean() ? serializer.deserialize(param, in) : null; + } + public static T deserializeNullable(DataInputPlus in, int version, IVersionedSerializer serializer) throws IOException { return in.readBoolean() ? serializer.deserialize(in, version) : null; @@ -72,6 +85,13 @@ public static long serializedNullableSize(T value, UnversionedSerializer : TypeSizes.sizeof(false); } + public static long serializedNullableSize(T value, P param, ParameterisedUnversionedSerializer serializer) + { + return value != null + ? TypeSizes.sizeof(true) + serializer.serializedSize(value, param) + : TypeSizes.sizeof(false); + } + public static long serializedNullableSize(T value, int version, IVersionedSerializer serializer) { return value != null diff --git a/test/distributed/org/apache/cassandra/distributed/test/accord/AccordCQLTestBase.java b/test/distributed/org/apache/cassandra/distributed/test/accord/AccordCQLTestBase.java index f38b84ede287..e1af11631331 100644 --- a/test/distributed/org/apache/cassandra/distributed/test/accord/AccordCQLTestBase.java +++ b/test/distributed/org/apache/cassandra/distributed/test/accord/AccordCQLTestBase.java @@ -3604,4 +3604,33 @@ public void userSeesInvalidRejection() throws Exception .hasMessage("Attempted to set an element on a list which is null"); }); } + + @Test + public void testLetComparisonTransactionStatement() throws Throwable + { + test("CREATE TABLE " + qualifiedAccordTableName + " (k int PRIMARY KEY, v int) WITH " + transactionalMode.asCqlParam(), cluster -> { + String insert = "BEGIN TRANSACTION\n" + + "INSERT INTO " + qualifiedAccordTableName + " (k, v) VALUES (1, 2);\n" + + "INSERT INTO " + qualifiedAccordTableName + " (k, v) VALUES (2, 3);\n" + + "COMMIT TRANSACTION"; + + cluster.coordinator(1).executeWithResult(insert, ConsistencyLevel.SERIAL); + + String query = "BEGIN TRANSACTION\n" + + "LET k1 = (SELECT v FROM " + qualifiedAccordTableName + " WHERE k = 1);\n" + + "LET k2 = (SELECT v FROM " + qualifiedAccordTableName + " WHERE k = 2);\n" + + "IF k1.v < k2.v THEN \n" + + " UPDATE " + qualifiedAccordTableName + " SET v = 10 WHERE k = 1;\n" + + "END IF\n" + + "COMMIT TRANSACTION"; + + cluster.coordinator(1).executeWithResult(query, ConsistencyLevel.SERIAL); + + String read = "BEGIN TRANSACTION\n" + + "SELECT * FROM " + qualifiedAccordTableName + " WHERE k = 1;\n" + + "COMMIT TRANSACTION"; + SimpleQueryResult result = cluster.coordinator(1).executeWithResult(read, ConsistencyLevel.SERIAL); + assertThat(result).hasSize(1).contains(1, 10); + }); + } } From 473035c8ca57676610547723e168efa8a03b2deb Mon Sep 17 00:00:00 2001 From: Alan Wang Date: Fri, 12 Jun 2026 13:42:32 -0700 Subject: [PATCH 02/16] tests --- .../test/accord/AccordCQLTestBase.java | 30 +++++ .../service/accord/txn/TxnConditionTest.java | 113 +++++++++++++++++- 2 files changed, 140 insertions(+), 3 deletions(-) diff --git a/test/distributed/org/apache/cassandra/distributed/test/accord/AccordCQLTestBase.java b/test/distributed/org/apache/cassandra/distributed/test/accord/AccordCQLTestBase.java index e1af11631331..f77d31ebb875 100644 --- a/test/distributed/org/apache/cassandra/distributed/test/accord/AccordCQLTestBase.java +++ b/test/distributed/org/apache/cassandra/distributed/test/accord/AccordCQLTestBase.java @@ -3633,4 +3633,34 @@ public void testLetComparisonTransactionStatement() throws Throwable assertThat(result).hasSize(1).contains(1, 10); }); } + + @Test + public void testLetComparisionWithDifferingTypes() throws Throwable + { + + test("CREATE TABLE " + qualifiedAccordTableName + " (k int PRIMARY KEY, v int) WITH " + transactionalMode.asCqlParam(), cluster -> { + String insert = "BEGIN TRANSACTION\n" + + "INSERT INTO " + qualifiedAccordTableName + " (k, v) VALUES (1, 2);\n" + + "INSERT INTO " + qualifiedAccordTableName + " (k, v) VALUES (2, 3);\n" + + "COMMIT TRANSACTION"; + + cluster.coordinator(1).executeWithResult(insert, ConsistencyLevel.SERIAL); + + String query = "BEGIN TRANSACTION\n" + + "LET k1 = (SELECT v FROM " + qualifiedAccordTableName + " WHERE k = 1);\n" + + "LET k2 = (SELECT v FROM " + qualifiedAccordTableName + " WHERE k = 2);\n" + + "IF k1.v < k2.v THEN \n" + + " UPDATE " + qualifiedAccordTableName + " SET v = 10 WHERE k = 1;\n" + + "END IF\n" + + "COMMIT TRANSACTION"; + + cluster.coordinator(1).executeWithResult(query, ConsistencyLevel.SERIAL); + + String read = "BEGIN TRANSACTION\n" + + "SELECT * FROM " + qualifiedAccordTableName + " WHERE k = 1;\n" + + "COMMIT TRANSACTION"; + SimpleQueryResult result = cluster.coordinator(1).executeWithResult(read, ConsistencyLevel.SERIAL); + assertThat(result).hasSize(1).contains(1, 10); + }); + } } diff --git a/test/unit/org/apache/cassandra/service/accord/txn/TxnConditionTest.java b/test/unit/org/apache/cassandra/service/accord/txn/TxnConditionTest.java index 287e921a0228..e327b4017019 100644 --- a/test/unit/org/apache/cassandra/service/accord/txn/TxnConditionTest.java +++ b/test/unit/org/apache/cassandra/service/accord/txn/TxnConditionTest.java @@ -68,6 +68,12 @@ import org.apache.cassandra.utils.Generators; import static accord.utils.Property.qt; +import static org.apache.cassandra.service.accord.txn.TxnCondition.Kind.EQUAL_REF; +import static org.apache.cassandra.service.accord.txn.TxnCondition.Kind.GREATER_THAN_OR_EQUAL_REF; +import static org.apache.cassandra.service.accord.txn.TxnCondition.Kind.GREATER_THAN_REF; +import static org.apache.cassandra.service.accord.txn.TxnCondition.Kind.LESS_THAN_OR_EQUAL_REF; +import static org.apache.cassandra.service.accord.txn.TxnCondition.Kind.LESS_THAN_REF; +import static org.apache.cassandra.service.accord.txn.TxnCondition.Kind.NOT_EQUAL_REF; import static org.apache.cassandra.utils.ByteBufferUtil.EMPTY_BYTE_BUFFER; import static org.apache.cassandra.utils.Generators.toGen; @@ -147,6 +153,9 @@ private static ColumnIdentifier name(ColumnMetadata.Kind kind, int offset) private static Gen VALUE_KIND_GEN = Gens.pick(TxnCondition.Kind.EQUAL, TxnCondition.Kind.NOT_EQUAL, TxnCondition.Kind.GREATER_THAN, TxnCondition.Kind.GREATER_THAN_OR_EQUAL, TxnCondition.Kind.LESS_THAN, TxnCondition.Kind.LESS_THAN_OR_EQUAL); + private static Gen VALUE_KIND_REF_GEN = Gens.pick(EQUAL_REF, NOT_EQUAL_REF, + TxnCondition.Kind.GREATER_THAN_REF, TxnCondition.Kind.GREATER_THAN_OR_EQUAL_REF, + TxnCondition.Kind.LESS_THAN_REF, TxnCondition.Kind.LESS_THAN_OR_EQUAL_REF); private static Gen PROTOCOL_VERSION_GEN = Gens.enums().all(ProtocolVersion.class); private static Gen COLUM_METADATA_GEN = toGen(CassandraGenerators.columnMetadataGen()).map(cm -> { SCHEMA.add(cm); @@ -471,6 +480,89 @@ public void value() }); } + @Test + public void reference() + { + Gen> typeGen = toGen(new AbstractTypeGenerators.TypeGenBuilder() + .withoutUnsafeEquality() + .build()); + qt().check(rs -> { + AbstractType type = typeGen.next(rs); + TableMetadata metadata = TableMetadata.builder("ks", "tbl") + .addPartitionKeyColumn("pk", type.freeze()) + .addClusteringColumn("ck", type.freeze()) + .addRegularColumn("r", type) + .addStaticColumn("s", type) + .partitioner(Murmur3Partitioner.instance) + .build(); + + ByteBuffer valueLHS = toGen(AbstractTypeGenerators.getTypeSupport(type).bytesGen()).next(rs); + List complexValueLHS = type.isMultiCell() ? split(type, valueLHS) : null; + Clustering clusteringLHS = BufferClustering.make(valueLHS); + SimplePartition partitionLHS = new SimplePartition(metadata, metadata.partitioner.decorateKey(valueLHS)); + + ByteBuffer valueRHS = toGen(AbstractTypeGenerators.getTypeSupport(type).bytesGen()).next(rs); + List complexValueRHS = type.isMultiCell() ? split(type, valueRHS) : null; + Clustering clusteringRHS = BufferClustering.make(valueRHS); + SimplePartition partitionRHS = new SimplePartition(metadata, metadata.partitioner.decorateKey(valueRHS)); + for (TxnCondition.Kind kind : TxnCondition.Value.supported()) + { + for (ProtocolVersion version : ProtocolVersion.SUPPORTED) + { + for (ColumnMetadata column : metadata.columns()) + { + TxnReference refLHS = TxnReference.column(0, metadata, column); + TxnReference refRHS = TxnReference.column(1, metadata, column); + + TxnCondition.Value value = new TxnCondition.Value(refLHS.asColumn(), kind, valueRHS, version); + TxnCondition.Reference condition = new TxnCondition.Reference(refLHS.asColumn(), convertKindToReferenceKind(kind), refRHS.asColumn(), version); + + partitionLHS.clear().addEmptyAndLive(clusteringLHS); + partitionRHS.clear().addEmptyAndLive(clusteringRHS); + + TxnData data = TxnData.of(0, new TxnDataKeyValue(partitionLHS.filtered())).merge(TxnData.of(1, new TxnDataKeyValue(partitionRHS.filtered()))); + + Assertions.assertThat(condition.applies(data)) + .describedAs("column=%s, type=%s, kind=%s", column.name, type.asCQL3Type(), kind.name()) + .isEqualTo(value.applies(data)); + + if (column.isPrimaryKeyColumn()) continue; + + // with value + if (type.isMultiCell()) + { + partitionLHS.clear() + .add(column.isStatic() ? Clustering.STATIC_CLUSTERING : clusteringLHS) + .addComplex(column, complexValueLHS) + .build(); + + partitionRHS.clear() + .add(column.isStatic() ? Clustering.STATIC_CLUSTERING : clusteringRHS) + .addComplex(column, complexValueRHS) + .build(); + } + else + { + partitionLHS.clear() + .add(column.isStatic() ? Clustering.STATIC_CLUSTERING : clusteringLHS) + .add(column, valueLHS) + .build(); + + partitionRHS.clear() + .add(column.isStatic() ? Clustering.STATIC_CLUSTERING : clusteringRHS) + .add(column, valueRHS) + .build(); + } + + Assertions.assertThat(condition.applies(data)) + .describedAs("column=%s, type=%s, kind=%s", column.name, type.asCQL3Type(), kind.name()) + .isEqualTo(value.applies(data)); + } + } + } + }); + } + private static List split(AbstractType type, ByteBuffer value) { type = type.unwrap(); @@ -494,18 +586,33 @@ private static void assertExists(TxnData data, TxnReference ref, boolean exists) private Gen txnConditionGen() { return rs -> { - switch (rs.nextInt(1, 5)) + switch (rs.nextInt(1, 6)) { case 0: return TxnCondition.none(); case 1: return new TxnCondition.Exists(TXN_REF_GEN.next(rs), EXISTS_KIND_GEN.next(rs)); case 2: return new TxnCondition.Value(TXN_REF_GEN.next(rs).asColumn(), VALUE_KIND_GEN.next(rs), BYTES_GEN.next(rs), PROTOCOL_VERSION_GEN.next(rs)); - case 3: return new TxnCondition.ColumnConditionsAdapter(CLUSTERING_GEN.next(rs), Gens.lists(BOUND_GEN).ofSizeBetween(0, 3).next(rs)); - case 4: return new TxnCondition.BooleanGroup(BOOLEAN_KIND_GEN.next(rs), Gens.lists(txnConditionGen()).ofSizeBetween(0, 3).next(rs)); + case 3: return new TxnCondition.Reference(TXN_REF_GEN.next(rs).asColumn(), VALUE_KIND_REF_GEN.next(rs), TXN_REF_GEN.next(rs).asColumn(), PROTOCOL_VERSION_GEN.next(rs)); + case 4: return new TxnCondition.ColumnConditionsAdapter(CLUSTERING_GEN.next(rs), Gens.lists(BOUND_GEN).ofSizeBetween(0, 3).next(rs)); + case 5: return new TxnCondition.BooleanGroup(BOOLEAN_KIND_GEN.next(rs), Gens.lists(txnConditionGen()).ofSizeBetween(0, 3).next(rs)); default: throw new AssertionError(); } }; } + private TxnCondition.Kind convertKindToReferenceKind(TxnCondition.Kind kind) + { + switch (kind) + { + case EQUAL: return EQUAL_REF; + case NOT_EQUAL: return NOT_EQUAL_REF; + case GREATER_THAN: return GREATER_THAN_REF; + case GREATER_THAN_OR_EQUAL: return GREATER_THAN_OR_EQUAL_REF; + case LESS_THAN: return LESS_THAN_REF; + case LESS_THAN_OR_EQUAL: return LESS_THAN_OR_EQUAL_REF; + default: throw new UnsupportedOperationException(kind.name()); + } + } + private interface IsNullTest // jdk16+ lets this be in-lined with the test method rather than be here { void test(SimplePartition partition, Clustering clustering, ColumnMetadata column, ByteBuffer nonNullValue); From 50cb0dcea3b0e5d1965362aeb88096552fd41426 Mon Sep 17 00:00:00 2001 From: Alan Wang Date: Fri, 12 Jun 2026 13:46:17 -0700 Subject: [PATCH 03/16] removed test --- .../test/accord/AccordCQLTestBase.java | 30 ------------------- 1 file changed, 30 deletions(-) diff --git a/test/distributed/org/apache/cassandra/distributed/test/accord/AccordCQLTestBase.java b/test/distributed/org/apache/cassandra/distributed/test/accord/AccordCQLTestBase.java index f77d31ebb875..e1af11631331 100644 --- a/test/distributed/org/apache/cassandra/distributed/test/accord/AccordCQLTestBase.java +++ b/test/distributed/org/apache/cassandra/distributed/test/accord/AccordCQLTestBase.java @@ -3633,34 +3633,4 @@ public void testLetComparisonTransactionStatement() throws Throwable assertThat(result).hasSize(1).contains(1, 10); }); } - - @Test - public void testLetComparisionWithDifferingTypes() throws Throwable - { - - test("CREATE TABLE " + qualifiedAccordTableName + " (k int PRIMARY KEY, v int) WITH " + transactionalMode.asCqlParam(), cluster -> { - String insert = "BEGIN TRANSACTION\n" + - "INSERT INTO " + qualifiedAccordTableName + " (k, v) VALUES (1, 2);\n" + - "INSERT INTO " + qualifiedAccordTableName + " (k, v) VALUES (2, 3);\n" + - "COMMIT TRANSACTION"; - - cluster.coordinator(1).executeWithResult(insert, ConsistencyLevel.SERIAL); - - String query = "BEGIN TRANSACTION\n" + - "LET k1 = (SELECT v FROM " + qualifiedAccordTableName + " WHERE k = 1);\n" + - "LET k2 = (SELECT v FROM " + qualifiedAccordTableName + " WHERE k = 2);\n" + - "IF k1.v < k2.v THEN \n" + - " UPDATE " + qualifiedAccordTableName + " SET v = 10 WHERE k = 1;\n" + - "END IF\n" + - "COMMIT TRANSACTION"; - - cluster.coordinator(1).executeWithResult(query, ConsistencyLevel.SERIAL); - - String read = "BEGIN TRANSACTION\n" + - "SELECT * FROM " + qualifiedAccordTableName + " WHERE k = 1;\n" + - "COMMIT TRANSACTION"; - SimpleQueryResult result = cluster.coordinator(1).executeWithResult(read, ConsistencyLevel.SERIAL); - assertThat(result).hasSize(1).contains(1, 10); - }); - } } From b0a90334b6c71b323b93a109cc0ffcc1dc464f9a Mon Sep 17 00:00:00 2001 From: Alan Wang Date: Fri, 12 Jun 2026 13:54:32 -0700 Subject: [PATCH 04/16] add missing line --- .../apache/cassandra/service/accord/txn/TxnConditionTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/unit/org/apache/cassandra/service/accord/txn/TxnConditionTest.java b/test/unit/org/apache/cassandra/service/accord/txn/TxnConditionTest.java index e327b4017019..dc5f0ae134c4 100644 --- a/test/unit/org/apache/cassandra/service/accord/txn/TxnConditionTest.java +++ b/test/unit/org/apache/cassandra/service/accord/txn/TxnConditionTest.java @@ -554,6 +554,8 @@ public void reference() .build(); } + data = TxnData.of(0, new TxnDataKeyValue(partitionLHS.filtered())).merge(TxnData.of(1, new TxnDataKeyValue(partitionRHS.filtered()))); + Assertions.assertThat(condition.applies(data)) .describedAs("column=%s, type=%s, kind=%s", column.name, type.asCQL3Type(), kind.name()) .isEqualTo(value.applies(data)); From 8672edf2308a010697ba472685965a08ad52bc83 Mon Sep 17 00:00:00 2001 From: Alan Wang Date: Fri, 12 Jun 2026 14:12:24 -0700 Subject: [PATCH 05/16] update --- .../distributed/test/accord/AccordCQLTestBase.java | 1 + .../cassandra/service/accord/txn/TxnConditionTest.java | 10 ++++++---- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/test/distributed/org/apache/cassandra/distributed/test/accord/AccordCQLTestBase.java b/test/distributed/org/apache/cassandra/distributed/test/accord/AccordCQLTestBase.java index e1af11631331..70a480615eaa 100644 --- a/test/distributed/org/apache/cassandra/distributed/test/accord/AccordCQLTestBase.java +++ b/test/distributed/org/apache/cassandra/distributed/test/accord/AccordCQLTestBase.java @@ -3629,6 +3629,7 @@ public void testLetComparisonTransactionStatement() throws Throwable String read = "BEGIN TRANSACTION\n" + "SELECT * FROM " + qualifiedAccordTableName + " WHERE k = 1;\n" + "COMMIT TRANSACTION"; + SimpleQueryResult result = cluster.coordinator(1).executeWithResult(read, ConsistencyLevel.SERIAL); assertThat(result).hasSize(1).contains(1, 10); }); diff --git a/test/unit/org/apache/cassandra/service/accord/txn/TxnConditionTest.java b/test/unit/org/apache/cassandra/service/accord/txn/TxnConditionTest.java index dc5f0ae134c4..156c6a503dd0 100644 --- a/test/unit/org/apache/cassandra/service/accord/txn/TxnConditionTest.java +++ b/test/unit/org/apache/cassandra/service/accord/txn/TxnConditionTest.java @@ -520,11 +520,12 @@ public void reference() partitionLHS.clear().addEmptyAndLive(clusteringLHS); partitionRHS.clear().addEmptyAndLive(clusteringRHS); - TxnData data = TxnData.of(0, new TxnDataKeyValue(partitionLHS.filtered())).merge(TxnData.of(1, new TxnDataKeyValue(partitionRHS.filtered()))); + TxnData dataLHS = TxnData.of(0, new TxnDataKeyValue(partitionLHS.filtered())); + TxnData data = dataLHS.merge(TxnData.of(1, new TxnDataKeyValue(partitionRHS.filtered()))); Assertions.assertThat(condition.applies(data)) .describedAs("column=%s, type=%s, kind=%s", column.name, type.asCQL3Type(), kind.name()) - .isEqualTo(value.applies(data)); + .isEqualTo(value.applies(dataLHS)); if (column.isPrimaryKeyColumn()) continue; @@ -554,11 +555,12 @@ public void reference() .build(); } - data = TxnData.of(0, new TxnDataKeyValue(partitionLHS.filtered())).merge(TxnData.of(1, new TxnDataKeyValue(partitionRHS.filtered()))); + dataLHS = TxnData.of(0, new TxnDataKeyValue(partitionLHS.filtered())); + data = dataLHS.merge(TxnData.of(1, new TxnDataKeyValue(partitionRHS.filtered()))); Assertions.assertThat(condition.applies(data)) .describedAs("column=%s, type=%s, kind=%s", column.name, type.asCQL3Type(), kind.name()) - .isEqualTo(value.applies(data)); + .isEqualTo(value.applies(dataLHS)); } } } From f34462be26f355690afe7a0c94f9a60844086fe5 Mon Sep 17 00:00:00 2001 From: Alan Wang Date: Fri, 12 Jun 2026 15:27:40 -0700 Subject: [PATCH 06/16] add additional checks to prepare --- .../cassandra/cql3/transactions/ConditionStatement.java | 6 ++++++ .../apache/cassandra/service/accord/txn/TxnCondition.java | 2 -- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/java/org/apache/cassandra/cql3/transactions/ConditionStatement.java b/src/java/org/apache/cassandra/cql3/transactions/ConditionStatement.java index d051353ba510..d4e490711e44 100644 --- a/src/java/org/apache/cassandra/cql3/transactions/ConditionStatement.java +++ b/src/java/org/apache/cassandra/cql3/transactions/ConditionStatement.java @@ -109,6 +109,12 @@ public ConditionStatement prepare(String keyspace, VariableSpecifications bindVa if (lhs instanceof RowDataReference.Raw && rhs instanceof RowDataReference.Raw) { + if (((RowDataReference.Raw) lhs).column() == null) + throw new IllegalStateException(String.format("Row reference (%s) can only be used with IS NULL/IS NOT NULL conditions", lhs.getText())); + if (((RowDataReference.Raw) rhs).column() == null) + throw new IllegalStateException(String.format("Row reference (%s) can only be used with IS NULL/IS NOT NULL conditions", rhs.getText())); + if (!((RowDataReference.Raw) lhs).column().type.equals(((RowDataReference.Raw) rhs).column().type)) + throw new IllegalStateException(String.format("Row reference (%s) must have the same type as row reference (%s)", lhs.getText(), rhs.getText())); reference = ((RowDataReference.Raw) lhs).prepareAsReceiver(); value = ((RowDataReference.Raw) rhs).prepareAsReceiver(); } diff --git a/src/java/org/apache/cassandra/service/accord/txn/TxnCondition.java b/src/java/org/apache/cassandra/service/accord/txn/TxnCondition.java index 93df1ec5063d..7accf3b9895c 100644 --- a/src/java/org/apache/cassandra/service/accord/txn/TxnCondition.java +++ b/src/java/org/apache/cassandra/service/accord/txn/TxnCondition.java @@ -693,8 +693,6 @@ public boolean applies(TxnData data) ColumnMetadata columnLHS = referenceLHS.column(); ColumnMetadata columnRHS = referenceRHS.column(); - Preconditions.checkArgument(columnLHS.type.equals(columnRHS.type), columnLHS.type + " != " + columnRHS.type); - ByteBuffer lhs = referenceLHS.toByteBuffer(data, columnLHS.type); ByteBuffer rhs = referenceRHS.toByteBuffer(data, columnRHS.type); From 1008bf29d01c5d74d2c97d06e5a373b5652bc504 Mon Sep 17 00:00:00 2001 From: Alan Wang Date: Fri, 12 Jun 2026 15:43:27 -0700 Subject: [PATCH 07/16] remove unused param --- .../service/accord/txn/TxnCondition.java | 44 +++++++++---------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/src/java/org/apache/cassandra/service/accord/txn/TxnCondition.java b/src/java/org/apache/cassandra/service/accord/txn/TxnCondition.java index 7accf3b9895c..00be3878aec5 100644 --- a/src/java/org/apache/cassandra/service/accord/txn/TxnCondition.java +++ b/src/java/org/apache/cassandra/service/accord/txn/TxnCondition.java @@ -104,39 +104,39 @@ private interface ConditionSerializer long serializedSize(T condition, TableMetadatas tables); } + // For enums with a REF suffixed, we are performing a comparison of two LET variables + // otherwise EQUAL, NOT_EQUAL, GREATER_THAN, GREATER_THAN_OR_EQUAL, LESS_THAN, LESS_THAN_OR_EQUAL + // are used for comparisions between a LET variable and a value public enum Kind { - NONE("n/a", null, null), - AND("AND", null, null), - OR("OR", null, null), - IS_NOT_NULL("IS NOT NULL", null, null), - IS_NULL("IS NULL", null, null), - EQUAL("=", Operator.EQ, false), - NOT_EQUAL("!=", Operator.NEQ, false), - GREATER_THAN(">", Operator.GT, false), - GREATER_THAN_OR_EQUAL(">=", Operator.GTE, false), - LESS_THAN("<", Operator.LT, false), - LESS_THAN_OR_EQUAL("<=", Operator.LTE, false), - COLUMN_CONDITIONS("COLUMN_CONDITIONS", null, false), - EQUAL_REF("=", Operator.EQ, true), - NOT_EQUAL_REF("!=", Operator.NEQ, true), - GREATER_THAN_REF(">", Operator.GT, true), - GREATER_THAN_OR_EQUAL_REF(">=", Operator.GTE, true), - LESS_THAN_REF("<", Operator.LT, true), - LESS_THAN_OR_EQUAL_REF("<=", Operator.LTE, true); + NONE("n/a", null), + AND("AND", null), + OR("OR", null), + IS_NOT_NULL("IS NOT NULL", null), + IS_NULL("IS NULL", null), + EQUAL("=", Operator.EQ), + NOT_EQUAL("!=", Operator.NEQ), + GREATER_THAN(">", Operator.GT), + GREATER_THAN_OR_EQUAL(">=", Operator.GTE), + LESS_THAN("<", Operator.LT), + LESS_THAN_OR_EQUAL("<=", Operator.LTE), + COLUMN_CONDITIONS("COLUMN_CONDITIONS", null), + EQUAL_REF("=", Operator.EQ), + NOT_EQUAL_REF("!=", Operator.NEQ), + GREATER_THAN_REF(">", Operator.GT), + GREATER_THAN_OR_EQUAL_REF(">=", Operator.GTE), + LESS_THAN_REF("<", Operator.LT), + LESS_THAN_OR_EQUAL_REF("<=", Operator.LTE); @Nonnull private final String symbol; @Nullable private final Operator operator; - @Nullable - private final Boolean isReference; - Kind(String symbol, Operator operator, Boolean isReference) + Kind(String symbol, Operator operator) { this.symbol = symbol; this.operator = operator; - this.isReference = isReference; } @SuppressWarnings("rawtypes") From 0982a8695a7461e74fdb469d57fb2f01de5055a7 Mon Sep 17 00:00:00 2001 From: Alan Wang Date: Mon, 15 Jun 2026 12:30:25 -0700 Subject: [PATCH 08/16] remove duplication of kind --- src/antlr/Parser.g | 11 +-- .../cql3/transactions/ConditionStatement.java | 35 +++------- .../service/accord/txn/TxnCondition.java | 69 ++++++++++++------- .../service/accord/txn/TxnConditionTest.java | 28 +------- 4 files changed, 57 insertions(+), 86 deletions(-) diff --git a/src/antlr/Parser.g b/src/antlr/Parser.g index f6ded99a9045..5cffb86e7476 100644 --- a/src/antlr/Parser.g +++ b/src/antlr/Parser.g @@ -836,15 +836,6 @@ txnConditionKind returns [ConditionStatement.Kind op] | '!=' { $op = ConditionStatement.Kind.NEQ; } ; -txnConditionKindRef returns [ConditionStatement.Kind op] - : '=' { $op = ConditionStatement.Kind.EQ_REF; } - | '<' { $op = ConditionStatement.Kind.LT_REF; } - | '<=' { $op = ConditionStatement.Kind.LTE_REF; } - | '>' { $op = ConditionStatement.Kind.GT_REF; } - | '>=' { $op = ConditionStatement.Kind.GTE_REF; } - | '!=' { $op = ConditionStatement.Kind.NEQ_REF; } - ; - txnColumnCondition[List conditions] : lhs=rowDataReference ( @@ -854,7 +845,7 @@ txnColumnCondition[List conditions] | K_NULL { conditions.add(new ConditionStatement.Raw(lhs, ConditionStatement.Kind.IS_NULL, null)); } ) | (txnConditionKind term)=> op=txnConditionKind t=term { conditions.add(new ConditionStatement.Raw(lhs, op, t)); } - | (txnConditionKindRef rowDataReference)=> op=txnConditionKindRef rhs=rowDataReference { conditions.add(new ConditionStatement.Raw(lhs, op, rhs)); } + | (txnConditionKind rowDataReference)=> op=txnConditionKind rhs=rowDataReference { conditions.add(new ConditionStatement.Raw(lhs, op, rhs)); } ) | lhs=term op=txnConditionKind rhs=rowDataReference { conditions.add(new ConditionStatement.Raw(lhs, op, rhs)); } ; diff --git a/src/java/org/apache/cassandra/cql3/transactions/ConditionStatement.java b/src/java/org/apache/cassandra/cql3/transactions/ConditionStatement.java index d4e490711e44..cc3ca6507ba8 100644 --- a/src/java/org/apache/cassandra/cql3/transactions/ConditionStatement.java +++ b/src/java/org/apache/cassandra/cql3/transactions/ConditionStatement.java @@ -40,13 +40,7 @@ public enum Kind GT(TxnCondition.Kind.GREATER_THAN, TxnCondition.Kind.LESS_THAN), GTE(TxnCondition.Kind.GREATER_THAN_OR_EQUAL, TxnCondition.Kind.LESS_THAN_OR_EQUAL), LT(TxnCondition.Kind.LESS_THAN, TxnCondition.Kind.GREATER_THAN), - LTE(TxnCondition.Kind.LESS_THAN_OR_EQUAL, TxnCondition.Kind.GREATER_THAN_OR_EQUAL), - EQ_REF(TxnCondition.Kind.EQUAL_REF, TxnCondition.Kind.EQUAL_REF), - NEQ_REF(TxnCondition.Kind.NOT_EQUAL_REF, TxnCondition.Kind.NOT_EQUAL_REF), - GT_REF(TxnCondition.Kind.GREATER_THAN_REF, TxnCondition.Kind.LESS_THAN_REF), - GTE_REF(TxnCondition.Kind.GREATER_THAN_OR_EQUAL_REF, TxnCondition.Kind.LESS_THAN_OR_EQUAL_REF), - LT_REF(TxnCondition.Kind.LESS_THAN_REF, TxnCondition.Kind.GREATER_THAN_REF), - LTE_REF(TxnCondition.Kind.LESS_THAN_OR_EQUAL_REF, TxnCondition.Kind.GREATER_THAN_OR_EQUAL_REF); + LTE(TxnCondition.Kind.LESS_THAN_OR_EQUAL, TxnCondition.Kind.GREATER_THAN_OR_EQUAL); // TODO: Support for IN, CONTAINS, CONTAINS KEY @@ -160,31 +154,22 @@ public TxnCondition createCondition(QueryOptions options) case GTE: case LT: case LTE: - { TxnReference refLHS = reference.toTxnReference(options); checkTrue(refLHS.kind == TxnReference.Kind.COLUMN, "Condition %s requires COLUMN reference but given %s", kind, refLHS.kind); + if (value instanceof RowDataReference) + { + TxnReference refRHS = ((RowDataReference) value).toTxnReference(options); + checkTrue(refRHS.kind == TxnReference.Kind.COLUMN, "Condition %s requires COLUMN reference but given %s", kind, refRHS.kind); + return new TxnCondition.Reference(refLHS.asColumn(), + kind.toTxnKind(reversed), + refRHS.asColumn(), + options.getProtocolVersion()); + } return new TxnCondition.Value(refLHS.asColumn(), kind.toTxnKind(reversed), value.bindAndGet(options), options.getProtocolVersion()); - } - case EQ_REF: - case NEQ_REF: - case GT_REF: - case GTE_REF: - case LT_REF: - case LTE_REF: - { - TxnReference refLHS = reference.toTxnReference(options); - checkTrue(refLHS.kind == TxnReference.Kind.COLUMN, "Condition %s requires COLUMN reference but given %s", kind, refLHS.kind); - TxnReference refRHS = ((RowDataReference) value).toTxnReference(options); - checkTrue(refRHS.kind == TxnReference.Kind.COLUMN, "Condition %s requires COLUMN reference but given %s", kind, refRHS.kind); - return new TxnCondition.Reference(refLHS.asColumn(), - kind.toTxnKind(reversed), - refRHS.asColumn(), - options.getProtocolVersion()); - } default: throw new IllegalStateException(); } diff --git a/src/java/org/apache/cassandra/service/accord/txn/TxnCondition.java b/src/java/org/apache/cassandra/service/accord/txn/TxnCondition.java index 00be3878aec5..8ba8b0856b00 100644 --- a/src/java/org/apache/cassandra/service/accord/txn/TxnCondition.java +++ b/src/java/org/apache/cassandra/service/accord/txn/TxnCondition.java @@ -104,9 +104,6 @@ private interface ConditionSerializer long serializedSize(T condition, TableMetadatas tables); } - // For enums with a REF suffixed, we are performing a comparison of two LET variables - // otherwise EQUAL, NOT_EQUAL, GREATER_THAN, GREATER_THAN_OR_EQUAL, LESS_THAN, LESS_THAN_OR_EQUAL - // are used for comparisions between a LET variable and a value public enum Kind { NONE("n/a", null), @@ -120,13 +117,7 @@ public enum Kind GREATER_THAN_OR_EQUAL(">=", Operator.GTE), LESS_THAN("<", Operator.LT), LESS_THAN_OR_EQUAL("<=", Operator.LTE), - COLUMN_CONDITIONS("COLUMN_CONDITIONS", null), - EQUAL_REF("=", Operator.EQ), - NOT_EQUAL_REF("!=", Operator.NEQ), - GREATER_THAN_REF(">", Operator.GT), - GREATER_THAN_OR_EQUAL_REF(">=", Operator.GTE), - LESS_THAN_REF("<", Operator.LT), - LESS_THAN_OR_EQUAL_REF("<=", Operator.LTE); + COLUMN_CONDITIONS("COLUMN_CONDITIONS", null); @Nonnull private final String symbol; @@ -154,13 +145,6 @@ private ConditionSerializer serializer() case GREATER_THAN: case GREATER_THAN_OR_EQUAL: return Value.serializer; - case EQUAL_REF: - case NOT_EQUAL_REF: - case LESS_THAN_REF: - case LESS_THAN_OR_EQUAL_REF: - case GREATER_THAN_REF: - case GREATER_THAN_OR_EQUAL_REF: - return Reference.serializer; case AND: case OR: return BooleanGroup.serializer; @@ -636,9 +620,9 @@ public long serializedSize(Value condition, TableMetadatas tables) public static class Reference extends TxnCondition { - private static final EnumSet KINDS = EnumSet.of(Kind.EQUAL_REF, Kind.NOT_EQUAL_REF, - Kind.GREATER_THAN_REF, Kind.GREATER_THAN_OR_EQUAL_REF, - Kind.LESS_THAN_REF, Kind.LESS_THAN_OR_EQUAL_REF); + private static final EnumSet KINDS = EnumSet.of(Kind.EQUAL, Kind.NOT_EQUAL, + Kind.GREATER_THAN, Kind.GREATER_THAN_OR_EQUAL, + Kind.LESS_THAN, Kind.LESS_THAN_OR_EQUAL); private final TxnReference.ColumnReference referenceLHS; private final TxnReference.ColumnReference referenceRHS; @@ -813,27 +797,60 @@ public long serializedSize(BooleanGroup condition, TableMetadatas tables) public static final ParameterisedUnversionedSerializer serializer = new ParameterisedUnversionedSerializer<>() { + // TOP_BIT is used to differentiate between Value.Serializer and Reference.Serialzer. + // This is so done to preserve upgrade compatibility with the prior serializer. + // Nodes that are not yet upgraded can still deserialize all values modulo those that are + // of Reference type and upgraded nodes can deserialize all values from older nodes. + // See CASSANDRA-21458 + private static final int TOP_BIT = 0x40000000; + @SuppressWarnings("unchecked") @Override public void serialize(TxnCondition condition, TableMetadatas tables, DataOutputPlus out) throws IOException { - out.writeUnsignedVInt32(condition.kind.ordinal()); - condition.kind.serializer().serialize(condition, tables, out); + if (condition instanceof Reference) + { + out.writeUnsignedVInt32(condition.kind.ordinal() | TOP_BIT); + Reference.serializer.serialize((Reference) condition, tables, out); + } + else + { + out.writeUnsignedVInt32(condition.kind.ordinal()); + condition.kind.serializer().serialize(condition, tables, out); + } } @Override public TxnCondition deserialize(TableMetadatas tables, DataInputPlus in) throws IOException { - Kind kind = Kind.values()[in.readUnsignedVInt32()]; - return kind.serializer().deserialize(tables, in, kind); + int flag = in.readUnsignedVInt32(); + if ((flag & TOP_BIT) != 0) + { + Kind kind = Kind.values()[flag ^ TOP_BIT]; + return Reference.serializer.deserialize(tables, in, kind); + } + else + { + Kind kind = Kind.values()[flag]; + return kind.serializer().deserialize(tables, in, kind); + } } @SuppressWarnings("unchecked") @Override public long serializedSize(TxnCondition condition, TableMetadatas tables) { - long size = TypeSizes.sizeofUnsignedVInt(condition.kind.ordinal()); - size += condition.kind.serializer().serializedSize(condition, tables); + long size; + if (condition instanceof Reference) + { + size = TypeSizes.sizeofUnsignedVInt(condition.kind.ordinal() | TOP_BIT); + size += Reference.serializer.serializedSize((Reference) condition, tables); + } + else + { + size = TypeSizes.sizeofUnsignedVInt(condition.kind.ordinal()); + size += condition.kind.serializer().serializedSize(condition, tables); + } return size; } }; diff --git a/test/unit/org/apache/cassandra/service/accord/txn/TxnConditionTest.java b/test/unit/org/apache/cassandra/service/accord/txn/TxnConditionTest.java index 156c6a503dd0..d64d3813ce07 100644 --- a/test/unit/org/apache/cassandra/service/accord/txn/TxnConditionTest.java +++ b/test/unit/org/apache/cassandra/service/accord/txn/TxnConditionTest.java @@ -68,12 +68,6 @@ import org.apache.cassandra.utils.Generators; import static accord.utils.Property.qt; -import static org.apache.cassandra.service.accord.txn.TxnCondition.Kind.EQUAL_REF; -import static org.apache.cassandra.service.accord.txn.TxnCondition.Kind.GREATER_THAN_OR_EQUAL_REF; -import static org.apache.cassandra.service.accord.txn.TxnCondition.Kind.GREATER_THAN_REF; -import static org.apache.cassandra.service.accord.txn.TxnCondition.Kind.LESS_THAN_OR_EQUAL_REF; -import static org.apache.cassandra.service.accord.txn.TxnCondition.Kind.LESS_THAN_REF; -import static org.apache.cassandra.service.accord.txn.TxnCondition.Kind.NOT_EQUAL_REF; import static org.apache.cassandra.utils.ByteBufferUtil.EMPTY_BYTE_BUFFER; import static org.apache.cassandra.utils.Generators.toGen; @@ -153,9 +147,6 @@ private static ColumnIdentifier name(ColumnMetadata.Kind kind, int offset) private static Gen VALUE_KIND_GEN = Gens.pick(TxnCondition.Kind.EQUAL, TxnCondition.Kind.NOT_EQUAL, TxnCondition.Kind.GREATER_THAN, TxnCondition.Kind.GREATER_THAN_OR_EQUAL, TxnCondition.Kind.LESS_THAN, TxnCondition.Kind.LESS_THAN_OR_EQUAL); - private static Gen VALUE_KIND_REF_GEN = Gens.pick(EQUAL_REF, NOT_EQUAL_REF, - TxnCondition.Kind.GREATER_THAN_REF, TxnCondition.Kind.GREATER_THAN_OR_EQUAL_REF, - TxnCondition.Kind.LESS_THAN_REF, TxnCondition.Kind.LESS_THAN_OR_EQUAL_REF); private static Gen PROTOCOL_VERSION_GEN = Gens.enums().all(ProtocolVersion.class); private static Gen COLUM_METADATA_GEN = toGen(CassandraGenerators.columnMetadataGen()).map(cm -> { SCHEMA.add(cm); @@ -505,6 +496,7 @@ public void reference() List complexValueRHS = type.isMultiCell() ? split(type, valueRHS) : null; Clustering clusteringRHS = BufferClustering.make(valueRHS); SimplePartition partitionRHS = new SimplePartition(metadata, metadata.partitioner.decorateKey(valueRHS)); + for (TxnCondition.Kind kind : TxnCondition.Value.supported()) { for (ProtocolVersion version : ProtocolVersion.SUPPORTED) @@ -515,7 +507,7 @@ public void reference() TxnReference refRHS = TxnReference.column(1, metadata, column); TxnCondition.Value value = new TxnCondition.Value(refLHS.asColumn(), kind, valueRHS, version); - TxnCondition.Reference condition = new TxnCondition.Reference(refLHS.asColumn(), convertKindToReferenceKind(kind), refRHS.asColumn(), version); + TxnCondition.Reference condition = new TxnCondition.Reference(refLHS.asColumn(), kind, refRHS.asColumn(), version); partitionLHS.clear().addEmptyAndLive(clusteringLHS); partitionRHS.clear().addEmptyAndLive(clusteringRHS); @@ -595,7 +587,7 @@ private Gen txnConditionGen() case 0: return TxnCondition.none(); case 1: return new TxnCondition.Exists(TXN_REF_GEN.next(rs), EXISTS_KIND_GEN.next(rs)); case 2: return new TxnCondition.Value(TXN_REF_GEN.next(rs).asColumn(), VALUE_KIND_GEN.next(rs), BYTES_GEN.next(rs), PROTOCOL_VERSION_GEN.next(rs)); - case 3: return new TxnCondition.Reference(TXN_REF_GEN.next(rs).asColumn(), VALUE_KIND_REF_GEN.next(rs), TXN_REF_GEN.next(rs).asColumn(), PROTOCOL_VERSION_GEN.next(rs)); + case 3: return new TxnCondition.Reference(TXN_REF_GEN.next(rs).asColumn(), VALUE_KIND_GEN.next(rs), TXN_REF_GEN.next(rs).asColumn(), PROTOCOL_VERSION_GEN.next(rs)); case 4: return new TxnCondition.ColumnConditionsAdapter(CLUSTERING_GEN.next(rs), Gens.lists(BOUND_GEN).ofSizeBetween(0, 3).next(rs)); case 5: return new TxnCondition.BooleanGroup(BOOLEAN_KIND_GEN.next(rs), Gens.lists(txnConditionGen()).ofSizeBetween(0, 3).next(rs)); default: throw new AssertionError(); @@ -603,20 +595,6 @@ private Gen txnConditionGen() }; } - private TxnCondition.Kind convertKindToReferenceKind(TxnCondition.Kind kind) - { - switch (kind) - { - case EQUAL: return EQUAL_REF; - case NOT_EQUAL: return NOT_EQUAL_REF; - case GREATER_THAN: return GREATER_THAN_REF; - case GREATER_THAN_OR_EQUAL: return GREATER_THAN_OR_EQUAL_REF; - case LESS_THAN: return LESS_THAN_REF; - case LESS_THAN_OR_EQUAL: return LESS_THAN_OR_EQUAL_REF; - default: throw new UnsupportedOperationException(kind.name()); - } - } - private interface IsNullTest // jdk16+ lets this be in-lined with the test method rather than be here { void test(SimplePartition partition, Clustering clustering, ColumnMetadata column, ByteBuffer nonNullValue); From f4f11c4c1360fc9e6910a42dd05fa73e3e19afe5 Mon Sep 17 00:00:00 2001 From: Alan Wang Date: Mon, 15 Jun 2026 16:49:29 -0700 Subject: [PATCH 09/16] update comment --- .../cassandra/service/accord/txn/TxnCondition.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/java/org/apache/cassandra/service/accord/txn/TxnCondition.java b/src/java/org/apache/cassandra/service/accord/txn/TxnCondition.java index 8ba8b0856b00..34c2992f771c 100644 --- a/src/java/org/apache/cassandra/service/accord/txn/TxnCondition.java +++ b/src/java/org/apache/cassandra/service/accord/txn/TxnCondition.java @@ -797,10 +797,14 @@ public long serializedSize(BooleanGroup condition, TableMetadatas tables) public static final ParameterisedUnversionedSerializer serializer = new ParameterisedUnversionedSerializer<>() { - // TOP_BIT is used to differentiate between Value.Serializer and Reference.Serialzer. - // This is so done to preserve upgrade compatibility with the prior serializer. - // Nodes that are not yet upgraded can still deserialize all values modulo those that are - // of Reference type and upgraded nodes can deserialize all values from older nodes. + // TOP_BIT is used to differentiate between Value.Serializer and Reference.Serialzer, + // in order to implement comparison between LET variables. + // The reason we use TOP_BIT is to support users who have been deploying off trunk + // to upgrade nodes without breaking them. Upgrading is safe under the following assumptions: + // 1) `ref op ref` feature is only used after all nodes have been upgraded + // 2) cluster can be mixed mode as long as `ref op ref` is not used + // If a user tries to use `ref op ref` in a mixed mode this will lead to undefined errors, + // where the only recovery process is to force older nodes to upgrade // See CASSANDRA-21458 private static final int TOP_BIT = 0x40000000; From 257a6b1ee93721547dabc4f08bbbd54a17feec64 Mon Sep 17 00:00:00 2001 From: Alan Wang Date: Tue, 16 Jun 2026 10:38:16 -0700 Subject: [PATCH 10/16] updated tests --- .../test/accord/AccordCQLTestBase.java | 2 +- .../apache/cassandra/utils/ASTGenerators.java | 30 ++++++++++++------- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/test/distributed/org/apache/cassandra/distributed/test/accord/AccordCQLTestBase.java b/test/distributed/org/apache/cassandra/distributed/test/accord/AccordCQLTestBase.java index 70a480615eaa..f9fb4a2758d6 100644 --- a/test/distributed/org/apache/cassandra/distributed/test/accord/AccordCQLTestBase.java +++ b/test/distributed/org/apache/cassandra/distributed/test/accord/AccordCQLTestBase.java @@ -3619,7 +3619,7 @@ public void testLetComparisonTransactionStatement() throws Throwable String query = "BEGIN TRANSACTION\n" + "LET k1 = (SELECT v FROM " + qualifiedAccordTableName + " WHERE k = 1);\n" + "LET k2 = (SELECT v FROM " + qualifiedAccordTableName + " WHERE k = 2);\n" + - "IF k1.v < k2.v THEN \n" + + "IF k1.v IS NOT NULL AND k1.v < k2.v THEN \n" + " UPDATE " + qualifiedAccordTableName + " SET v = 10 WHERE k = 1;\n" + "END IF\n" + "COMMIT TRANSACTION"; diff --git a/test/unit/org/apache/cassandra/utils/ASTGenerators.java b/test/unit/org/apache/cassandra/utils/ASTGenerators.java index 31df22f11ad2..68244c27dfbd 100644 --- a/test/unit/org/apache/cassandra/utils/ASTGenerators.java +++ b/test/unit/org/apache/cassandra/utils/ASTGenerators.java @@ -1366,27 +1366,32 @@ public Gen build() { Gen boolGen = SourceDSL.booleans().all(); return rnd -> { - var pk = partitionKeyValuesGen.generate(rnd); - var mutation = mutationGen(rs, pk).generate(rnd); + var pk1 = partitionKeyValuesGen.generate(rnd); + var pk2 = partitionKeyValuesGen.generate(rnd); + var mutation = mutationGen(rs, pk1).generate(rnd); - Select select = select(metadata, pk).withLimit(1); - var columns = model.columns(select); + Select select1 = select(metadata, pk1).withLimit(1); + Select select2 = select(metadata, pk2).withLimit(1); + var columns = model.columns(select1); Txn.Builder builder = Txn.builder(); - builder.addLet("r1", select); - Reference ref = Reference.of(Symbol.unknownType("r1")); + builder.addLet("r1", select1); + Reference ref1 = Reference.of(Symbol.unknownType("r1")); + builder.addLet("r2", select1); + Reference ref2 = Reference.of(Symbol.unknownType("r2")); - builder.addReturn(select(metadata, pk)); + builder.addReturn(select(metadata, pk1)); Conditional.Builder condition = Conditional.builder(); for (var col : columns) { if (boolGen.generate(rnd)) continue; - Reference colRef = ref.add(col); + Reference colRef1 = ref1.add(col); + Reference colRef2 = ref2.add(col); if (boolGen.generate(rnd)) - condition.is(colRef, SourceDSL.arbitrary().enumValues(Conditional.Is.Kind.class).generate(rnd)); + condition.is(colRef1, SourceDSL.arbitrary().enumValues(Conditional.Is.Kind.class).generate(rnd)); if (boolGen.generate(rnd)) { - Expression lhs = colRef; + Expression lhs = colRef1; Expression rhs = value(rnd, getTypeSupport(lhs.type()).bytesGen().generate(rnd), lhs.type()); if (boolGen.generate(rnd)) { @@ -1397,6 +1402,11 @@ public Gen build() Conditional.Where.Inequality inequality = SourceDSL.arbitrary().enumValues(Conditional.Where.Inequality.class).generate(rnd); condition.where(lhs, inequality, rhs); } + if (boolGen.generate(rnd)) + { + Conditional.Where.Inequality inequality = SourceDSL.arbitrary().enumValues(Conditional.Where.Inequality.class).generate(rnd); + condition.where(colRef1, inequality, colRef2); + } } if (condition.isEmpty()) condition.is("r1", Conditional.Is.Kind.NotNull); From 426423133f49beebb1ec74d049647bd5e23a5a71 Mon Sep 17 00:00:00 2001 From: Alan Wang Date: Tue, 16 Jun 2026 15:35:22 -0700 Subject: [PATCH 11/16] changed exception thronw --- .../cql3/transactions/ConditionStatement.java | 3 ++- .../test/accord/AccordCQLTestBase.java | 26 +++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/java/org/apache/cassandra/cql3/transactions/ConditionStatement.java b/src/java/org/apache/cassandra/cql3/transactions/ConditionStatement.java index cc3ca6507ba8..b237b76a41c5 100644 --- a/src/java/org/apache/cassandra/cql3/transactions/ConditionStatement.java +++ b/src/java/org/apache/cassandra/cql3/transactions/ConditionStatement.java @@ -24,6 +24,7 @@ import org.apache.cassandra.cql3.QueryOptions; import org.apache.cassandra.cql3.VariableSpecifications; import org.apache.cassandra.cql3.terms.Term; +import org.apache.cassandra.exceptions.InvalidRequestException; import org.apache.cassandra.service.accord.txn.TxnCondition; import org.apache.cassandra.service.accord.txn.TxnReference; @@ -108,7 +109,7 @@ public ConditionStatement prepare(String keyspace, VariableSpecifications bindVa if (((RowDataReference.Raw) rhs).column() == null) throw new IllegalStateException(String.format("Row reference (%s) can only be used with IS NULL/IS NOT NULL conditions", rhs.getText())); if (!((RowDataReference.Raw) lhs).column().type.equals(((RowDataReference.Raw) rhs).column().type)) - throw new IllegalStateException(String.format("Row reference (%s) must have the same type as row reference (%s)", lhs.getText(), rhs.getText())); + throw new InvalidRequestException(String.format("Row reference (%s) must have the same type as row reference (%s)", lhs.getText(), rhs.getText())); reference = ((RowDataReference.Raw) lhs).prepareAsReceiver(); value = ((RowDataReference.Raw) rhs).prepareAsReceiver(); } diff --git a/test/distributed/org/apache/cassandra/distributed/test/accord/AccordCQLTestBase.java b/test/distributed/org/apache/cassandra/distributed/test/accord/AccordCQLTestBase.java index f9fb4a2758d6..d55a68a58dd4 100644 --- a/test/distributed/org/apache/cassandra/distributed/test/accord/AccordCQLTestBase.java +++ b/test/distributed/org/apache/cassandra/distributed/test/accord/AccordCQLTestBase.java @@ -3634,4 +3634,30 @@ public void testLetComparisonTransactionStatement() throws Throwable assertThat(result).hasSize(1).contains(1, 10); }); } + + @Test + public void testLetComparisonWithDifferentTypesFails() throws Throwable + { + test("CREATE TABLE " + qualifiedAccordTableName + " (k int PRIMARY KEY, v int, c text) WITH " + transactionalMode.asCqlParam(), cluster -> { + try + { + String query = "BEGIN TRANSACTION\n" + + "LET k1 = (SELECT v FROM " + qualifiedAccordTableName + " WHERE k = 1);\n" + + "LET k2 = (SELECT c FROM " + qualifiedAccordTableName + " WHERE k = 2);\n" + + "IF k1.v < k2.c THEN \n" + + " UPDATE " + qualifiedAccordTableName + " SET v = 10 WHERE k = 1;\n" + + "END IF\n" + + "COMMIT TRANSACTION"; + + + cluster.coordinator(1).executeWithResult(query, ConsistencyLevel.SERIAL); + fail("Expected exception"); + } + catch (Throwable t) + { + assertEquals(InvalidRequestException.class.getName(), t.getClass().getName()); + assertEquals("Row reference (k1.v) must have the same type as row reference (k2.c)", t.getMessage()); + } + }); + } } From e7fd97d686e3149dc05ea14dc372ee8c310ce005 Mon Sep 17 00:00:00 2001 From: Alan Wang Date: Tue, 16 Jun 2026 15:39:33 -0700 Subject: [PATCH 12/16] remove dead code --- .../cassandra/utils/NullableSerializer.java | 21 ------------------- 1 file changed, 21 deletions(-) diff --git a/src/java/org/apache/cassandra/utils/NullableSerializer.java b/src/java/org/apache/cassandra/utils/NullableSerializer.java index b2ed0124decd..bddabf407c2e 100644 --- a/src/java/org/apache/cassandra/utils/NullableSerializer.java +++ b/src/java/org/apache/cassandra/utils/NullableSerializer.java @@ -22,7 +22,6 @@ import org.apache.cassandra.db.TypeSizes; import org.apache.cassandra.io.IVersionedSerializer; -import org.apache.cassandra.io.ParameterisedUnversionedSerializer; import org.apache.cassandra.io.UnversionedSerializer; import org.apache.cassandra.io.VersionedSerializer; import org.apache.cassandra.io.util.DataInputPlus; @@ -36,14 +35,6 @@ public static void serializeNullable(T value, DataOutputPlus out, Unversione if (value != null) serializer.serialize(value, out); } - - public static void serializeNullable(T value, P param, DataOutputPlus out, ParameterisedUnversionedSerializer serializer) throws IOException - { - out.writeBoolean(value != null); - if (value != null) - serializer.serialize(value, param, out); - } - public static void serializeNullable(T value, DataOutputPlus out, int version, IVersionedSerializer serializer) throws IOException { out.writeBoolean(value != null); @@ -63,11 +54,6 @@ public static T deserializeNullable(DataInputPlus in, UnversionedSerializer< return in.readBoolean() ? serializer.deserialize(in) : null; } - public static T deserializeNullable(DataInputPlus in, P param, ParameterisedUnversionedSerializer serializer) throws IOException - { - return in.readBoolean() ? serializer.deserialize(param, in) : null; - } - public static T deserializeNullable(DataInputPlus in, int version, IVersionedSerializer serializer) throws IOException { return in.readBoolean() ? serializer.deserialize(in, version) : null; @@ -85,13 +71,6 @@ public static long serializedNullableSize(T value, UnversionedSerializer : TypeSizes.sizeof(false); } - public static long serializedNullableSize(T value, P param, ParameterisedUnversionedSerializer serializer) - { - return value != null - ? TypeSizes.sizeof(true) + serializer.serializedSize(value, param) - : TypeSizes.sizeof(false); - } - public static long serializedNullableSize(T value, int version, IVersionedSerializer serializer) { return value != null From cb12252c534ebc2ba57ff62c7510b437a398fe07 Mon Sep 17 00:00:00 2001 From: Alan Wang Date: Wed, 17 Jun 2026 13:28:56 -0700 Subject: [PATCH 13/16] fixed bugs --- .../cql3/transactions/ConditionStatement.java | 3 - .../service/accord/txn/TxnCondition.java | 37 +++++- .../test/accord/AccordCQLTestBase.java | 110 +++++++++++++++--- 3 files changed, 123 insertions(+), 27 deletions(-) diff --git a/src/java/org/apache/cassandra/cql3/transactions/ConditionStatement.java b/src/java/org/apache/cassandra/cql3/transactions/ConditionStatement.java index b237b76a41c5..963f5a97a559 100644 --- a/src/java/org/apache/cassandra/cql3/transactions/ConditionStatement.java +++ b/src/java/org/apache/cassandra/cql3/transactions/ConditionStatement.java @@ -24,7 +24,6 @@ import org.apache.cassandra.cql3.QueryOptions; import org.apache.cassandra.cql3.VariableSpecifications; import org.apache.cassandra.cql3.terms.Term; -import org.apache.cassandra.exceptions.InvalidRequestException; import org.apache.cassandra.service.accord.txn.TxnCondition; import org.apache.cassandra.service.accord.txn.TxnReference; @@ -108,8 +107,6 @@ public ConditionStatement prepare(String keyspace, VariableSpecifications bindVa throw new IllegalStateException(String.format("Row reference (%s) can only be used with IS NULL/IS NOT NULL conditions", lhs.getText())); if (((RowDataReference.Raw) rhs).column() == null) throw new IllegalStateException(String.format("Row reference (%s) can only be used with IS NULL/IS NOT NULL conditions", rhs.getText())); - if (!((RowDataReference.Raw) lhs).column().type.equals(((RowDataReference.Raw) rhs).column().type)) - throw new InvalidRequestException(String.format("Row reference (%s) must have the same type as row reference (%s)", lhs.getText(), rhs.getText())); reference = ((RowDataReference.Raw) lhs).prepareAsReceiver(); value = ((RowDataReference.Raw) rhs).prepareAsReceiver(); } diff --git a/src/java/org/apache/cassandra/service/accord/txn/TxnCondition.java b/src/java/org/apache/cassandra/service/accord/txn/TxnCondition.java index 34c2992f771c..05a1b94e87a5 100644 --- a/src/java/org/apache/cassandra/service/accord/txn/TxnCondition.java +++ b/src/java/org/apache/cassandra/service/accord/txn/TxnCondition.java @@ -42,12 +42,16 @@ import org.apache.cassandra.db.TypeSizes; import org.apache.cassandra.db.marshal.AbstractType; import org.apache.cassandra.db.marshal.CollectionType; +import org.apache.cassandra.db.marshal.ListType; +import org.apache.cassandra.db.marshal.MapType; +import org.apache.cassandra.db.marshal.SetType; import org.apache.cassandra.db.marshal.UserType; import org.apache.cassandra.db.partitions.FilteredPartition; import org.apache.cassandra.db.rows.Cell; import org.apache.cassandra.db.rows.ColumnData; import org.apache.cassandra.db.rows.ComplexColumnData; import org.apache.cassandra.db.rows.Row; +import org.apache.cassandra.exceptions.InvalidRequestException; import org.apache.cassandra.io.ParameterisedUnversionedSerializer; import org.apache.cassandra.io.util.DataInputPlus; import org.apache.cassandra.io.util.DataOutputPlus; @@ -671,19 +675,42 @@ public String toString() return referenceLHS.toString() + ' ' + kind.symbol + ' ' + referenceRHS.toString(); } + public AbstractType getColumnType(TxnReference.ColumnReference reference) + { + ColumnMetadata column = reference.column(); + if (reference.isElementSelection()) + { + if (column.type instanceof ListType) + return ((ListType) column.type).valueComparator(); + else if (column.type instanceof SetType) + return ((SetType) column.type).nameComparator(); + else if (column.type instanceof MapType) + return ((MapType) column.type).valueComparator(); + } + else if (reference.isFieldSelection()) + { + return reference.getFieldSelectionType(); + } + + return column.type; + } + @Override public boolean applies(TxnData data) { - ColumnMetadata columnLHS = referenceLHS.column(); - ColumnMetadata columnRHS = referenceRHS.column(); + AbstractType typeLHS = getColumnType(referenceLHS); + AbstractType typeRHS = getColumnType(referenceRHS); + + if (typeLHS != typeRHS) + throw new InvalidRequestException(String.format("Invalid type comparison: cannot compare type %s with type %s", typeLHS.asCQL3Type(), typeRHS.asCQL3Type())); - ByteBuffer lhs = referenceLHS.toByteBuffer(data, columnLHS.type); - ByteBuffer rhs = referenceRHS.toByteBuffer(data, columnRHS.type); + ByteBuffer lhs = referenceLHS.toByteBuffer(data, typeLHS); + ByteBuffer rhs = referenceRHS.toByteBuffer(data, typeRHS); if (lhs == null || rhs == null) return false; - return kind.operator.isSatisfiedBy(columnLHS.type, lhs, rhs); + return kind.operator.isSatisfiedBy(typeLHS, lhs, rhs); } private static final ConditionSerializer serializer = new ConditionSerializer<>() diff --git a/test/distributed/org/apache/cassandra/distributed/test/accord/AccordCQLTestBase.java b/test/distributed/org/apache/cassandra/distributed/test/accord/AccordCQLTestBase.java index d55a68a58dd4..2a53212105d7 100644 --- a/test/distributed/org/apache/cassandra/distributed/test/accord/AccordCQLTestBase.java +++ b/test/distributed/org/apache/cassandra/distributed/test/accord/AccordCQLTestBase.java @@ -3605,6 +3605,34 @@ public void userSeesInvalidRejection() throws Exception }); } + @Test + public void testLetComparisonWithDifferentTypesFails() throws Throwable + { + test("CREATE TABLE " + qualifiedAccordTableName + " (k text PRIMARY KEY, customer person) WITH " + transactionalMode.asCqlParam(), cluster -> { + try + { + Object personValue = CQLTester.userType("height", 74, "age", 37); + ByteBuffer personBuffer = CQLTester.makeByteBuffer(personValue, null); + String query = "BEGIN TRANSACTION\n" + + "LET k1 = (SELECT * FROM " + qualifiedAccordTableName + " WHERE k = 'first');\n" + + "LET k2 = (SELECT * FROM " + qualifiedAccordTableName + " WHERE k = 'second');\n" + + "IF k1.k < k2.customer.height THEN \n" + + " UPDATE " + qualifiedAccordTableName + " SET customer = ? WHERE k = 'first';\n" + + "END IF\n" + + "COMMIT TRANSACTION"; + + + cluster.coordinator(1).executeWithResult(query, ConsistencyLevel.SERIAL, personBuffer); + fail("Expected exception"); + } + catch (Throwable t) + { + assertEquals(InvalidRequestException.class.getName(), t.getClass().getName()); + assertEquals("Invalid type comparison: cannot compare type text with type int", t.getMessage()); + } + }); + } + @Test public void testLetComparisonTransactionStatement() throws Throwable { @@ -3636,28 +3664,72 @@ public void testLetComparisonTransactionStatement() throws Throwable } @Test - public void testLetComparisonWithDifferentTypesFails() throws Throwable + public void testLetComparisonWithDifferentTypes() throws Throwable { - test("CREATE TABLE " + qualifiedAccordTableName + " (k int PRIMARY KEY, v int, c text) WITH " + transactionalMode.asCqlParam(), cluster -> { - try - { - String query = "BEGIN TRANSACTION\n" + - "LET k1 = (SELECT v FROM " + qualifiedAccordTableName + " WHERE k = 1);\n" + - "LET k2 = (SELECT c FROM " + qualifiedAccordTableName + " WHERE k = 2);\n" + - "IF k1.v < k2.c THEN \n" + - " UPDATE " + qualifiedAccordTableName + " SET v = 10 WHERE k = 1;\n" + - "END IF\n" + - "COMMIT TRANSACTION"; + test("CREATE TABLE " + qualifiedAccordTableName + " (k int PRIMARY KEY, customer person) WITH " + transactionalMode.asCqlParam(), cluster -> { + Object personValue1 = CQLTester.userType("height", 74, "age", 37); + ByteBuffer personBuffer1 = CQLTester.makeByteBuffer(personValue1, null); + Object personValue2 = CQLTester.userType("height", 74, "age", 38); + ByteBuffer personBuffer2 = CQLTester.makeByteBuffer(personValue2, null); - cluster.coordinator(1).executeWithResult(query, ConsistencyLevel.SERIAL); - fail("Expected exception"); - } - catch (Throwable t) - { - assertEquals(InvalidRequestException.class.getName(), t.getClass().getName()); - assertEquals("Row reference (k1.v) must have the same type as row reference (k2.c)", t.getMessage()); - } + String insert = "BEGIN TRANSACTION\n" + + " INSERT INTO " + qualifiedAccordTableName + " (k, customer) VALUES (?, ?);\n" + + " INSERT INTO " + qualifiedAccordTableName + " (k, customer) VALUES (?, ?);\n" + + "COMMIT TRANSACTION"; + cluster.coordinator(1).executeWithResult(insert, ConsistencyLevel.ANY, 0, personBuffer1, 32, personBuffer2); + + String update = "BEGIN TRANSACTION\n" + + "LET k1 = (SELECT * FROM " + qualifiedAccordTableName + " WHERE k = 0);\n" + + "LET k2 = (SELECT * FROM " + qualifiedAccordTableName + " WHERE k = 32);\n" + + "IF k1.customer.height > k2.k THEN \n" + + " UPDATE " + qualifiedAccordTableName + " SET customer = ? WHERE k = 32;\n" + + "END IF\n" + + "COMMIT TRANSACTION"; + + cluster.coordinator(1).executeWithResult(update, ConsistencyLevel.SERIAL, personBuffer1); + + String read = "BEGIN TRANSACTION\n" + + "SELECT * FROM " + qualifiedAccordTableName + " WHERE k = 32;\n" + + "COMMIT TRANSACTION"; + + SimpleQueryResult result = cluster.coordinator(1).executeWithResult(read, ConsistencyLevel.SERIAL); + assertThat(result).hasSize(1).contains(32, personBuffer1); + }); + } + + @Test + public void testLetComparisonWithUDT() throws Throwable + { + test("CREATE TABLE " + qualifiedAccordTableName + " (k int PRIMARY KEY, customer person) WITH " + transactionalMode.asCqlParam(), cluster -> { + Object personValue1 = CQLTester.userType("height", 74, "age", 37); + ByteBuffer personBuffer1 = CQLTester.makeByteBuffer(personValue1, null); + + Object personValue2 = CQLTester.userType("height", 74, "age", 38); + ByteBuffer personBuffer2 = CQLTester.makeByteBuffer(personValue2, null); + + String insert = "BEGIN TRANSACTION\n" + + " INSERT INTO " + qualifiedAccordTableName + " (k, customer) VALUES (?, ?);\n" + + " INSERT INTO " + qualifiedAccordTableName + " (k, customer) VALUES (?, ?);\n" + + "COMMIT TRANSACTION"; + cluster.coordinator(1).executeWithResult(insert, ConsistencyLevel.ANY, 0, personBuffer1, 1, personBuffer2); + + String update = "BEGIN TRANSACTION\n" + + "LET k1 = (SELECT * FROM " + qualifiedAccordTableName + " WHERE k = 0);\n" + + "LET k2 = (SELECT * FROM " + qualifiedAccordTableName + " WHERE k = 1);\n" + + "IF k1.customer.height = k2.customer.height THEN \n" + + " UPDATE " + qualifiedAccordTableName + " SET customer = ? WHERE k = 1;\n" + + "END IF\n" + + "COMMIT TRANSACTION"; + + cluster.coordinator(1).executeWithResult(update, ConsistencyLevel.SERIAL, personBuffer1); + + String read = "BEGIN TRANSACTION\n" + + "SELECT * FROM " + qualifiedAccordTableName + " WHERE k = 1;\n" + + "COMMIT TRANSACTION"; + + SimpleQueryResult result = cluster.coordinator(1).executeWithResult(read, ConsistencyLevel.SERIAL); + assertThat(result).hasSize(1).contains(1, personBuffer1); }); } } From f3d2d727afe22e6f1a19eb9e6f3c108822c24e43 Mon Sep 17 00:00:00 2001 From: Alan Wang Date: Wed, 17 Jun 2026 13:42:02 -0700 Subject: [PATCH 14/16] map test --- .../test/accord/AccordCQLTestBase.java | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/test/distributed/org/apache/cassandra/distributed/test/accord/AccordCQLTestBase.java b/test/distributed/org/apache/cassandra/distributed/test/accord/AccordCQLTestBase.java index 2a53212105d7..685a7d444b40 100644 --- a/test/distributed/org/apache/cassandra/distributed/test/accord/AccordCQLTestBase.java +++ b/test/distributed/org/apache/cassandra/distributed/test/accord/AccordCQLTestBase.java @@ -3732,4 +3732,33 @@ public void testLetComparisonWithUDT() throws Throwable assertThat(result).hasSize(1).contains(1, personBuffer1); }); } + + @Test + public void testLetComparisonWithMap() throws Throwable + { + test("CREATE TABLE " + qualifiedAccordTableName + " (k int PRIMARY KEY, v map) WITH " + transactionalMode.asCqlParam(), cluster -> { + String insert = "BEGIN TRANSACTION\n" + + " INSERT INTO " + qualifiedAccordTableName + " (k, v) VALUES (0, {1:3});\n" + + " INSERT INTO " + qualifiedAccordTableName + " (k, v) VALUES (1, {0:5});\n" + + "COMMIT TRANSACTION"; + cluster.coordinator(1).executeWithResult(insert, ConsistencyLevel.ANY); + + String update = "BEGIN TRANSACTION\n" + + "LET k1 = (SELECT * FROM " + qualifiedAccordTableName + " WHERE k = 0);\n" + + "LET k2 = (SELECT * FROM " + qualifiedAccordTableName + " WHERE k = 1);\n" + + "IF k1.v[1] < k2.v[0] THEN \n" + + " UPDATE " + qualifiedAccordTableName + " SET v = {1:10} WHERE k = 1;\n" + + "END IF\n" + + "COMMIT TRANSACTION"; + + cluster.coordinator(1).executeWithResult(update, ConsistencyLevel.SERIAL); + + String read = "BEGIN TRANSACTION\n" + + "SELECT v[1] FROM " + qualifiedAccordTableName + " WHERE k = 1;\n" + + "COMMIT TRANSACTION"; + + SimpleQueryResult result = cluster.coordinator(1).executeWithResult(read, ConsistencyLevel.SERIAL); + assertThat(result).hasSize(1).contains(10); + }); + } } From 5e66e099c7d86ca970c4ea28ac399682e84fe29c Mon Sep 17 00:00:00 2001 From: Alan Wang Date: Wed, 17 Jun 2026 13:44:15 -0700 Subject: [PATCH 15/16] fix --- .../cassandra/distributed/test/accord/AccordCQLTestBase.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/distributed/org/apache/cassandra/distributed/test/accord/AccordCQLTestBase.java b/test/distributed/org/apache/cassandra/distributed/test/accord/AccordCQLTestBase.java index 685a7d444b40..3dcf0fde4081 100644 --- a/test/distributed/org/apache/cassandra/distributed/test/accord/AccordCQLTestBase.java +++ b/test/distributed/org/apache/cassandra/distributed/test/accord/AccordCQLTestBase.java @@ -3738,7 +3738,7 @@ public void testLetComparisonWithMap() throws Throwable { test("CREATE TABLE " + qualifiedAccordTableName + " (k int PRIMARY KEY, v map) WITH " + transactionalMode.asCqlParam(), cluster -> { String insert = "BEGIN TRANSACTION\n" + - " INSERT INTO " + qualifiedAccordTableName + " (k, v) VALUES (0, {1:3});\n" + + " INSERT INTO " + qualifiedAccordTableName + " (k, v) VALUES (0, {1:3, 2:6});\n" + " INSERT INTO " + qualifiedAccordTableName + " (k, v) VALUES (1, {0:5});\n" + "COMMIT TRANSACTION"; cluster.coordinator(1).executeWithResult(insert, ConsistencyLevel.ANY); From 5b6844d92f741e9a80c79bf10898cac1c594675e Mon Sep 17 00:00:00 2001 From: Alan Wang Date: Thu, 18 Jun 2026 15:29:22 -0700 Subject: [PATCH 16/16] move type comparison --- .../cassandra/cql3/transactions/ConditionStatement.java | 3 +++ .../org/apache/cassandra/service/accord/txn/TxnCondition.java | 4 ---- .../cassandra/distributed/test/accord/AccordCQLTestBase.java | 2 +- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/java/org/apache/cassandra/cql3/transactions/ConditionStatement.java b/src/java/org/apache/cassandra/cql3/transactions/ConditionStatement.java index 963f5a97a559..4f02d5dda85a 100644 --- a/src/java/org/apache/cassandra/cql3/transactions/ConditionStatement.java +++ b/src/java/org/apache/cassandra/cql3/transactions/ConditionStatement.java @@ -24,6 +24,7 @@ import org.apache.cassandra.cql3.QueryOptions; import org.apache.cassandra.cql3.VariableSpecifications; import org.apache.cassandra.cql3.terms.Term; +import org.apache.cassandra.exceptions.InvalidRequestException; import org.apache.cassandra.service.accord.txn.TxnCondition; import org.apache.cassandra.service.accord.txn.TxnReference; @@ -109,6 +110,8 @@ public ConditionStatement prepare(String keyspace, VariableSpecifications bindVa throw new IllegalStateException(String.format("Row reference (%s) can only be used with IS NULL/IS NOT NULL conditions", rhs.getText())); reference = ((RowDataReference.Raw) lhs).prepareAsReceiver(); value = ((RowDataReference.Raw) rhs).prepareAsReceiver(); + if (!reference.toResultMetadata().type.equals(((RowDataReference) value).toResultMetadata().type)) + throw new InvalidRequestException(String.format("Row reference (%s) must have the same type as row reference (%s)", lhs.getText(), rhs.getText())); } else if (lhs instanceof RowDataReference.Raw) { diff --git a/src/java/org/apache/cassandra/service/accord/txn/TxnCondition.java b/src/java/org/apache/cassandra/service/accord/txn/TxnCondition.java index 05a1b94e87a5..ad40789443d2 100644 --- a/src/java/org/apache/cassandra/service/accord/txn/TxnCondition.java +++ b/src/java/org/apache/cassandra/service/accord/txn/TxnCondition.java @@ -51,7 +51,6 @@ import org.apache.cassandra.db.rows.ColumnData; import org.apache.cassandra.db.rows.ComplexColumnData; import org.apache.cassandra.db.rows.Row; -import org.apache.cassandra.exceptions.InvalidRequestException; import org.apache.cassandra.io.ParameterisedUnversionedSerializer; import org.apache.cassandra.io.util.DataInputPlus; import org.apache.cassandra.io.util.DataOutputPlus; @@ -701,9 +700,6 @@ public boolean applies(TxnData data) AbstractType typeLHS = getColumnType(referenceLHS); AbstractType typeRHS = getColumnType(referenceRHS); - if (typeLHS != typeRHS) - throw new InvalidRequestException(String.format("Invalid type comparison: cannot compare type %s with type %s", typeLHS.asCQL3Type(), typeRHS.asCQL3Type())); - ByteBuffer lhs = referenceLHS.toByteBuffer(data, typeLHS); ByteBuffer rhs = referenceRHS.toByteBuffer(data, typeRHS); diff --git a/test/distributed/org/apache/cassandra/distributed/test/accord/AccordCQLTestBase.java b/test/distributed/org/apache/cassandra/distributed/test/accord/AccordCQLTestBase.java index 3dcf0fde4081..1cbde6b5f24b 100644 --- a/test/distributed/org/apache/cassandra/distributed/test/accord/AccordCQLTestBase.java +++ b/test/distributed/org/apache/cassandra/distributed/test/accord/AccordCQLTestBase.java @@ -3628,7 +3628,7 @@ public void testLetComparisonWithDifferentTypesFails() throws Throwable catch (Throwable t) { assertEquals(InvalidRequestException.class.getName(), t.getClass().getName()); - assertEquals("Invalid type comparison: cannot compare type text with type int", t.getMessage()); + assertEquals("Row reference (k1.k) must have the same type as row reference (k2.customer.height)", t.getMessage()); } }); }