tomtongue commented on code in PR #16859:
URL: https://github.com/apache/iceberg/pull/16859#discussion_r3854624011
##########
core/src/main/java/org/apache/iceberg/MetadataLogEntriesTable.java:
##########
@@ -119,6 +140,32 @@ private static StaticDataTask.Row metadataLogEntryToRow(
// latest snapshot in this file corresponding to the log entry
latestSnapshotId,
latestSnapshot != null ? latestSnapshot.schemaId() : null,
- latestSnapshot != null ? latestSnapshot.sequenceNumber() : null);
+ latestSnapshot != null ? latestSnapshot.sequenceNumber() : null,
+ properties);
+ }
+
+ private static Map<String, String> tablePropertiesResolver(
+ TableMetadata.MetadataLogEntry metadataLogEntry,
+ FileIO io,
+ TableMetadata current,
+ boolean skipPropertiesLoad) {
+
+ // Avoid loading metadata file when properties are not projected.
+ if (skipPropertiesLoad) {
+ return null;
+ }
+
+ // Reuse the already loaded current metadata.
+ if (metadataLogEntry.file().equals(current.metadataFileLocation())) {
+ return current.properties();
+ }
+
+ try {
+ return TableMetadataParser.read(io,
metadataLogEntry.file()).properties();
+ } catch (NotFoundException e) {
Review Comment:
Thanks for calling this out. I chose the current behavior and I think it
looks appropriate. Missing historical files can be expected due to metadata
cleanup or else, so returning NULL with a warning is reasonable. A
`RuntimeIOException` indicates corruption or an access issue, so it should
surface instead of being treated as a missing file. I’ll keep the current
behavior as is, if there's no other issues.
--
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]
