bereng commented on a change in pull request #1380:
URL: https://github.com/apache/cassandra/pull/1380#discussion_r795387378
##########
File path: src/java/org/apache/cassandra/cql3/conditions/ColumnCondition.java
##########
@@ -531,6 +534,25 @@ private static boolean setOrListAppliesTo(AbstractType<?>
type, Iterator<Cell<?>
return operator == Operator.EQ || operator == Operator.LTE ||
operator == Operator.GTE;
}
+ private static boolean valueAppliesTo(Iterator<Cell<?>> iter,
ByteBuffer value, boolean appliesToSetOrMapKeys)
+ {
+ while(iter.hasNext())
+ {
+ // for lists and map values we use the cell value; for sets
and map keys we use the cell name
+ ByteBuffer cellValue = appliesToSetOrMapKeys ?
iter.next().path().get(0) : iter.next().buffer();
+ int comparison = BytesType.instance.compare(cellValue, value);
Review comment:
Correct that's clear.
But notice how across the file comparisons are done through `type
.compare()` methods and then `evaluateComparisonWithOperator()` is called. In
your solution iiuc you compare ByteBuffers directly through the Bytes type and
the check for `==0` side stepping the logic already existing for those needs.
You don't check for the key or element type and use that API.
I think you should check key/element type to get the right type comparator,
instead of going to a raw BB comparison. And then you could rely on
`evaluateComparisonWithOperator()` to piggyback the already existing logic and
have a single point of responsibility for that.
I would then, although this more a personal cosmetic choice, try to make it
pretty by consolidating everything in an overloaded `compareWithOperator()`.
But this is more involved refactoring etc.
Reading again the short comment I dropped I have to apologize as it was
really misleading lol. I was thinking along the lines of jumping back into the
already existing logics, my bad.
--
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]
