dcapwell commented on PR #4412:
URL: https://github.com/apache/cassandra/pull/4412#issuecomment-3741329107
took awhile but found the issue
```
diff --git a/src/java/org/apache/cassandra/cql3/Operator.java
b/src/java/org/apache/cassandra/cql3/Operator.java
index 63b78a4091..89ff700310 100644
--- a/src/java/org/apache/cassandra/cql3/Operator.java
+++ b/src/java/org/apache/cassandra/cql3/Operator.java
@@ -788,8 +788,10 @@ public enum Operator
public boolean isSatisfiedBy(AbstractType<?> type, ByteBuffer
leftOperand, ByteBuffer rightOperand)
{
List<ByteBuffer> buffers = ListType.getInstance(type,
false).unpack(rightOperand);
- // We use compare instead of compareForCQL to deal properly
with reversed clustering columns
- return type.compare(leftOperand, buffers.get(0)) >= 0 &&
type.compare(leftOperand, buffers.get(1)) <= 0;
+ // Use unwrapped type for comparison to handle reversed
clustering columns correctly.
+ // BETWEEN semantics are always in logical (non-reversed)
order: value >= low AND value <= high.
+ AbstractType<?> unwrapped = type.unwrap();
+ return unwrapped.compare(leftOperand, buffers.get(0)) >= 0 &&
unwrapped.compare(leftOperand, buffers.get(1)) <= 0;
}
@Override
diff --git a/src/java/org/apache/cassandra/index/sai/plan/Expression.java
b/src/java/org/apache/cassandra/index/sai/plan/Expression.java
index a5cf07e35d..6f8bac0bb3 100644
--- a/src/java/org/apache/cassandra/index/sai/plan/Expression.java
+++ b/src/java/org/apache/cassandra/index/sai/plan/Expression.java
@@ -246,8 +246,8 @@ public abstract class Expression
Value first = new Value(buffers.get(0), indexTermType);
Value second = new Value(buffers.get(1), indexTermType);
- // SimpleRestriction#addToRowFilter() ensures correct
bounds ordering, but SAI enforces a non-arbitrary
- // ordering between IPv4 and IPv6 addresses, so correction
may still be necessary.
+ // BETWEEN values are not sorted by SimpleRestriction to
preserve semantics (BETWEEN 5 AND 2 is invalid).
+ // SAI enforces a non-arbitrary ordering between IPv4 and
IPv6 addresses, so correction may still be necessary.
boolean outOfOrder = indexTermType.compare(first.encoded,
second.encoded) > 0;
lower = new Bound(outOfOrder ? second : first, true);
upper = new Bound(outOfOrder ? first : second, true);
(END)
```
`Operator.isSatisfiedBy` was not correctly working when the column is
reversed. The other part of the diff is fixing a SAI comment as its no longer
valid with this patch.
fuzz tests are passing without issue but i think this points out that slice
queries are very under tested with BETWEEN, so tomorrow ill try to enhance the
fuzz tests to make that better
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
