uros-b commented on code in PR #17762:
URL: https://github.com/apache/iceberg/pull/17762#discussion_r3839624087
##########
core/src/main/java/org/apache/iceberg/MetadataLogEntriesTable.java:
##########
@@ -59,18 +68,46 @@ public Schema schema() {
private DataTask task(TableScan scan) {
TableMetadata current = table().operations().current();
+
List<TableMetadata.MetadataLogEntry> metadataLogEntries =
Lists.newArrayList(current.previousFiles().listIterator());
+
metadataLogEntries.add(
new TableMetadata.MetadataLogEntry(
current.lastUpdatedMillis(), current.metadataFileLocation()));
+
+ Schema projectedSchema = scan.schema();
+ boolean shouldLoadSnapshotDetails =
+ projectedSchema.findField(LATEST_SNAPSHOT_ID.fieldId()) != null
+ || projectedSchema.findField(LATEST_SCHEMA_ID.fieldId()) != null
+ || projectedSchema.findField(LATEST_SEQUENCE_NUMBER.fieldId()) !=
null;
+
return StaticDataTask.of(
table().io().newInputFile(current.metadataFileLocation()),
schema(),
- scan.schema(),
+ projectedSchema,
metadataLogEntries,
metadataLogEntry ->
- MetadataLogEntriesTable.metadataLogEntryToRow(metadataLogEntry,
table()));
+ metadataLogEntryToRow(metadataLogEntry, current,
shouldLoadSnapshotDetails));
+ }
+
+ private Snapshot latestSnapshotForEntry(
+ TableMetadata.MetadataLogEntry metadataLogEntry, TableMetadata current) {
+ // Resolve snapshot details from the metadata file represented by this
entry because snapshots
+ // may have been removed from the current table history after snapshot
expiration.
+ TableMetadata metadata =
+ metadataLogEntry.file().equals(current.metadataFileLocation())
+ ? current
+ : TableMetadataParser.read(table().io(), metadataLogEntry.file());
Review Comment:
TableMetadataParser.read(table().io(), metadataLogEntry.file()) is
unguarded. When a snapshot column is projected, any log entry whose metadata
file is missing/inaccessible now throws (NotFoundException /
RuntimeIOException) and fails the ENTIRE metadata_log_entries scan; the
previous code touched no historical files and degraded gracefully to per-row
null. This is NOT reachable under normal operation:
CatalogUtil.deleteRemovedMetadataFiles deletes only files pruned OUT of the log
(base.previousFiles() minus metadata.previousFiles()), so files still
referenced by the log are retained even with
write.metadata.delete-after-commit.enabled=true — the failure requires
manual/unsupported deletion of a still-referenced file or an already-corrupt
table. Kept as a (minor, non-blocking) Concern because it regresses the prior
graceful-degradation contract on a debugging/observability table and the remedy
is a one-line try/catch returning null with a warn (or a documented failure
mode); a c
ommitter would reasonably request one before merge.
--
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]
