blerer commented on code in PR #3301:
URL: https://github.com/apache/cassandra/pull/3301#discussion_r1598472034
##########
src/java/org/apache/cassandra/cql3/Operator.java:
##########
@@ -266,6 +266,59 @@ public boolean canBeUsedWith(ColumnsExpression.Kind kind)
return kind != ColumnsExpression.Kind.MAP_ELEMENT;
}
},
+ BETWEEN(16)
+ {
+ @Override
+ public String toString()
+ {
+ return "BETWEEN";
+ }
+
+ @Override
+ public boolean isSatisfiedBy(AbstractType<?> type, ByteBuffer
leftOperand, ByteBuffer rightOperand)
+ {
+ ListSerializer<?> serializer = ListType.getInstance(type,
false).getSerializer();
+
+ ByteBuffer leftBound = serializer.getElement(rightOperand, 0);
+ ByteBuffer rightBound = serializer.getElement(rightOperand, 1);
+
+ ByteBuffer larger = type.compareForCQL(leftBound, rightBound) > 0
? leftBound : rightBound;
+ ByteBuffer smaller = type.compareForCQL(larger, leftOperand) == 0
? rightBound : leftBound;
+
+ return type.compareForCQL(leftOperand, larger) <= 0 &&
type.compareForCQL(leftOperand, smaller) >= 0;
+ }
+
+ @Override
+ public boolean requiresFilteringOrIndexingFor(ColumnMetadata.Kind
columnKind)
+ {
+ return !columnKind.isPrimaryKeyKind();
+ }
+
+ @Override
+ public void restrict(RangeSet<ClusteringElements> rangeSet,
List<ClusteringElements> args)
+ {
+ assert args.size() == 2 : this + " accepts exactly two values";
+ ClusteringElements left = args.get(0);
+ ClusteringElements right = args.get(1);
+ int comp = ClusteringElements.compareForCQL(left, right);
Review Comment:
I would use a `Comparator` instead of a method as it would allow you to
simply sort the `List`.
##########
src/java/org/apache/cassandra/cql3/restrictions/ClusteringElements.java:
##########
@@ -370,6 +370,65 @@ public int compareTo(ClusteringElements that)
: that instanceof
Top ? -1 : 1;
}
+ // Very similar to compareTo, the differences are: compareForCQL is
static, compareForCQL unwraps column types
+ // before comparing with them.
+ public static int compareForCQL(ClusteringElements a, ClusteringElements b)
Review Comment:
CQL values cannot be represented as bounds (`Top` or `Bottom`) so you should
reject those. That should allow you to simplify the logic greatly.
Do not forget to add tests in` ClusteringElementsTest`
##########
src/java/org/apache/cassandra/cql3/Operator.java:
##########
@@ -266,6 +266,59 @@ public boolean canBeUsedWith(ColumnsExpression.Kind kind)
return kind != ColumnsExpression.Kind.MAP_ELEMENT;
}
},
+ BETWEEN(16)
Review Comment:
I did not thought about it before but I am guessing that `BETWEEN` is
breaking the `toString` method of `Relation` and `SimpleRestriction`.
`RelationTest` contains some tests for the `Relation` method.
##########
src/java/org/apache/cassandra/cql3/Operator.java:
##########
@@ -266,6 +266,59 @@ public boolean canBeUsedWith(ColumnsExpression.Kind kind)
return kind != ColumnsExpression.Kind.MAP_ELEMENT;
}
},
+ BETWEEN(16)
+ {
+ @Override
+ public String toString()
+ {
+ return "BETWEEN";
+ }
+
+ @Override
+ public boolean isSatisfiedBy(AbstractType<?> type, ByteBuffer
leftOperand, ByteBuffer rightOperand)
+ {
+ ListSerializer<?> serializer = ListType.getInstance(type,
false).getSerializer();
+
+ ByteBuffer leftBound = serializer.getElement(rightOperand, 0);
+ ByteBuffer rightBound = serializer.getElement(rightOperand, 1);
+
+ ByteBuffer larger = type.compareForCQL(leftBound, rightBound) > 0
? leftBound : rightBound;
+ ByteBuffer smaller = type.compareForCQL(larger, leftOperand) == 0
? rightBound : leftBound;
+
+ return type.compareForCQL(leftOperand, larger) <= 0 &&
type.compareForCQL(leftOperand, smaller) >= 0;
Review Comment:
The logic could be simplified by using `ListType.unpack`:
```
List<ByteBuffer> buffers = ListType.getInstance(type,
false).unpack(rightOperand);
Collections.sort(buffers, type);
return type.compareForCQL(leftOperand, buffers.get(1)) <= 0 &&
type.compareForCQL(leftOperand, buffers.get(0)) >= 0;
```
--
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]
