You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iceberg.apache.org by ze...@apache.org on 2026/04/13 20:48:12 UTC
(iceberg-go) branch main updated: refactor(table): collapse DataFileStatistics.ToDataFile args into DataFileOpts (#888)
This is an automated email from the ASF dual-hosted git repository.
zeroshade pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-go.git
The following commit(s) were added to refs/heads/main by this push:
new 8ab09f56 refactor(table): collapse DataFileStatistics.ToDataFile args into DataFileOpts (#888)
8ab09f56 is described below
commit 8ab09f560fe8b234723bd16c231a50fb77e9b1d7
Author: Ali Asghar <98...@users.noreply.github.com>
AuthorDate: Tue Apr 14 01:48:06 2026 +0500
refactor(table): collapse DataFileStatistics.ToDataFile args into DataFileOpts (#888)
## Summary
Follow-up to the review comment on
[#875](https://github.com/apache/iceberg-go/pull/875#discussion_r3074078181)
— `DataFileStatistics.ToDataFile` grew to eight positional arguments
including two adjacent `int`s (`filesize` and `sortOrderID`), which is
hard to read at call sites and easy to swap by mistake.
This PR collapses the parameters into a `DataFileOpts` struct:
```go
type DataFileOpts struct {
Schema *iceberg.Schema
Spec iceberg.PartitionSpec
Path string
Format iceberg.FileFormat
Content iceberg.ManifestEntryContent
FileSize int64
PartitionValues map[int]any
SortOrderID int
}
func (d *DataFileStatistics) ToDataFile(opts DataFileOpts) iceberg.DataFile
```
All six call sites (two production, four tests) are updated to the
named-field form. Future additions are source-compatible.
## Notes
- Stacked on #875 — this PR will show both diffs until #875 merges, at
which point only the refactor remains.
- No behaviour change.
## Test plan
- [x] `go test ./...`
- [x] `gofmt` clean
Signed-off-by: Ali <al...@gmail.com>
Signed-off-by: Ali <al...@gmail.com>
---
table/arrow_utils.go | 11 ++++++++++-
table/arrow_utils_internal_test.go | 11 +++++++++--
table/internal/parquet_files.go | 11 ++++++++++-
table/internal/parquet_files_test.go | 31 +++++++++++++++++++++++++------
table/internal/utils.go | 33 ++++++++++++++++++++++++---------
5 files changed, 78 insertions(+), 19 deletions(-)
diff --git a/table/arrow_utils.go b/table/arrow_utils.go
index f84aa956..6d062d22 100644
--- a/table/arrow_utils.go
+++ b/table/arrow_utils.go
@@ -1418,7 +1418,16 @@ func fileToDataFile(ctx context.Context, fileIO iceio.IO, filePath string, curre
}
}
- return statistics.ToDataFile(currentSchema, currentSpec, filePath, iceberg.ParquetFile, iceberg.EntryContentData, rdr.SourceFileSize(), partitionValues, sortOrderID)
+ return statistics.ToDataFile(tblutils.DataFileOpts{
+ Schema: currentSchema,
+ Spec: currentSpec,
+ Path: filePath,
+ Format: iceberg.ParquetFile,
+ Content: iceberg.EntryContentData,
+ FileSize: rdr.SourceFileSize(),
+ PartitionValues: partitionValues,
+ SortOrderID: sortOrderID,
+ })
}
func recordNBytes(rec arrow.RecordBatch) (total int64) {
diff --git a/table/arrow_utils_internal_test.go b/table/arrow_utils_internal_test.go
index acd11c02..ed646e0d 100644
--- a/table/arrow_utils_internal_test.go
+++ b/table/arrow_utils_internal_test.go
@@ -199,8 +199,15 @@ func (suite *FileStatsMetricsSuite) getDataFile(meta iceberg.Properties, writeSt
stats := format.DataFileStatsFromMeta(fileMeta, collector, mapping)
- return stats.ToDataFile(tableMeta.CurrentSchema(), tableMeta.PartitionSpec(), "fake-path.parquet",
- iceberg.ParquetFile, iceberg.EntryContentData, fileMeta.GetSourceFileSize(), nil, tableMeta.SortOrder().OrderID())
+ return stats.ToDataFile(internal.DataFileOpts{
+ Schema: tableMeta.CurrentSchema(),
+ Spec: tableMeta.PartitionSpec(),
+ Path: "fake-path.parquet",
+ Format: iceberg.ParquetFile,
+ Content: iceberg.EntryContentData,
+ FileSize: fileMeta.GetSourceFileSize(),
+ SortOrderID: tableMeta.SortOrder().OrderID(),
+ })
}
func (suite *FileStatsMetricsSuite) TestRecordCount() {
diff --git a/table/internal/parquet_files.go b/table/internal/parquet_files.go
index 456624b3..f97197b5 100644
--- a/table/internal/parquet_files.go
+++ b/table/internal/parquet_files.go
@@ -383,7 +383,16 @@ func (w *ParquetFileWriter) Close() (_ iceberg.DataFile, err error) {
stats := w.format.DataFileStatsFromMeta(filemeta, w.info.StatsCols, w.colMapping)
stats.EqualityFieldIDs = w.info.EqualityFieldIDs
- return stats.ToDataFile(w.info.FileSchema, w.info.Spec, w.info.FileName, iceberg.ParquetFile, w.info.Content, w.counter.Count, w.partition, w.info.SortOrderID), nil
+ return stats.ToDataFile(DataFileOpts{
+ Schema: w.info.FileSchema,
+ Spec: w.info.Spec,
+ Path: w.info.FileName,
+ Format: iceberg.ParquetFile,
+ Content: w.info.Content,
+ FileSize: w.counter.Count,
+ PartitionValues: w.partition,
+ SortOrderID: w.info.SortOrderID,
+ }), nil
}
type decAsIntAgg[T int32 | int64] struct {
diff --git a/table/internal/parquet_files_test.go b/table/internal/parquet_files_test.go
index 9e1a71da..2fe90cde 100644
--- a/table/internal/parquet_files_test.go
+++ b/table/internal/parquet_files_test.go
@@ -261,8 +261,15 @@ func TestMetricsPrimitiveTypes(t *testing.T) {
stats := format.DataFileStatsFromMeta(internal.Metadata(meta), getCollector(), mapping)
const sortOrderID = 7
- df := stats.ToDataFile(tblMeta.CurrentSchema(), tblMeta.PartitionSpec(), "fake-path.parquet",
- iceberg.ParquetFile, iceberg.EntryContentData, meta.GetSourceFileSize(), nil, sortOrderID)
+ df := stats.ToDataFile(internal.DataFileOpts{
+ Schema: tblMeta.CurrentSchema(),
+ Spec: tblMeta.PartitionSpec(),
+ Path: "fake-path.parquet",
+ Format: iceberg.ParquetFile,
+ Content: iceberg.EntryContentData,
+ FileSize: meta.GetSourceFileSize(),
+ SortOrderID: sortOrderID,
+ })
require.NotNil(t, df.SortOrderID())
assert.Equal(t, sortOrderID, *df.SortOrderID())
@@ -415,8 +422,14 @@ func TestNanosecondTimestampMetrics(t *testing.T) {
}
stats := format.DataFileStatsFromMeta(internal.Metadata(meta), collector, mapping)
- df := stats.ToDataFile(tableMeta.CurrentSchema(), tableMeta.PartitionSpec(), "fake-path.parquet",
- iceberg.ParquetFile, iceberg.EntryContentData, meta.GetSourceFileSize(), nil, 0)
+ df := stats.ToDataFile(internal.DataFileOpts{
+ Schema: tableMeta.CurrentSchema(),
+ Spec: tableMeta.PartitionSpec(),
+ Path: "fake-path.parquet",
+ Format: iceberg.ParquetFile,
+ Content: iceberg.EntryContentData,
+ FileSize: meta.GetSourceFileSize(),
+ })
assert.Len(t, df.ValueCounts(), 2)
assert.Len(t, df.NullValueCounts(), 2)
@@ -560,8 +573,14 @@ func TestDecimalPhysicalTypes(t *testing.T) {
stats := format.DataFileStatsFromMeta(internal.Metadata(meta), collector, mapping)
require.NotNil(t, stats)
- df := stats.ToDataFile(tableMeta.CurrentSchema(), tableMeta.PartitionSpec(), "test.parquet",
- iceberg.ParquetFile, iceberg.EntryContentData, meta.GetSourceFileSize(), nil, 0)
+ df := stats.ToDataFile(internal.DataFileOpts{
+ Schema: tableMeta.CurrentSchema(),
+ Spec: tableMeta.PartitionSpec(),
+ Path: "test.parquet",
+ Format: iceberg.ParquetFile,
+ Content: iceberg.EntryContentData,
+ FileSize: meta.GetSourceFileSize(),
+ })
// Verify bounds are correctly extracted
require.Contains(t, df.LowerBoundValues(), 1)
diff --git a/table/internal/utils.go b/table/internal/utils.go
index a7a9770e..02e3d949 100644
--- a/table/internal/utils.go
+++ b/table/internal/utils.go
@@ -236,17 +236,32 @@ func (d *DataFileStatistics) PartitionValue(field iceberg.PartitionField, sc *ic
return lowerT.Val.Any()
}
-func (d *DataFileStatistics) ToDataFile(schema *iceberg.Schema, spec iceberg.PartitionSpec, path string, format iceberg.FileFormat, content iceberg.ManifestEntryContent, filesize int64, partitionValues map[int]any, sortOrderID int) iceberg.DataFile {
+// DataFileOpts groups the fields needed to finalize a DataFile from
+// DataFileStatistics. Collapsing the former positional arguments into a
+// struct keeps call sites readable and makes future additions source-
+// compatible.
+type DataFileOpts struct {
+ Schema *iceberg.Schema
+ Spec iceberg.PartitionSpec
+ Path string
+ Format iceberg.FileFormat
+ Content iceberg.ManifestEntryContent
+ FileSize int64
+ PartitionValues map[int]any
+ SortOrderID int
+}
+
+func (d *DataFileStatistics) ToDataFile(opts DataFileOpts) iceberg.DataFile {
var fieldIDToPartitionData map[int]any
fieldIDToLogicalType := make(map[int]avro.LogicalType)
fieldIDToFixedSize := make(map[int]int)
- if !spec.Equals(*iceberg.UnpartitionedSpec) {
+ if !opts.Spec.Equals(*iceberg.UnpartitionedSpec) {
fieldIDToPartitionData = make(map[int]any)
- for _, field := range spec.Fields() {
- partitionVal := partitionValues[field.FieldID]
+ for _, field := range opts.Spec.Fields() {
+ partitionVal := opts.PartitionValues[field.FieldID]
if partitionVal != nil {
- val := d.PartitionValue(field, schema)
+ val := d.PartitionValue(field, opts.Schema)
if val != nil {
fieldIDToPartitionData[field.FieldID] = val
} else {
@@ -256,7 +271,7 @@ func (d *DataFileStatistics) ToDataFile(schema *iceberg.Schema, spec iceberg.Par
fieldIDToPartitionData[field.FieldID] = nil
}
- if sourceField, ok := schema.FindFieldByID(field.SourceID()); ok {
+ if sourceField, ok := opts.Schema.FindFieldByID(field.SourceID()); ok {
resultType := field.Transform.ResultType(sourceField.Type)
switch rt := resultType.(type) {
@@ -278,8 +293,8 @@ func (d *DataFileStatistics) ToDataFile(schema *iceberg.Schema, spec iceberg.Par
}
}
- bldr, err := iceberg.NewDataFileBuilder(spec, content,
- path, format, fieldIDToPartitionData, fieldIDToLogicalType, fieldIDToFixedSize, d.RecordCount, filesize)
+ bldr, err := iceberg.NewDataFileBuilder(opts.Spec, opts.Content,
+ opts.Path, opts.Format, fieldIDToPartitionData, fieldIDToLogicalType, fieldIDToFixedSize, d.RecordCount, opts.FileSize)
if err != nil {
panic(err)
}
@@ -315,7 +330,7 @@ func (d *DataFileStatistics) ToDataFile(schema *iceberg.Schema, spec iceberg.Par
bldr.EqualityFieldIDs(d.EqualityFieldIDs)
}
- bldr.SortOrderID(sortOrderID)
+ bldr.SortOrderID(opts.SortOrderID)
return bldr.Build()
}