ferhatelmas commented on code in PR #1918:
URL: https://github.com/apache/iceberg-go/pull/1918#discussion_r3875132373
##########
table/internal/parquet_files.go:
##########
@@ -1470,27 +1470,28 @@ func (p parquetFormat) DataFileStatsFromMeta(meta
Metadata, statsCols map[int]St
}
for pos := range rowGroup.NumColumns() {
- colChunk, err = rowGroup.ColumnChunk(pos)
- if err != nil {
- panic(err)
- }
-
column := columns[pos]
if column.resolveErr != nil {
+ if _, err = rowGroup.ColumnChunk(pos); err !=
nil {
+ panic(err)
+ }
+
panic(column.resolveErr)
}
- if column.variantChild || column.skipStats {
+ if column.variantChild || column.skipStats ||
column.statsCol.Mode.Typ == MetricModeNone {
continue
}
fieldID, statsCol := column.fieldID, column.statsCol
- if statsCol.Mode.Typ == MetricModeNone {
- continue
- }
if _, invalid := invalidateCol[fieldID]; invalid {
continue
}
+ colChunk, err = rowGroup.ColumnChunk(pos)
+ if err != nil {
+ panic(err)
+ }
Review Comment:
I think this should be before `invalidateCol` check because otherwise there
is a regression in metadata error reporting.
For example, an active field is using full metrics and `StatsSet` errors
below for row group 1, then it will be added into `invalidateCol` in line 1503
and metadata failure will be skipped for row group 2 due to early continue in
line 1487
--
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]
