amogh-jahagirdar commented on code in PR #13695:
URL: https://github.com/apache/iceberg/pull/13695#discussion_r2243714790
##########
api/src/main/java/org/apache/iceberg/Metrics.java:
##########
@@ -74,6 +77,25 @@ public Metrics(
this.upperBounds = upperBounds;
}
+ public Metrics(
+ Long rowCount,
+ Map<Integer, Long> columnSizes,
+ Map<Integer, Long> valueCounts,
+ Map<Integer, Long> nullValueCounts,
+ Map<Integer, Long> nanValueCounts,
+ Map<Integer, ByteBuffer> lowerBounds,
+ Map<Integer, ByteBuffer> upperBounds,
+ Map<Integer, Type> originalTypes) {
+ this.rowCount = rowCount;
+ this.columnSizes = columnSizes;
+ this.valueCounts = valueCounts;
+ this.nullValueCounts = nullValueCounts;
+ this.nanValueCounts = nanValueCounts;
+ this.lowerBounds = lowerBounds;
+ this.upperBounds = upperBounds;
+ this.originalTypes = originalTypes;
Review Comment:
Nit: Shall we update the existing constructors to call `this(...)` and pass
in nulls for the extraneous arguments?
##########
core/src/main/java/org/apache/iceberg/DataFiles.java:
##########
@@ -151,6 +152,7 @@ public static class Builder {
private Map<Integer, Long> nanValueCounts = null;
private Map<Integer, ByteBuffer> lowerBounds = null;
private Map<Integer, ByteBuffer> upperBounds = null;
+ private Map<Integer, Type> originalTypes = null;
Review Comment:
So I think the principle here is that anywhere we need to build new
`DataFile` or `DeleteFile` metadata we need to make sure that the metrics
original types information is copied over so that it can be used when
converting to the future structure. The only place that really needs read
exposure to the `originalTypes` is the Metrics class itself.
But I think I agree with @pvary, I do think the field does need to be copied
over (we don't need to expose it neccesarily). I think the field needs to be
copied over for the DeleteFile case since DeleteFile extends BaseFile and we
could have stats for those that we would want to convert
--
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]
