amogh-jahagirdar commented on code in PR #17960:
URL: https://github.com/apache/iceberg/pull/17960#discussion_r3938237100


##########
core/src/main/java/org/apache/iceberg/MetricsConfig.java:
##########
@@ -315,6 +321,21 @@ public static MetricsConfig from(Map<String, String> 
props, Schema schema, SortO
       }
     }
 
+    // configure metrics collection for partition fields ignoring any overrides
+    if (spec != null && schema != null) {
+      for (PartitionField field : spec.fields()) {
+        if (!field.transform().preservesOrder()) {
+          continue;
+        }
+
+        String name = schema.findColumnName(field.sourceId());
+        if (name != null) {
+          columnModes.put(name, MetricsModes.Full.get());

Review Comment:
   Parquet technically allows for a configurable truncation for an entire file 
(not per column) but it defaults to Int max so we're OK for now. 
https://github.com/apache/parquet-java/blob/master/parquet-column/src/main/java/org/apache/parquet/column/ParquetProperties.java#L60.
 Since there's no API for finer grained guarantees from Parquet , we 
practically rely on  the default from the parquet java library.
   
   What we can do separately is add a unit test which just writes a big 
string/binary and in case the Parquet library changes out from under us, tests 
would fail and we would know. I didn't want to pollute this PR with that.



-- 
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