stoty commented on code in PR #6361:
URL: https://github.com/apache/hbase/pull/6361#discussion_r1819243336
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/filter/PrefixFilter.java:
##########
@@ -69,13 +74,19 @@ public boolean filterRowKey(Cell firstRowCell) {
if ((!isReversed() && cmp > 0) || (isReversed() && cmp < 0)) {
passedPrefix = true;
}
- filterRow = (cmp != 0);
+ filterRow = (!isReversed() && cmp > 0) || (isReversed() && cmp < 0);
+ provideHint = (!isReversed() && cmp < 0) || (isReversed() && cmp > 0);
Review Comment:
We only provide a hint.
I'm not sure if the underlaying code guarantees that once we provide a hint,
we don't get any calls that are smaller/larger than the hint we gave.
At the same time, If for some reson the hint is not honored, I think it's
fine to provide the hint again, so sending the hint only once is not a good
idea.
I was thinking about whether it is possible to have a cell that is smaller
than our hint for the reverse case, but does not have the expected prefix. I
think you are right, and there is no such rowkey.
i.e. if the prefix is AAAA, then the hint is AAAB.
The next smallest cell after AAAB is AAAAFFFFFF... , which does have the
AAAA prefix.
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/filter/PrefixFilter.java:
##########
@@ -69,13 +74,19 @@ public boolean filterRowKey(Cell firstRowCell) {
if ((!isReversed() && cmp > 0) || (isReversed() && cmp < 0)) {
passedPrefix = true;
}
- filterRow = (cmp != 0);
+ filterRow = (!isReversed() && cmp > 0) || (isReversed() && cmp < 0);
Review Comment:
This is the same as passedPrefix above. No need to compute twice.
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/filter/PrefixFilter.java:
##########
@@ -142,6 +155,31 @@ boolean areSerializedFieldsEqual(Filter o) {
return Bytes.equals(this.getPrefix(), other.getPrefix());
}
+ @Override
+ public Cell getNextCellHint(Cell cell) {
+ byte[] hintBytes;
+ if (reversed) {
+ // On reversed scan hint should be the prefix with last byte incremented
+ hintBytes = increaseLastNonMaxByte(this.prefix);
Review Comment:
You have a point.
However, the API does not explicitly state that it will honor the hint, so I
think it's better to pre-compute it, in case we have to return the hint several
times.
We lose a few cycles if we never need to return a hint, but we win a lot of
cycles if we have to return it a lot of times.
--
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]
