stevenzwu commented on code in PR #8803:
URL: https://github.com/apache/iceberg/pull/8803#discussion_r1373440573
##########
core/src/main/java/org/apache/iceberg/BaseFile.java:
##########
@@ -504,6 +508,27 @@ private static Map<Integer, ByteBuffer>
toReadableByteBufferMap(Map<Integer, Byt
}
}
+ private static <TypeT> Map<Integer, TypeT> filterColumnsStats(
+ Map<Integer, TypeT> map, Set<Integer> columnIds) {
+ if (columnIds == null || columnIds.isEmpty()) {
+ return SerializableMap.copyOf(map);
+ }
+
+ if (map == null) {
+ return null;
+ }
+
+ Map<Integer, TypeT> filtered =
Maps.newHashMapWithExpectedSize(columnIds.size());
+ for (Integer columnId : columnIds) {
+ TypeT value = map.get(columnId);
+ if (value != null) {
Review Comment:
thanks for the unit test code. I am still not comfortable of making the
assumption as the Iceberg spec clearly said null stats value is possible. what
if Trino parquet writer produces min/max stats with null value.
```
Lower bound for the non-null, non-NaN values in the partition field, or null
if all values are null or NaN [2]
```
As least, there is a mismatch in the Parquet writer implementation and the
spec. Unless we are going to clarify/update the spec, I feel it is safer to do
`map.containsKey(key)`.
Maybe @RussellSpitzer and @rdblue can chime in here.
--
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]
