rdblue commented on a change in pull request #2048:
URL: https://github.com/apache/iceberg/pull/2048#discussion_r556073685
##########
File path: core/src/main/java/org/apache/iceberg/hadoop/HadoopTables.java
##########
@@ -303,6 +304,9 @@ public Table create() {
}
Map<String, String> properties = propertiesBuilder.build();
+ // Validate the metrics
+ MetricsConfig.fromProperties(properties).validateProperties(schema);
Review comment:
After thinking about this a bit more, I don't think it is a good idea to
add the check in the `TableMetadata` constructor after all. The problem with
doing that is that it would break existing tables because they would validate
when loading a table. Instead, we need to validate each case where a table is
created so we could put it in the `newTableMetadata` factory method instead.
That would mean we also need to keep the validation in the property and
schema update operations as you already have in this PR.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
