anoopj commented on code in PR #17656:
URL: https://github.com/apache/iceberg/pull/17656#discussion_r3785455800


##########
parquet/src/test/java/org/apache/iceberg/parquet/TestDictionaryRowGroupFilter.java:
##########
@@ -462,12 +462,17 @@ SCHEMA, notStartsWith("no_stats", 
UUID.randomUUID().toString()))
     shouldRead =
         new ParquetDictionaryRowGroupFilter(SCHEMA, 
notStartsWith("some_nulls", "some"))
             .shouldRead(parquetSchema, rowGroupMetadata, dictionaryStore);
-    assertThat(shouldRead).as("Should skip: no match in dictionary").isFalse();
+    assertThat(shouldRead).as("Should read: null values do not start with the 
prefix").isTrue();
 
     shouldRead =
         new ParquetDictionaryRowGroupFilter(SCHEMA, notStartsWith("no_nulls", 
"xxx"))
             .shouldRead(parquetSchema, rowGroupMetadata, dictionaryStore);
     assertThat(shouldRead).as("Should read: dictionary contains a matching 
entry").isTrue();
+
+    shouldRead =
+        new ParquetDictionaryRowGroupFilter(SCHEMA, notStartsWith("no_nulls", 
""))

Review Comment:
   I suspect this assertion might be passing without the fix also.  `no_nulls` 
is written as "" for every row, so mayContainNull returns false, so the if 
condition you added might not fire.  But I think the  
`notStartsWith("some_nulls", "some"` line above might be catching the issue. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to