From c8592c2de4bbc534b6ca859fd31048c7392c17da Mon Sep 17 00:00:00 2001 From: Minal Kyada Date: Mon, 10 Aug 2026 10:42:23 -0700 Subject: [PATCH 1/3] added isEmptyValueMeaningless support for LWT/CAS --- .../org/apache/cassandra/cql3/Operator.java | 33 +++++++++++++++++ .../cql3/conditions/ColumnCondition.java | 19 ++++++++-- .../harry/model/ASTSingleTableModel.java | 3 ++ .../cql3/conditions/ColumnConditionTest.java | 37 +++++++++++++++++++ 4 files changed, 89 insertions(+), 3 deletions(-) diff --git a/src/java/org/apache/cassandra/cql3/Operator.java b/src/java/org/apache/cassandra/cql3/Operator.java index b9393054fc48..21946a604de4 100644 --- a/src/java/org/apache/cassandra/cql3/Operator.java +++ b/src/java/org/apache/cassandra/cql3/Operator.java @@ -114,6 +114,12 @@ public boolean isSupportedByRestrictionsOn(ColumnsExpression expression) { return true; } + + @Override + public boolean supportsNullCondition() + { + return true; + } }, LT(4) { @@ -371,6 +377,12 @@ public boolean isSupportedByRestrictionsOn(ColumnsExpression expression) { return expression.kind() == ColumnsExpression.Kind.SINGLE_COLUMN || expression.kind() == ColumnsExpression.Kind.MULTI_COLUMN; } + + @Override + public boolean supportsNullCondition() + { + return true; + } }, CONTAINS(5) { @@ -534,6 +546,12 @@ public boolean isSlice() { return true; } + + @Override + public boolean supportsNullCondition() + { + return true; + } }, IS_NOT(9) { @@ -693,6 +711,12 @@ public boolean isSupportedByRestrictionsOn(ColumnsExpression expression) { return expression.kind() == ColumnsExpression.Kind.SINGLE_COLUMN || expression.kind() == ColumnsExpression.Kind.MULTI_COLUMN; } + + @Override + public boolean supportsNullCondition() + { + return true; + } }, NOT_CONTAINS(17) { @@ -1095,6 +1119,15 @@ public boolean requiresIndexing() return false; } + /** + * Checks if this operator supports null value comparisons, as used in LWT IF conditions. + * @return {@code true} if null is supported, {@code false} otherwise. + */ + public boolean supportsNullCondition() + { + return false; + } + /** * Checks if this operator returning a slice of the data. * @return {@code true} if this operator is a slice operator, {@code false} otherwise. diff --git a/src/java/org/apache/cassandra/cql3/conditions/ColumnCondition.java b/src/java/org/apache/cassandra/cql3/conditions/ColumnCondition.java index cbe2607cfc9a..f38e30a35427 100644 --- a/src/java/org/apache/cassandra/cql3/conditions/ColumnCondition.java +++ b/src/java/org/apache/cassandra/cql3/conditions/ColumnCondition.java @@ -297,7 +297,14 @@ public SimpleBound(ColumnMetadata column, TableMetadata table, Operator operator @Override public boolean appliesTo(Row row) { - return operator.isSatisfiedBy(column.type, rowValue(row), value); + ByteBuffer left = rowValue(row); + ByteBuffer right = value; + if (operator.supportsNullCondition()) + { + left = column.type.sanitize(left); + right = column.type.sanitize(right); + } + return operator.isSatisfiedBy(column.type, left, right); } @Override @@ -411,8 +418,14 @@ public BoundKind kind() @Override public boolean appliesTo(Row row) { - ByteBuffer element = elementValue(row); - return operator.isSatisfiedBy(elementType, element, value); + ByteBuffer left = elementValue(row); + ByteBuffer right = value; + if (operator.supportsNullCondition()) + { + left = elementType.sanitize(left); + right = elementType.sanitize(right); + } + return operator.isSatisfiedBy(elementType, left, right); } private ByteBuffer elementValue(Row row) 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 58cfa4905a3c..7d09e05cb75a 100644 --- a/test/harry/main/org/apache/cassandra/harry/model/ASTSingleTableModel.java +++ b/test/harry/main/org/apache/cassandra/harry/model/ASTSingleTableModel.java @@ -943,6 +943,9 @@ else if (condition.getClass() == Conditional.Where.class) { case cas: { + lhs = where.lhs.type().sanitize(lhs); + rhs = where.rhs.type().sanitize(rhs); + // If anything is null avoid doing the test, but there is a special case where this returns true... both sides are null! // This logic isn't consistent with other parts of the database and is local to CAS IF clause // see ML@Inconsistent null handling between WHERE and IF clauses diff --git a/test/unit/org/apache/cassandra/cql3/conditions/ColumnConditionTest.java b/test/unit/org/apache/cassandra/cql3/conditions/ColumnConditionTest.java index df2de479a0be..6dc932ff836f 100644 --- a/test/unit/org/apache/cassandra/cql3/conditions/ColumnConditionTest.java +++ b/test/unit/org/apache/cassandra/cql3/conditions/ColumnConditionTest.java @@ -342,6 +342,8 @@ public void testSimpleBoundIsSatisfiedByValue() throws InvalidRequestException assertFalse(appliesSimpleCondition(ONE, EQ, null)); assertFalse(appliesSimpleCondition(null, EQ, ONE)); assertTrue(appliesSimpleCondition(null, EQ, null)); + assertTrue(appliesSimpleCondition(null, EQ, EMPTY_BYTE_BUFFER)); + assertTrue(appliesSimpleCondition(EMPTY_BYTE_BUFFER, EQ, null)); // NEQ assertFalse(appliesSimpleCondition(ONE, NEQ, ONE)); @@ -353,6 +355,8 @@ public void testSimpleBoundIsSatisfiedByValue() throws InvalidRequestException assertTrue(appliesSimpleCondition(ONE, NEQ, null)); assertTrue(appliesSimpleCondition(null, NEQ, ONE)); assertFalse(appliesSimpleCondition(null, NEQ, null)); + assertFalse(appliesSimpleCondition(null, NEQ, EMPTY_BYTE_BUFFER)); + assertFalse(appliesSimpleCondition(EMPTY_BYTE_BUFFER, NEQ, null)); // LT assertFalse(appliesSimpleCondition(ONE, LT, ONE)); @@ -395,6 +399,33 @@ public void testSimpleBoundIsSatisfiedByValue() throws InvalidRequestException assertFalse(appliesSimpleCondition(null, GTE, ONE)); } + private static boolean appliesINCondition(ByteBuffer rowValue, List inValues) + { + ColumnMetadata definition = ColumnMetadata.regularColumn("ks", "cf", "c", Int32Type.instance, ColumnMetadata.NO_UNIQUE_ID); + ByteBuffer packed = ListType.getInstance(Int32Type.instance, false).pack(inValues); + ColumnCondition.SimpleBound bound = new ColumnCondition.SimpleBound(definition, null, Operator.IN, packed); + return bound.appliesTo(newRow(definition, rowValue)); + } + + @Test + public void testSimpleBoundINNullHandling() throws InvalidRequestException + { + // Normal matching + assertTrue(appliesINCondition(ONE, list(ONE, TWO))); + assertFalse(appliesINCondition(ONE, list(TWO))); + assertFalse(appliesINCondition(ONE, list((ByteBuffer) null, TWO))); + + // Absent/null cell matches null element in IN list + assertTrue(appliesINCondition(null, list((ByteBuffer) null, ONE))); + assertFalse(appliesINCondition(null, list(ONE, TWO))); + // TODO: it should be assertTrue once list sanitization is added. + assertFalse(appliesINCondition(null, list(EMPTY_BYTE_BUFFER, ONE))); + + // Legacy 0-byte stored value (Int32Type is meaningless-empty) matches null in IN list + assertTrue(appliesINCondition(EMPTY_BYTE_BUFFER, list((ByteBuffer) null, ONE))); + assertFalse(appliesINCondition(EMPTY_BYTE_BUFFER, list(ONE, TWO))); + } + private static List list(ByteBuffer... values) { return asList(values); @@ -842,6 +873,9 @@ public void testUDTBound() throws InvalidRequestException assertFalse(conditionUDTApplies(ONE, EQ, ByteBufferUtil.EMPTY_BYTE_BUFFER)); assertFalse(conditionUDTApplies(ByteBufferUtil.EMPTY_BYTE_BUFFER, EQ, ONE)); assertTrue(conditionUDTApplies(ByteBufferUtil.EMPTY_BYTE_BUFFER, EQ, ByteBufferUtil.EMPTY_BYTE_BUFFER)); + // Legacy 0-byte stored value must match null condition and vice versa (elementValue() sanitizes left) + assertTrue(conditionUDTApplies(null, EQ, ByteBufferUtil.EMPTY_BYTE_BUFFER)); + assertTrue(conditionUDTApplies(ByteBufferUtil.EMPTY_BYTE_BUFFER, EQ, null)); // NEQ assertFalse(conditionUDTApplies(ONE, NEQ, ONE)); @@ -857,6 +891,9 @@ public void testUDTBound() throws InvalidRequestException assertTrue(conditionUDTApplies(ONE, NEQ, ByteBufferUtil.EMPTY_BYTE_BUFFER)); assertTrue(conditionUDTApplies(ByteBufferUtil.EMPTY_BYTE_BUFFER, NEQ, ONE)); assertFalse(conditionUDTApplies(ByteBufferUtil.EMPTY_BYTE_BUFFER, NEQ, ByteBufferUtil.EMPTY_BYTE_BUFFER)); + // Legacy 0-byte stored value must not differ from null condition and vice versa + assertFalse(conditionUDTApplies(null, NEQ, ByteBufferUtil.EMPTY_BYTE_BUFFER)); + assertFalse(conditionUDTApplies(ByteBufferUtil.EMPTY_BYTE_BUFFER, NEQ, null)); // LT assertFalse(conditionUDTApplies(ONE, LT, ONE)); From d4440e2b50452227c8a4ee9a020f7a09cbbba587 Mon Sep 17 00:00:00 2001 From: Minal Kyada Date: Fri, 14 Aug 2026 12:13:44 -0700 Subject: [PATCH 2/3] harry model fix for CAS/LWT null/empty bytes comparisons --- .../org/apache/cassandra/cql3/Operator.java | 33 ----------------- .../cql3/conditions/ColumnCondition.java | 19 ++-------- .../harry/model/ASTSingleTableModel.java | 19 +++++----- .../cassandra/cql3/ast/Conditional.java | 5 +++ .../cql3/conditions/ColumnConditionTest.java | 37 ------------------- 5 files changed, 18 insertions(+), 95 deletions(-) diff --git a/src/java/org/apache/cassandra/cql3/Operator.java b/src/java/org/apache/cassandra/cql3/Operator.java index 21946a604de4..b9393054fc48 100644 --- a/src/java/org/apache/cassandra/cql3/Operator.java +++ b/src/java/org/apache/cassandra/cql3/Operator.java @@ -114,12 +114,6 @@ public boolean isSupportedByRestrictionsOn(ColumnsExpression expression) { return true; } - - @Override - public boolean supportsNullCondition() - { - return true; - } }, LT(4) { @@ -377,12 +371,6 @@ public boolean isSupportedByRestrictionsOn(ColumnsExpression expression) { return expression.kind() == ColumnsExpression.Kind.SINGLE_COLUMN || expression.kind() == ColumnsExpression.Kind.MULTI_COLUMN; } - - @Override - public boolean supportsNullCondition() - { - return true; - } }, CONTAINS(5) { @@ -546,12 +534,6 @@ public boolean isSlice() { return true; } - - @Override - public boolean supportsNullCondition() - { - return true; - } }, IS_NOT(9) { @@ -711,12 +693,6 @@ public boolean isSupportedByRestrictionsOn(ColumnsExpression expression) { return expression.kind() == ColumnsExpression.Kind.SINGLE_COLUMN || expression.kind() == ColumnsExpression.Kind.MULTI_COLUMN; } - - @Override - public boolean supportsNullCondition() - { - return true; - } }, NOT_CONTAINS(17) { @@ -1119,15 +1095,6 @@ public boolean requiresIndexing() return false; } - /** - * Checks if this operator supports null value comparisons, as used in LWT IF conditions. - * @return {@code true} if null is supported, {@code false} otherwise. - */ - public boolean supportsNullCondition() - { - return false; - } - /** * Checks if this operator returning a slice of the data. * @return {@code true} if this operator is a slice operator, {@code false} otherwise. diff --git a/src/java/org/apache/cassandra/cql3/conditions/ColumnCondition.java b/src/java/org/apache/cassandra/cql3/conditions/ColumnCondition.java index f38e30a35427..cbe2607cfc9a 100644 --- a/src/java/org/apache/cassandra/cql3/conditions/ColumnCondition.java +++ b/src/java/org/apache/cassandra/cql3/conditions/ColumnCondition.java @@ -297,14 +297,7 @@ public SimpleBound(ColumnMetadata column, TableMetadata table, Operator operator @Override public boolean appliesTo(Row row) { - ByteBuffer left = rowValue(row); - ByteBuffer right = value; - if (operator.supportsNullCondition()) - { - left = column.type.sanitize(left); - right = column.type.sanitize(right); - } - return operator.isSatisfiedBy(column.type, left, right); + return operator.isSatisfiedBy(column.type, rowValue(row), value); } @Override @@ -418,14 +411,8 @@ public BoundKind kind() @Override public boolean appliesTo(Row row) { - ByteBuffer left = elementValue(row); - ByteBuffer right = value; - if (operator.supportsNullCondition()) - { - left = elementType.sanitize(left); - right = elementType.sanitize(right); - } - return operator.isSatisfiedBy(elementType, left, right); + ByteBuffer element = elementValue(row); + return operator.isSatisfiedBy(elementType, element, value); } private ByteBuffer elementValue(Row row) 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 7d09e05cb75a..d9a695f94b6d 100644 --- a/test/harry/main/org/apache/cassandra/harry/model/ASTSingleTableModel.java +++ b/test/harry/main/org/apache/cassandra/harry/model/ASTSingleTableModel.java @@ -934,7 +934,7 @@ else if (condition.getClass() == Conditional.Where.class) if (!where.lhs.type().equals(where.rhs.type())) throw new UnsupportedOperationException("For now where clause must always have matching types: given " + where.lhs.type() + ' ' + where.rhs.type()); ByteBuffer lhs = where.lhs instanceof ReferenceExpression - ? (ByteBuffer) extract((ReferenceExpression) where.lhs, lets) + ? (ByteBuffer) extract((ReferenceExpression) where.lhs, lets, who == Who.cas && where.kind.isEqualityBased()) : eval(where.lhs); ByteBuffer rhs = where.rhs instanceof ReferenceExpression ? (ByteBuffer) extract((ReferenceExpression) where.rhs, lets) @@ -943,9 +943,6 @@ else if (condition.getClass() == Conditional.Where.class) { case cas: { - lhs = where.lhs.type().sanitize(lhs); - rhs = where.rhs.type().sanitize(rhs); - // If anything is null avoid doing the test, but there is a special case where this returns true... both sides are null! // This logic isn't consistent with other parts of the database and is local to CAS IF clause // see ML@Inconsistent null handling between WHERE and IF clauses @@ -982,10 +979,14 @@ else if (condition.getClass() == Conditional.And.class) } } - // Either ByteBuffer (cell) or ByteBuffer[] (row) private static Object extract(ReferenceExpression expr, Map lets) { - Object result = extract0(expr, lets); + return extract(expr, lets, false); + } + + private static Object extract(ReferenceExpression expr, Map lets, boolean preserveEmpty) + { + Object result = extract0(expr, lets, preserveEmpty); if (result instanceof SelectResult) { var rows = ((SelectResult) result).rows; @@ -995,14 +996,14 @@ private static Object extract(ReferenceExpression expr, Map (lets), SelectResult (row), ByteBuffer (cell) - private static Object extract0(ReferenceExpression expr, @Nullable Object o) + private static Object extract0(ReferenceExpression expr, @Nullable Object o, boolean preserveEmpty) { if (o == null) return null; if (expr instanceof Reference) { Reference ref = (Reference) expr; for (var symbol : ref.path) - o = extract0(symbol, o); + o = extract0(symbol, o, preserveEmpty); return o; } else if (expr instanceof Symbol) @@ -1019,7 +1020,7 @@ else if (o instanceof SelectResult) if (result.rows.length == 0) return null; ByteBuffer bb = result.rows[0][result.columns.indexOf(symbol)]; - if (bb != null && symbol.type().isNull(bb)) + if (!preserveEmpty && bb != null && symbol.type().isNull(bb)) bb = null; return bb; } diff --git a/test/unit/org/apache/cassandra/cql3/ast/Conditional.java b/test/unit/org/apache/cassandra/cql3/ast/Conditional.java index c7323127f66e..b3855fd96d09 100644 --- a/test/unit/org/apache/cassandra/cql3/ast/Conditional.java +++ b/test/unit/org/apache/cassandra/cql3/ast/Conditional.java @@ -83,6 +83,11 @@ public enum Inequality this.value = value; } + public boolean isEqualityBased() + { + return this == EQUAL || this == NOT_EQUAL; + } + public boolean test(AbstractType type, ByteBuffer a, ByteBuffer b) { int rc = type.compare(a, b); diff --git a/test/unit/org/apache/cassandra/cql3/conditions/ColumnConditionTest.java b/test/unit/org/apache/cassandra/cql3/conditions/ColumnConditionTest.java index 6dc932ff836f..df2de479a0be 100644 --- a/test/unit/org/apache/cassandra/cql3/conditions/ColumnConditionTest.java +++ b/test/unit/org/apache/cassandra/cql3/conditions/ColumnConditionTest.java @@ -342,8 +342,6 @@ public void testSimpleBoundIsSatisfiedByValue() throws InvalidRequestException assertFalse(appliesSimpleCondition(ONE, EQ, null)); assertFalse(appliesSimpleCondition(null, EQ, ONE)); assertTrue(appliesSimpleCondition(null, EQ, null)); - assertTrue(appliesSimpleCondition(null, EQ, EMPTY_BYTE_BUFFER)); - assertTrue(appliesSimpleCondition(EMPTY_BYTE_BUFFER, EQ, null)); // NEQ assertFalse(appliesSimpleCondition(ONE, NEQ, ONE)); @@ -355,8 +353,6 @@ public void testSimpleBoundIsSatisfiedByValue() throws InvalidRequestException assertTrue(appliesSimpleCondition(ONE, NEQ, null)); assertTrue(appliesSimpleCondition(null, NEQ, ONE)); assertFalse(appliesSimpleCondition(null, NEQ, null)); - assertFalse(appliesSimpleCondition(null, NEQ, EMPTY_BYTE_BUFFER)); - assertFalse(appliesSimpleCondition(EMPTY_BYTE_BUFFER, NEQ, null)); // LT assertFalse(appliesSimpleCondition(ONE, LT, ONE)); @@ -399,33 +395,6 @@ public void testSimpleBoundIsSatisfiedByValue() throws InvalidRequestException assertFalse(appliesSimpleCondition(null, GTE, ONE)); } - private static boolean appliesINCondition(ByteBuffer rowValue, List inValues) - { - ColumnMetadata definition = ColumnMetadata.regularColumn("ks", "cf", "c", Int32Type.instance, ColumnMetadata.NO_UNIQUE_ID); - ByteBuffer packed = ListType.getInstance(Int32Type.instance, false).pack(inValues); - ColumnCondition.SimpleBound bound = new ColumnCondition.SimpleBound(definition, null, Operator.IN, packed); - return bound.appliesTo(newRow(definition, rowValue)); - } - - @Test - public void testSimpleBoundINNullHandling() throws InvalidRequestException - { - // Normal matching - assertTrue(appliesINCondition(ONE, list(ONE, TWO))); - assertFalse(appliesINCondition(ONE, list(TWO))); - assertFalse(appliesINCondition(ONE, list((ByteBuffer) null, TWO))); - - // Absent/null cell matches null element in IN list - assertTrue(appliesINCondition(null, list((ByteBuffer) null, ONE))); - assertFalse(appliesINCondition(null, list(ONE, TWO))); - // TODO: it should be assertTrue once list sanitization is added. - assertFalse(appliesINCondition(null, list(EMPTY_BYTE_BUFFER, ONE))); - - // Legacy 0-byte stored value (Int32Type is meaningless-empty) matches null in IN list - assertTrue(appliesINCondition(EMPTY_BYTE_BUFFER, list((ByteBuffer) null, ONE))); - assertFalse(appliesINCondition(EMPTY_BYTE_BUFFER, list(ONE, TWO))); - } - private static List list(ByteBuffer... values) { return asList(values); @@ -873,9 +842,6 @@ public void testUDTBound() throws InvalidRequestException assertFalse(conditionUDTApplies(ONE, EQ, ByteBufferUtil.EMPTY_BYTE_BUFFER)); assertFalse(conditionUDTApplies(ByteBufferUtil.EMPTY_BYTE_BUFFER, EQ, ONE)); assertTrue(conditionUDTApplies(ByteBufferUtil.EMPTY_BYTE_BUFFER, EQ, ByteBufferUtil.EMPTY_BYTE_BUFFER)); - // Legacy 0-byte stored value must match null condition and vice versa (elementValue() sanitizes left) - assertTrue(conditionUDTApplies(null, EQ, ByteBufferUtil.EMPTY_BYTE_BUFFER)); - assertTrue(conditionUDTApplies(ByteBufferUtil.EMPTY_BYTE_BUFFER, EQ, null)); // NEQ assertFalse(conditionUDTApplies(ONE, NEQ, ONE)); @@ -891,9 +857,6 @@ public void testUDTBound() throws InvalidRequestException assertTrue(conditionUDTApplies(ONE, NEQ, ByteBufferUtil.EMPTY_BYTE_BUFFER)); assertTrue(conditionUDTApplies(ByteBufferUtil.EMPTY_BYTE_BUFFER, NEQ, ONE)); assertFalse(conditionUDTApplies(ByteBufferUtil.EMPTY_BYTE_BUFFER, NEQ, ByteBufferUtil.EMPTY_BYTE_BUFFER)); - // Legacy 0-byte stored value must not differ from null condition and vice versa - assertFalse(conditionUDTApplies(null, NEQ, ByteBufferUtil.EMPTY_BYTE_BUFFER)); - assertFalse(conditionUDTApplies(ByteBufferUtil.EMPTY_BYTE_BUFFER, NEQ, null)); // LT assertFalse(conditionUDTApplies(ONE, LT, ONE)); From b7c55393da6d2f13564a9e284b7294caf3d1df67 Mon Sep 17 00:00:00 2001 From: Minal Kyada Date: Thu, 20 Aug 2026 14:25:14 -0700 Subject: [PATCH 3/3] added the same semantic for rhs --- .../org/apache/cassandra/harry/model/ASTSingleTableModel.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 d9a695f94b6d..3288c9fd139d 100644 --- a/test/harry/main/org/apache/cassandra/harry/model/ASTSingleTableModel.java +++ b/test/harry/main/org/apache/cassandra/harry/model/ASTSingleTableModel.java @@ -937,7 +937,7 @@ else if (condition.getClass() == Conditional.Where.class) ? (ByteBuffer) extract((ReferenceExpression) where.lhs, lets, who == Who.cas && where.kind.isEqualityBased()) : eval(where.lhs); ByteBuffer rhs = where.rhs instanceof ReferenceExpression - ? (ByteBuffer) extract((ReferenceExpression) where.rhs, lets) + ? (ByteBuffer) extract((ReferenceExpression) where.rhs, lets, who == Who.cas && where.kind.isEqualityBased()) : eval(where.rhs); switch (who) {