szehon-ho commented on a change in pull request #3959:
URL: https://github.com/apache/iceberg/pull/3959#discussion_r799928644
##########
File path: core/src/main/java/org/apache/iceberg/MetricsConfig.java
##########
@@ -94,15 +96,21 @@ public static MetricsConfig getDefault() {
**/
@Deprecated
public static MetricsConfig fromProperties(Map<String, String> props) {
- return from(props, null);
+ return from(props, null, DEFAULT_WRITE_METRICS_MODE_DEFAULT);
}
/**
* Creates a metrics config from a table.
* @param table iceberg table
*/
public static MetricsConfig forTable(Table table) {
- return from(table.properties(), table.sortOrder());
+ String defaultMode;
+ if (table.schema().columns().size() <= MAX_COLUMNS) {
Review comment:
Took another look, and actually it's a bit messy to make this a catalog
property in the current code (the Table is serialized and sent to writers, but
Catalog is not..).
Was thinking it makes sense as a table property as well (ie, if i know this
table is worth optimizing, set the max columns to be higher). May be a bit
awkward to have a different catalog just for this. And if user really wants to
have one global setting, this change is coming:
https://github.com/apache/iceberg/pull/4011
--
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]
