HBASE-18879 HBase FilterList cause KeyOnlyFilter not work

hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterList.java

@@ -72,6 +72,8 @@ final public class FilterList extends FilterBase {
       filterListBase = new FilterListWithAND(filters);
     } else if (operator == Operator.MUST_PASS_ONE) {
       filterListBase = new FilterListWithOR(filters);
+    } else {
+      throw new IllegalArgumentException("Invalid operator: " + operator);
     }
     this.operator = operator;
   }
@@ -168,6 +170,10 @@ final public class FilterList extends FilterBase {
     return filterListBase.transformCell(c);
   }
 
+  ReturnCode internalFilterKeyValue(Cell c, Cell currentTransformedCell) throws IOException {
+    return this.filterListBase.internalFilterKeyValue(c, currentTransformedCell);
+  }
+
   @Override
   public ReturnCode filterKeyValue(Cell c) throws IOException {
     return filterListBase.filterKeyValue(c);

hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterListBase.java

@@ -107,6 +107,9 @@ public abstract class FilterListBase extends FilterBase {
     return cell;
   }
 
+  abstract ReturnCode internalFilterKeyValue(Cell c, Cell currentTransformedCell)
+      throws IOException;
+
   /**
    * Filters that never filter by modifying the returned List of Cells can inherit this
    * implementation that does nothing. {@inheritDoc}

hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterListWithAND.java

@@ -147,16 +147,26 @@ public class FilterListWithAND extends FilterListBase {
         "Received code is not valid. rc: " + rc + ", localRC: " + localRC);
   }
 
-  private ReturnCode filterKeyValueWithMustPassAll(Cell c) throws IOException {
+  @Override
+  ReturnCode internalFilterKeyValue(Cell c, Cell currentTransformedCell) throws IOException {
+    if (isEmpty()) {
+      return ReturnCode.INCLUDE;
+    }
     ReturnCode rc = ReturnCode.INCLUDE;
-    Cell transformed = c;
+    Cell transformed = currentTransformedCell;
+    this.referenceCell = c;
     this.seekHintFilter.clear();
     for (int i = 0, n = filters.size(); i < n; i++) {
       Filter filter = filters.get(i);
       if (filter.filterAllRemaining()) {
         return ReturnCode.NEXT_ROW;
       }
-      ReturnCode localRC = filter.filterKeyValue(c);
+      ReturnCode localRC;
+      if (filter instanceof FilterList) {
+        localRC = ((FilterList) filter).internalFilterKeyValue(c, transformed);
+      } else {
+        localRC = filter.filterKeyValue(c);
+      }
       rc = mergeReturnCode(rc, localRC);
 
       // For INCLUDE* case, we need to update the transformed cell.
@@ -177,11 +187,7 @@ public class FilterListWithAND extends FilterListBase {
 
   @Override
   public ReturnCode filterKeyValue(Cell c) throws IOException {
-    if (isEmpty()) {
+    return internalFilterKeyValue(c, c);
-      return ReturnCode.INCLUDE;
-    }
-    this.referenceCell = c;
-    return filterKeyValueWithMustPassAll(c);
   }
 
   @Override

hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterListWithOR.java

@@ -235,10 +235,15 @@ public class FilterListWithOR extends FilterListBase {
     }
   }
 
-  private ReturnCode filterKeyValueWithMustPassOne(Cell c) throws IOException {
+  @Override
+  ReturnCode internalFilterKeyValue(Cell c, Cell currentTransformCell) throws IOException {
+    if (isEmpty()) {
+      return ReturnCode.INCLUDE;
+    }
     ReturnCode rc = null;
     boolean everyFilterReturnHint = true;
-    Cell transformed = c;
+    Cell transformed = currentTransformCell;
+    this.referenceCell = c;
     for (int i = 0, n = filters.size(); i < n; i++) {
       Filter filter = filters.get(i);
 
@@ -248,7 +253,12 @@ public class FilterListWithOR extends FilterListBase {
         continue;
       }
 
-      ReturnCode localRC = filter.filterKeyValue(c);
+      ReturnCode localRC;
+      if (filter instanceof FilterList) {
+        localRC = ((FilterList) filter).internalFilterKeyValue(c, transformed);
+      } else {
+        localRC = filter.filterKeyValue(c);
+      }
 
       // Update previous return code and previous cell for filter[i].
       updatePrevFilterRCList(i, localRC);
@@ -280,11 +290,7 @@ public class FilterListWithOR extends FilterListBase {
 
   @Override
   public ReturnCode filterKeyValue(Cell c) throws IOException {
-    if (isEmpty()) {
+    return internalFilterKeyValue(c, c);
-      return ReturnCode.INCLUDE;
-    }
-    this.referenceCell = c;
-    return filterKeyValueWithMustPassOne(c);
   }
 
   @Override

hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFilterList.java

@@ -888,5 +888,53 @@ public class TestFilterList {
     filterList.filterRowKey(KeyValueUtil.createFirstOnRow(r1));
     assertEquals(ReturnCode.NEXT_COL, filterList.filterKeyValue(new KeyValue(r1, r1, r1)));
   }
+
+  @Test
+  public void testKeyOnlyFilterTransformCell() throws IOException {
+    Cell c;
+    KeyValue kv1 = new KeyValue(Bytes.toBytes("row"), Bytes.toBytes("cf"), Bytes.toBytes("column1"),
+        1, Bytes.toBytes("value1"));
+    KeyValue kv2 = new KeyValue(Bytes.toBytes("row"), Bytes.toBytes("cf"), Bytes.toBytes("column1"),
+        2, Bytes.toBytes("value2"));
+
+    Filter filter1 = new SingleColumnValueFilter(Bytes.toBytes("cf"), Bytes.toBytes("column1"),
+        CompareOperator.EQUAL, Bytes.toBytes("value1"));
+    Filter filter2 = new SingleColumnValueFilter(Bytes.toBytes("cf"), Bytes.toBytes("column1"),
+        CompareOperator.EQUAL, Bytes.toBytes("value2"));
+    FilterList internalFilterList = new FilterList(Operator.MUST_PASS_ONE, filter1, filter2);
+
+    FilterList keyOnlyFilterFirst =
+        new FilterList(Operator.MUST_PASS_ALL, new KeyOnlyFilter(), internalFilterList);
+
+    assertEquals(ReturnCode.INCLUDE, keyOnlyFilterFirst.filterKeyValue(kv1));
+    c = keyOnlyFilterFirst.transformCell(kv1);
+    assertEquals(0, c.getValueLength());
+    assertEquals(ReturnCode.INCLUDE, keyOnlyFilterFirst.filterKeyValue(kv2));
+    c = keyOnlyFilterFirst.transformCell(kv2);
+    assertEquals(0, c.getValueLength());
+
+    internalFilterList.reset();
+    FilterList keyOnlyFilterLast =
+        new FilterList(Operator.MUST_PASS_ALL, new KeyOnlyFilter(), internalFilterList);
+    assertEquals(ReturnCode.INCLUDE, keyOnlyFilterLast.filterKeyValue(kv1));
+    c = keyOnlyFilterLast.transformCell(kv1);
+    assertEquals(0, c.getValueLength());
+    assertEquals(ReturnCode.INCLUDE, keyOnlyFilterLast.filterKeyValue(kv2));
+    c = keyOnlyFilterLast.transformCell(kv2);
+    assertEquals(0, c.getValueLength());
+  }
+
+  @Test
+  public void testEmptyFilterListTransformCell() throws IOException {
+    KeyValue kv = new KeyValue(Bytes.toBytes("row"), Bytes.toBytes("cf"), Bytes.toBytes("column1"),
+        1, Bytes.toBytes("value"));
+    FilterList filterList = new FilterList(Operator.MUST_PASS_ALL);
+    assertEquals(ReturnCode.INCLUDE, filterList.filterKeyValue(kv));
+    assertEquals(kv, filterList.transformCell(kv));
+
+    filterList = new FilterList(Operator.MUST_PASS_ONE);
+    assertEquals(ReturnCode.INCLUDE, filterList.filterKeyValue(kv));
+    assertEquals(kv, filterList.transformCell(kv));
+  }
 }