Mailing-List: contact commits-help@hbase.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@hbase.apache.org
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
From: zhangduo@apache.org
To: commits@hbase.apache.org
Date: Wed, 25 Oct 2017 12:37:42 -0000
Message-Id: <31b6a54f34ac4f1db3f5b6c5ae364e60@git.apache.org>
In-Reply-To: <7c5df1279e8742e980fb9bd71dadd1e3@git.apache.org>
References: <7c5df1279e8742e980fb9bd71dadd1e3@git.apache.org>
Subject: [21/23] hbase git commit: HBASE-18368 FilterList with multiple
 FamilyFilters concatenated by OR does not work
archived-at: Wed, 25 Oct 2017 12:37:27 -0000

HBASE-18368 FilterList with multiple FamilyFilters concatenated by OR does not work

Signed-off-by: Guanghao Zhang <zghao@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/fcaf71d2
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/fcaf71d2
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/fcaf71d2

Branch: refs/heads/HBASE-18410
Commit: fcaf71d20687e350d9357e1a303da6caf4c6ac08
Parents: 8a5114b
Author: huzheng <openinx@gmail.com>
Authored: Tue Oct 17 19:25:23 2017 +0800
Committer: zhangduo <zhangduo@apache.org>
Committed: Wed Oct 25 20:36:19 2017 +0800

----------------------------------------------------------------------
 .../org/apache/hadoop/hbase/filter/Filter.java  | 10 +++++---
 .../hadoop/hbase/filter/FilterListWithOR.java   | 10 ++++++--
 .../hadoop/hbase/filter/TestFilterList.java     | 26 ++++++++++++++++++++
 .../hbase/filter/TestFilterListOnMini.java      |  7 +++---
 4 files changed, 44 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/fcaf71d2/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/Filter.java
----------------------------------------------------------------------
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/Filter.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/Filter.java
index 70c68b6..a92ea0b 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/Filter.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/Filter.java
@@ -172,8 +172,12 @@ public abstract class Filter {
      */
     NEXT_COL,
     /**
-     * Done with columns, skip to next row. Note that filterRow() will
-     * still be called.
+     * Seek to next row in current family. It may still pass a cell whose family is different but
+     * row is the same as previous cell to {@link #filterKeyValue(Cell)} , even if we get a NEXT_ROW
+     * returned for previous cell. For more details see HBASE-18368. <br>
+     * Once reset() method was invoked, then we switch to the next row for all family, and you can
+     * catch the event by invoking CellUtils.matchingRows(previousCell, currentCell). <br>
+     * Note that filterRow() will still be called. <br>
      */
     NEXT_ROW,
     /**
@@ -181,7 +185,7 @@ public abstract class Filter {
      */
     SEEK_NEXT_USING_HINT,
     /**
-     * Include KeyValue and done with row, seek to next.
+     * Include KeyValue and done with row, seek to next. See NEXT_ROW.
      */
     INCLUDE_AND_SEEK_NEXT_ROW,
 }

