Apache9 commented on code in PR #5955:
URL: https://github.com/apache/hbase/pull/5955#discussion_r1622491040
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterListWithAND.java:
##########
@@ -206,17 +238,20 @@ public boolean filterRowKey(Cell firstRowCell) throws
IOException {
if (isEmpty()) {
return super.filterRowKey(firstRowCell);
}
- boolean retVal = false;
+ boolean anyFiltered = false;
+ boolean anyHintingPassed = false;
for (int i = 0, n = filters.size(); i < n; i++) {
Filter filter = filters.get(i);
if (filter.filterAllRemaining() || filter.filterRowKey(firstRowCell)) {
// Can't just return true here, because there are some filters (such
as PrefixFilter) which
// will catch the row changed event by filterRowKey(). If we return
early here, those
// filters will have no chance to update their row state.
- retVal = true;
+ anyFiltered = true;
+ } else if (hintingFilters[i]) {
+ anyHintingPassed = true;
Review Comment:
Why we need this logic here?
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterListWithAND.java:
##########
@@ -184,9 +205,20 @@ public ReturnCode filterCell(Cell c) throws IOException {
// otherwise we may mess up the global state (such as offset, count..)
in the following
// sub-filters. (HBASE-20565)
if (!isIncludeRelatedReturnCode(rc)) {
- return rc;
+ i++;
Review Comment:
Ditto.
I think the intention here is that, usually SEEK_NEXT_USING_HINT can skip
more rows, so even if we decide to skip the current cell, we'd better still
looking to remaining filters to see if we can get a SEEK_NEXT_USING_HINT.
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterListWithAND.java:
##########
@@ -169,10 +186,14 @@ public ReturnCode filterCell(Cell c) throws IOException {
}
ReturnCode rc = ReturnCode.INCLUDE;
this.seekHintFilters.clear();
- for (int i = 0, n = filters.size(); i < n; i++) {
+ int i = 0;
+ int n = filters.size();
+ for (; i < n; i++) {
Filter filter = filters.get(i);
if (filter.filterAllRemaining()) {
- return ReturnCode.NEXT_ROW;
+ rc = ReturnCode.NEXT_ROW;
Review Comment:
Let's add more comments here?
The implementation is already very complicated...
--
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]
