fix: filter out results on no-op filters (#3456)

kolea2 · web-flow · commit 84bf4ca95c0d · 2022-01-28T11:18:20.000-05:00
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -40,10 +40,10 @@ again.
    
    There are also tests that perform compatibility tests against an HBase Minicluster, which can be invoked with the following commands for HBase 1 and HBase 2 respectively: 
    ```sh
-   mvn clean verify -PhbaseLocalMiniClusterTest
+   mvn clean verify -Penable-integration-tests -PhbaseLocalMiniClusterTest
    ```
    ```sh
-   mvn clean verify -PhbaseLocalMiniClusterTestH2
+   mvn clean verify -Penable-integration-tests -PhbaseLocalMiniClusterTestH2
    ```
 
    You can run those commands at the top of the project, or you can run them at the appropriate integration-tests project.  
diff --git a/bigtable-client-core-parent/bigtable-hbase-integration-tests-common/src/test/java/com/google/cloud/bigtable/hbase/AbstractTestFilters.java b/bigtable-client-core-parent/bigtable-hbase-integration-tests-common/src/test/java/com/google/cloud/bigtable/hbase/AbstractTestFilters.java
@@ -1969,7 +1969,7 @@ public void testQualifierBinaryComparatorFilter() throws IOException {
     Filter noOpFilter = new QualifierFilter(CompareOp.NO_OP, binaryComparator);
     Get noOpGet = new Get(rowKey).setFilter(noOpFilter).addFamily(COLUMN_FAMILY);
     Result noOpResult = table.get(noOpGet);
-    Assert.assertEquals(10, noOpResult.size());
+    Assert.assertEquals(0, noOpResult.size());
 
     table.close();
   }
diff --git a/bigtable-client-core-parent/bigtable-hbase/src/main/java/com/google/cloud/bigtable/hbase/adapters/filters/QualifierFilterAdapter.java b/bigtable-client-core-parent/bigtable-hbase/src/main/java/com/google/cloud/bigtable/hbase/adapters/filters/QualifierFilterAdapter.java
@@ -100,9 +100,10 @@ private Filter adaptBinaryComparator(
             .rangeWithinFamily(FilterAdapterHelper.getSingleFamilyName(context))
             .startOpen(value);
       case NO_OP:
-        // No-op always passes. Instead of attempting to return null or default instance,
-        // include an always-match filter.
-        return FILTERS.pass();
+        // No-ops are always filtered out.
+        // See:
+        // https://github.com/apache/hbase/blob/master/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/ColumnValueFilter.java#L127-L138
+        return FILTERS.block();
       default:
         throw new IllegalStateException(
             String.format("Cannot handle unknown compare op %s", compareOp));
diff --git a/bigtable-client-core-parent/bigtable-hbase/src/main/java/com/google/cloud/bigtable/hbase/adapters/filters/ValueFilterAdapter.java b/bigtable-client-core-parent/bigtable-hbase/src/main/java/com/google/cloud/bigtable/hbase/adapters/filters/ValueFilterAdapter.java
@@ -98,9 +98,10 @@ private Filter adaptBinaryComparator(CompareOp compareOp, BinaryComparator compa
       case GREATER:
         return range().startOpen(value);
       case NO_OP:
-        // No-op always passes. Instead of attempting to return null or default instance,
-        // include an always-match filter.
-        return FILTERS.pass();
+        // No-ops are always filtered out.
+        // See:
+        // https://github.com/apache/hbase/blob/master/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/ColumnValueFilter.java#L127-L138
+        return FILTERS.block();
       default:
         throw new IllegalStateException(
             String.format("Cannot handle unknown compare op %s", compareOp));
@@ -116,8 +117,11 @@ private Filter adaptRegexStringComparator(CompareOp compareOp, RegexStringCompar
     switch (compareOp) {
       case EQUAL:
         return FILTERS.value().regex(pattern);
+        // No-ops are always filtered out.
+        // See:
+        // https://github.com/apache/hbase/blob/master/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/ColumnValueFilter.java#L127-L138
       case NO_OP:
-        return FILTERS.pass();
+        return FILTERS.block();
       case LESS:
       case LESS_OR_EQUAL:
       case NOT_EQUAL:

Original file line number	Diff line number	Diff line change
`@@ -1969,7 +1969,7 @@ public void testQualifierBinaryComparatorFilter() throws IOException {`
`1969`	`1969`	`Filter noOpFilter = new QualifierFilter(CompareOp.NO_OP, binaryComparator);`
`1970`	`1970`	`Get noOpGet = new Get(rowKey).setFilter(noOpFilter).addFamily(COLUMN_FAMILY);`
`1971`	`1971`	`Result noOpResult = table.get(noOpGet);`
`1972`		`- Assert.assertEquals(10, noOpResult.size());`
	`1972`	`+ Assert.assertEquals(0, noOpResult.size());`
`1973`	`1973`
`1974`	`1974`	`table.close();`
`1975`	`1975`	`}`