RussellSpitzer commented on code in PR #11947: URL: https://github.com/apache/iceberg/pull/11947#discussion_r1915353274
########## core/src/main/java/org/apache/iceberg/TableMetadataParser.java: ########## @@ -352,6 +352,7 @@ public static TableMetadata fromJson(String metadataLocation, JsonNode node) { ImmutableList.Builder<Schema> builder = ImmutableList.builder(); for (JsonNode schemaNode : schemaArray) { Schema current = SchemaParser.fromJson(schemaNode); + Schema.checkCompatibility(current, formatVersion); Review Comment: So this means we now have 2 different code paths which check compatibility TableMetadataParser -> Check Compatability -> Table Metadata Constructor TableMetadata.Builder -> Check Compatability -> Build -> Table Metadata Constructor I'm wondering if we should move both checks into the Constructor ... at the moment I think it's probably fine to add another check here. I'm just thinking long term -- 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