HBASE-18368 FilterList with multiple FamilyFilters concatenated by OR does not work
Signed-off-by: Guanghao Zhang <zg...@apache.org> Project: http://git-wip-us.apache.org/repos/asf/hbase/repo Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/b5896b7a Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/b5896b7a Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/b5896b7a Branch: refs/heads/HBASE-18410 Commit: b5896b7a45b5e2324dc5d3e5fbd775c3e784caf5 Parents: a17094f Author: huzheng <open...@gmail.com> Authored: Tue Oct 17 19:25:23 2017 +0800 Committer: zhangduo <zhang...@apache.org> Committed: Tue Oct 24 11:39:31 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/b5896b7a/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/b5896b7a/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/b5896b7a/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/b5896b7a/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());
