PARQUET-34: Fix DictionaryFilter

Author: clairemcginty

parquet-hadoop/src/main/java/org/apache/parquet/filter2/dictionarylevel/DictionaryFilter.java

@@ -508,16 +508,16 @@ public Boolean visit(Size size) {
     try {
       // We know the block has at least as many array elements as the dictionary sizes
       final Set<?> dict = expandDictionary(meta);
-      if (dict == null) {
+
+      // If the column doesn't have a dictionary encoding, we can't infer anything about its size
+      if (dict == null || dict.isEmpty()) {
         return BLOCK_MIGHT_MATCH;
       }
-      int numDistinctValues = dict.size();
-      final boolean blockCannotMatch = size.filter(
-          (eq) -> eq < numDistinctValues,
-          (lt) -> lt <= numDistinctValues,
-          (lte) -> lte < numDistinctValues,
-          (gt) -> false,
-          (gte) -> false);
+
+      // Column has at least (nonempty) dict.size() values spread across over all records;
+      // predicates that match empty arrays cannot match
+      final boolean blockCannotMatch =
+          size.filter((eq) -> eq == 0, (lt) -> lt == 1, (lte) -> lte <= 1, (gt) -> false, (gte) -> false);
 
       return blockCannotMatch ? BLOCK_CANNOT_MATCH : BLOCK_MIGHT_MATCH;
     } catch (IOException e) {

parquet-hadoop/src/test/java/org/apache/parquet/filter2/dictionarylevel/DictionaryFilterTest.java

@@ -509,16 +509,18 @@ public void testGtEqDouble() throws Exception {
 
   @Test
   public void testSizeBinary() throws Exception {
+    // repeated_binary_field dict has 26 distinct values
     BinaryColumn b = binaryColumn("repeated_binary_field");
 
-    // DictionaryFilter knows that `repeated_binary_field` column has at least 26 element values
-    assertFalse(canDrop(size(b, Operators.Size.Operator.GT, 26), ccmd, dictionaries));
-    assertFalse(canDrop(size(b, Operators.Size.Operator.GTE, 27), ccmd, dictionaries));
-    assertFalse(canDrop(size(b, Operators.Size.Operator.EQ, 27), ccmd, dictionaries));
+    // DictionaryFilter knows that `repeated_binary_field` column has at least 26 element values spread across
+    // records
+    assertTrue(canDrop(size(b, Operators.Size.Operator.EQ, 0), ccmd, dictionaries));
+    assertTrue(canDrop(size(b, Operators.Size.Operator.LT, 1), ccmd, dictionaries));
+    assertTrue(canDrop(size(b, Operators.Size.Operator.LTE, 0), ccmd, dictionaries));
 
-    assertTrue(canDrop(size(b, Operators.Size.Operator.LT, 26), ccmd, dictionaries));
-    assertTrue(canDrop(size(b, Operators.Size.Operator.LTE, 25), ccmd, dictionaries));
-    assertTrue(canDrop(size(b, Operators.Size.Operator.EQ, 25), ccmd, dictionaries));
+    assertFalse(canDrop(size(b, Operators.Size.Operator.EQ, 30), ccmd, dictionaries));
+    assertFalse(canDrop(size(b, Operators.Size.Operator.GT, 0), ccmd, dictionaries));
+    assertFalse(canDrop(size(b, Operators.Size.Operator.GTE, 1), ccmd, dictionaries));
 
     // If column doesn't exist in meta, it should be treated as having size 0
     BinaryColumn nonExistentColumn = binaryColumn("nonexistant_col");