[GitHub] [iceberg] RussellSpitzer commented on a diff in pull request #5376: Core: Add readable metrics columns to files metadata tables

[GitBox <gi...@apache.org>]

RussellSpitzer commented on code in PR #5376:
URL: https://github.com/apache/iceberg/pull/5376#discussion_r1014038296


##########
api/src/main/java/org/apache/iceberg/DataFile.java:
##########
@@ -99,10 +99,24 @@ public interface DataFile extends ContentFile<DataFile> {
       optional(140, "sort_order_id", IntegerType.get(), "Sort order ID");
   Types.NestedField SPEC_ID = optional(141, "spec_id", IntegerType.get(), "Partition spec ID");
 
+  Types.StructType READABLE_METRICS_VALUE_TYPE =
+      Types.StructType.of(
+          optional(142, "column_size", Types.LongType.get(), "Total size on disk"),
+          optional(143, "value_count", Types.LongType.get(), "Total count, including null and NaN"),
+          optional(144, "null_value_count", Types.LongType.get(), "Null value count"),
+          optional(145, "nan_value_count", Types.LongType.get(), "NaN value count"),
+          optional(146, "lower_bound", Types.StringType.get(), "Lower bound in string form"),

Review Comment:
   Is it crazy for us to have a different struct for every value type so we can return structs with the actual bound types? Probably ... just thinking about this a little. We could define them programmatically so it wouldn't be so bad? I guess strings are probably fine since most folks will be reading these values with human eyes and not using them in another system. 



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org