zeroshade commented on code in PR #577:
URL: https://github.com/apache/iceberg-go/pull/577#discussion_r2379789638
##########
table/metadata.go:
##########
@@ -134,6 +134,19 @@ type Metadata interface {
NameMapping() iceberg.NameMapping
LastSequenceNumber() int64
+ // StatisticsFile is an optional list of table statistics.
+ // Table statistics files are valid Puffin files.
+ // StatisticsFile are informational. A reader can choose to ignore
statistics information.
+ // StatisticsFile support is not required to read the table correctly.
+ // A table can contain many statistics files associated with different
table snapshots.
+ Statistics() []StatisticsFile
+ // PartitionStatisticsFile is an optional list of partition statistics
files.
+ // Partition statistics are not required for reading or planning
+ // and readers may ignore them. Each table snapshot may be associated
+ // with at most one partition statistics file. A writer can optionally
+ // write the partition statistics file during each write operation,
+ // or it can also be computed on demand.
+ PartitionStatistics() []PartitionStatisticsFile
Review Comment:
Same as above, should we return an `iter.Seq[PartitionStatisticsFile]`?
##########
table/metadata.go:
##########
@@ -134,6 +134,19 @@ type Metadata interface {
NameMapping() iceberg.NameMapping
LastSequenceNumber() int64
+ // StatisticsFile is an optional list of table statistics.
Review Comment:
```suggestion
// Statistics returns an optional list of table statistics.
```
##########
table/metadata.go:
##########
@@ -134,6 +134,19 @@ type Metadata interface {
NameMapping() iceberg.NameMapping
LastSequenceNumber() int64
+ // StatisticsFile is an optional list of table statistics.
+ // Table statistics files are valid Puffin files.
+ // StatisticsFile are informational. A reader can choose to ignore
statistics information.
+ // StatisticsFile support is not required to read the table correctly.
+ // A table can contain many statistics files associated with different
table snapshots.
+ Statistics() []StatisticsFile
+ // PartitionStatisticsFile is an optional list of partition statistics
files.
Review Comment:
```suggestion
// PartitionStatistics returns an optional list of partition statistics
files.
```
##########
table/metadata.go:
##########
@@ -134,6 +134,19 @@ type Metadata interface {
NameMapping() iceberg.NameMapping
LastSequenceNumber() int64
+ // StatisticsFile is an optional list of table statistics.
+ // Table statistics files are valid Puffin files.
+ // StatisticsFile are informational. A reader can choose to ignore
statistics information.
+ // StatisticsFile support is not required to read the table correctly.
+ // A table can contain many statistics files associated with different
table snapshots.
+ Statistics() []StatisticsFile
Review Comment:
should we return an `iter.Seq[StatisticsFile]` so that the slice isn't able
to be modified by users?
--
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]