http://git-wip-us.apache.org/repos/asf/hbase/blob/fcaf71d2/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterListWithOR.java
----------------------------------------------------------------------
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterListWithOR.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterListWithOR.java
index bac9023..31e2a55 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterListWithOR.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterListWithOR.java
@@ -74,7 +74,12 @@ public class FilterListWithOR extends FilterListBase {
    * as the previous cell even if filter-A has NEXT_COL returned for the previous cell. So we should
    * save the previous cell and the return code list when checking previous cell for every filter in
    * filter list, and verify if currentCell fit the previous return code, if fit then pass the
-   * currentCell to the corresponding filter. (HBASE-17678)
+   * currentCell to the corresponding filter. (HBASE-17678) <br>
+   * Note that: In StoreScanner level, NEXT_ROW will skip to the next row in current family, and in
+   * RegionScanner level, NEXT_ROW will skip to the next row in current family and switch to the
+   * next family for RegionScanner, INCLUDE_AND_NEXT_ROW is the same. so we should pass current cell
+   * to the filter, if row mismatch or row match but column family mismatch. (HBASE-18368)
+   * @see org.apache.hadoop.hbase.filter.Filter.ReturnCode
    */
   private boolean shouldPassCurrentCellToFilter(Cell prevCell, Cell currentCell, int filterIdx)
       throws IOException {
@@ -94,7 +99,8 @@ public class FilterListWithOR extends FilterListBase {
       return !CellUtil.matchingRowColumn(prevCell, currentCell);
     case NEXT_ROW:
     case INCLUDE_AND_SEEK_NEXT_ROW:
-      return !CellUtil.matchingRows(prevCell, currentCell);
+      return !CellUtil.matchingRows(prevCell, currentCell)
+          || !CellUtil.matchingFamily(prevCell, currentCell);
     default:
       throw new IllegalStateException("Received code is not valid.");
     }

http://git-wip-us.apache.org/repos/asf/hbase/blob/fcaf71d2/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFilterList.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFilterList.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFilterList.java
index 4c67c59..d420b02 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFilterList.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFilterList.java
@@ -936,5 +936,31 @@ public class TestFilterList {
     assertEquals(ReturnCode.INCLUDE, filterList.filterKeyValue(kv));
     assertEquals(kv, filterList.transformCell(kv));
   }
+
+  private static class MockNextRowFilter extends FilterBase {
+    private int hitCount = 0;
+
+    public ReturnCode filterKeyValue(Cell v) throws IOException {
+      hitCount++;
+      return ReturnCode.NEXT_ROW;
+    }
+
+    public int getHitCount() {
+      return hitCount;
+    }
+  }
+
+  @Test
+  public void testRowCountFilter() throws IOException {
+    KeyValue kv1 = new KeyValue(Bytes.toBytes("row"), Bytes.toBytes("fam1"), Bytes.toBytes("a"), 1,
+        Bytes.toBytes("value"));
+    KeyValue kv2 = new KeyValue(Bytes.toBytes("row"), Bytes.toBytes("fam2"), Bytes.toBytes("a"), 2,
+        Bytes.toBytes("value"));
+    MockNextRowFilter mockNextRowFilter = new MockNextRowFilter();
+    FilterList filter = new FilterList(Operator.MUST_PASS_ONE, mockNextRowFilter);
+    filter.filterKeyValue(kv1);
+    filter.filterKeyValue(kv2);
+    assertEquals(2, mockNextRowFilter.getHitCount());
+  }
 }
 

http://git-wip-us.apache.org/repos/asf/hbase/blob/fcaf71d2/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFilterListOnMini.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFilterListOnMini.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFilterListOnMini.java
index 590b26e..2a13ac8 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFilterListOnMini.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFilterListOnMini.java
@@ -36,10 +36,10 @@ import org.junit.experimental.categories.Category;
 import org.junit.rules.TestName;
 
 /**
- * Tests filter Lists in ways that rely on a MiniCluster.
- * Where possible, favor tests in TestFilterList and TestFilterFromRegionSide instead.
+ * Tests filter Lists in ways that rely on a MiniCluster. Where possible, favor tests in
+ * TestFilterList and TestFilterFromRegionSide instead.
  */
-@Category({MediumTests.class, FilterTests.class})
+@Category({ MediumTests.class, FilterTests.class })
 public class TestFilterListOnMini {
 
   private static final Log LOG = LogFactory.getLog(TestFilterListOnMini.class);
@@ -58,7 +58,6 @@ public class TestFilterListOnMini {
     TEST_UTIL.shutdownMiniCluster();
   }
 
-  @Ignore("HBASE-18410 Should not merge without this test running.")
   @Test
   public void testFiltersWithOR() throws Exception {
     TableName tn = TableName.valueOf(name.getMethodName());