aokolnychyi commented on code in PR #11620: URL: https://github.com/apache/iceberg/pull/11620#discussion_r1884369789
########## core/src/main/java/org/apache/iceberg/SerializableTable.java: ########## @@ -158,6 +160,21 @@ public Map<String, String> properties() { return properties; } + public int formatVersion() { + return formatVersion; + } + + private int formatVersion(Table table) { + if (table instanceof HasTableOperations) { + return ((HasTableOperations) table).operations().current().formatVersion(); Review Comment: Optional: What about an extra variable for `TableOperations`? We have an explicit casts and 3 method invocations on a single line. An extra variable may be easier to read and would be consistent with `metadataFileLocation`. ########## core/src/main/java/org/apache/iceberg/SerializableTable.java: ########## @@ -158,6 +160,21 @@ public Map<String, String> properties() { return properties; } + public int formatVersion() { + return formatVersion; + } + + private int formatVersion(Table table) { + if (table instanceof HasTableOperations) { + return ((HasTableOperations) table).operations().current().formatVersion(); + } else { + // formatVersion=-1 will never be used/returned, because Review Comment: Optional: We may actually avoid the override by doing something like below. ``` public int formatVersion() { if (formatVersion == UNKNOWN_FORMAT_VERSION) { throw new UnsupportedOperationException("Format version is unknown"); } return formatVersion; } private int formatVersion(Table table) { if (table instanceof HasTableOperations) { ... } else { return UNKNOWN_FORMAT_VERSION; } } ``` This would also cover cases for regular tables that didn't have `TableOperations` under the hood. -- 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