laskoviymishka commented on code in PR #16859:
URL: https://github.com/apache/iceberg/pull/16859#discussion_r3843389313
##########
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();
+ }
Review Comment:
Nice — these two cover the cheap cases well. One more scenario worth a look,
not blocking: on a `SELECT *`, or a filter like `WHERE properties['x'] = 'y'`,
neither optimization kicks in, so a table with deep metadata history ends up
reading and parsing every historical `metadata.json`, one at a time. That's
roughly the shape a monitoring job polling this table would hit. Might be worth
a javadoc note about the cost at least, or resolving the metadata once per scan
— wdyt?
--
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]
