stoty commented on code in PR #5955:
URL: https://github.com/apache/hbase/pull/5955#discussion_r1623767274
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterListWithAND.java:
##########
@@ -206,17 +242,23 @@ 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]) {
+ // If any of the hinting filters has returned false, then we must not
filter this rowkey.
Review Comment:
That's right, hintingFilters is just caching if a filter is a HintingFilter,
it's just a performance aid to reduce instanceof calls.
We know that filterRowKey() has returned false, becasue we are in the else
clause of the if clause which checks the return value.
We do want the case where filterRowKey() has returned false.
if filterRowKey() returns true, then we skip the whole rowkey.
If it returns false, then the we call filterCell() on the individual cells
(which is what we want in this case)
However, I realize that the patch regresses the filterAllRemaining() case
performance.
If any of the filters returns true for filterAllRemaining, then we can just
skip the row. I will update the patch.
--
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]
