You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iceberg.apache.org by at...@apache.org on 2026/07/10 15:21:24 UTC

(iceberg-go) branch main updated: fix(table): validate parquet file writer properties (#1400)

This is an automated email from the ASF dual-hosted git repository.

laskoviymishka 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 02b7ef1d fix(table): validate parquet file writer properties (#1400)
02b7ef1d is described below

commit 02b7ef1de397cc90f8f8b65d486a90e101550660
Author: Minh Vu <vu...@gmail.com>
AuthorDate: Fri Jul 10 17:21:19 2026 +0200

    fix(table): validate parquet file writer properties (#1400)
    
    Parquet writer initialization now validates WriteProps type before
    constructing writer properties so malformed file-info does not panic.
    
    - Adds safe getWriteProperties helper used by NewFileWriter.
    - Returns iceberg.ErrInvalidArgument-compatible errors when write
    properties are nil or the wrong type.
    - Adds regression tests for nil and non-slice write properties.
---
 table/internal/parquet_files.go      | 22 ++++++++++++++++-
 table/internal/parquet_files_test.go | 46 ++++++++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/table/internal/parquet_files.go b/table/internal/parquet_files.go
index 5a0a852b..0f62da5d 100644
--- a/table/internal/parquet_files.go
+++ b/table/internal/parquet_files.go
@@ -403,7 +403,12 @@ func (p parquetFormat) NewFileWriter(ctx context.Context, fs iceio.WriteFileIO,
 
 	counter := &internal.CountingWriter{W: fw}
 	mem := compute.GetAllocator(ctx)
-	writerProps := parquet.NewWriterProperties(info.WriteProps.([]parquet.WriterProperty)...)
+	writerProps, err := getWriteProperties(info.WriteProps)
+	if err != nil {
+		fw.Close()
+
+		return nil, err
+	}
 	arrProps := pqarrow.NewArrowWriterProperties(pqarrow.WithAllocator(mem), pqarrow.WithStoreSchema())
 
 	writer, err := pqarrow.NewFileWriter(arrowSchema, counter, writerProps, arrProps)
@@ -439,6 +444,21 @@ func (p parquetFormat) NewFileWriter(ctx context.Context, fs iceio.WriteFileIO,
 	}, nil
 }
 
+// getWriteProperties requires explicit write properties so misconfigured writer
+// plumbing fails fast instead of silently defaulting Parquet settings.
+func getWriteProperties(writeProps any) (*parquet.WriterProperties, error) {
+	if writeProps == nil {
+		return nil, fmt.Errorf("%w: write properties are required", iceberg.ErrInvalidArgument)
+	}
+
+	writerProperties, ok := writeProps.([]parquet.WriterProperty)
+	if !ok {
+		return nil, fmt.Errorf("%w: invalid write properties type %T", iceberg.ErrInvalidArgument, writeProps)
+	}
+
+	return parquet.NewWriterProperties(writerProperties...), nil
+}
+
 // Write appends a record batch to the Parquet file.
 func (w *ParquetFileWriter) Write(batch arrow.RecordBatch) error {
 	if err := w.accumulateGeoBounds(batch); err != nil {
diff --git a/table/internal/parquet_files_test.go b/table/internal/parquet_files_test.go
index 2cd6b366..75cf8a35 100644
--- a/table/internal/parquet_files_test.go
+++ b/table/internal/parquet_files_test.go
@@ -992,6 +992,52 @@ func TestWriteDataFileErrOnClose(t *testing.T) {
 	require.ErrorContains(t, err, "error on close")
 }
 
+func TestNewFileWriterRejectsInvalidWriteProps(t *testing.T) {
+	fm := internal.GetFileFormat(iceberg.ParquetFile)
+
+	icebergSchema := iceberg.NewSchema(0, iceberg.NestedField{
+		ID:       1,
+		Name:     "id",
+		Type:     iceberg.PrimitiveTypes.Int32,
+		Required: false,
+	})
+	arrowSchema, err := table.SchemaToArrowSchema(icebergSchema, nil, true, false)
+	require.NoError(t, err)
+
+	tests := []struct {
+		name       string
+		writeProps any
+		expected   string
+	}{
+		{
+			name:       "nil write props",
+			writeProps: nil,
+			expected:   "write properties are required",
+		},
+		{
+			name:       "wrong write props type",
+			writeProps: "bad",
+			expected:   "invalid write properties type",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			memfs := iceio.NewMemFS()
+
+			_, err = fm.NewFileWriter(context.Background(), memfs, nil, internal.WriteFileInfo{
+				FileSchema: icebergSchema,
+				FileName:   "f",
+				WriteProps: tt.writeProps,
+				Content:    iceberg.EntryContentData,
+				Spec:       *iceberg.UnpartitionedSpec,
+			}, arrowSchema)
+			require.ErrorIs(t, err, iceberg.ErrInvalidArgument)
+			require.ErrorContains(t, err, tt.expected)
+		})
+	}
+}
+
 func TestParquetFileWriterAbortRemovesFile(t *testing.T) {
 	memFS := iceio.NewMemFS()
 	fileName := "mem://abort-test/data.parquet"