RussellSpitzer commented on code in PR #18657:
URL: https://github.com/apache/iceberg/pull/18657#discussion_r4402404883
##########
spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/source/SparkScan.java:
##########
@@ -234,12 +235,14 @@ protected Statistics estimateStatistics(Snapshot
snapshot) {
Map<Integer, List<BlobMetadata>> groupedByField =
metadataList.stream()
+ .filter(metadata -> !metadata.fields().isEmpty())
.collect(
Collectors.groupingBy(
metadata -> metadata.fields().get(0),
Collectors.toList()));
+ Schema snapshotSchema = SnapshotUtil.schemaFor(table,
snapshot.snapshotId());
for (Map.Entry<Integer, List<BlobMetadata>> entry :
groupedByField.entrySet()) {
- String colName = table.schema().findColumnName(entry.getKey());
+ String colName = snapshotSchema.findColumnName(entry.getKey());
Review Comment:
That's a great point, It definitely makes sense to use the scan schema
(which would cover this ambiguity) instead of trying to guess the scan schema
by just pulling the snapshot's schema. @rajarshisarkar we can fix this on the
port comment (and in the 3.5 module) or we can do it as a followup.
So we basically tackle two problems
1. We use the scanSchema so we handle time travel correctly
2. We also skip fields we don't understand because to be safe about schema
old changes with drops
--
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]
