pvary commented on code in PR #12366:
URL: https://github.com/apache/iceberg/pull/12366#discussion_r1965827489


##########
data/src/main/java/org/apache/iceberg/data/GenericAppenderFactory.java:
##########
@@ -91,7 +118,13 @@ public FileAppender<Record> newAppender(OutputFile 
outputFile, FileFormat fileFo
   @Override
   public FileAppender<Record> newAppender(
       EncryptedOutputFile encryptedOutputFile, FileFormat fileFormat) {
-    MetricsConfig metricsConfig = MetricsConfig.fromProperties(config);
+    MetricsConfig metricsConfig;
+    if (table == null) {
+      metricsConfig = MetricsConfig.fromProperties(config);
+    } else {
+      metricsConfig = MetricsConfig.forTable(table);
+    }
+

Review Comment:
   This is a bit questionable for me.
   Do we really want to disregard the config properties specifically set by the 
`set`/`setAll` method if the table has provided?
   We should decide on the expected behavior and make sure that we don't get 
wrong configuration. Maybe by throwing an exception in the set methods when the 
table is set?



-- 
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-unsubscr...@iceberg.apache.org

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


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

Reply via email to