update from review comments

go.mod

@@ -21,11 +21,13 @@ go 1.20
 
 require (
 	github.com/stretchr/testify v1.8.4
-	golang.org/x/exp v0.0.0-20230811145659-89c5cff77bcb
+	golang.org/x/exp v0.0.0-20230905200255-921286631fa9
 )
 
 require (
 	github.com/davecgh/go-spew v1.1.1 // indirect
+	github.com/kr/pretty v0.3.1 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
+	gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 )

go.sum

@@ -1,12 +1,14 @@
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
+github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE=
+github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
+github.com/rogpeppe/go-internal v1.9.0 h1:73kH8U+JUqXU8lRuOHeVHaa/SZPifC7BkcraZVejAe8=
 github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=
 github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
-golang.org/x/exp v0.0.0-20230811145659-89c5cff77bcb h1:mIKbk8weKhSeLH2GmUTrvx8CjkyJmnU1wFmg59CUjFA=
-golang.org/x/exp v0.0.0-20230811145659-89c5cff77bcb/go.mod h1:FXUEEKJgO7OQYeo8N01OfiKP8RXMtf6e8aTskBGqWdc=
-gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
+golang.org/x/exp v0.0.0-20230905200255-921286631fa9 h1:GoHiUyI/Tp2nVkLI2mCxVkOjsbSXD66ic0XW0js0R9g=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
+gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk=
 gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
 gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=

partitions.go

@@ -30,10 +30,10 @@ const (
 	InitialPartitionSpecID = 0
 )
 
-// UnpartitionedPartitionSpec is the default unpartitioned spec which can
+// UnpartitionedSpec is the default unpartitioned spec which can
 // be used for comparisons or to just provide a convenience for referencing
 // the same unpartitioned spec object.
-var UnpartitionedPartitionSpec = &PartitionSpec{id: 0}
+var UnpartitionedSpec = &PartitionSpec{id: 0}
 
 // PartitionField represents how one partition value is derived from the
 // source column by transformation.
@@ -153,7 +153,21 @@ func (ps *PartitionSpec) initialize() {
 func (ps *PartitionSpec) ID() int                    { return ps.id }
 func (ps *PartitionSpec) NumFields() int             { return len(ps.fields) }
 func (ps *PartitionSpec) Field(i int) PartitionField { return ps.fields[i] }
-func (ps *PartitionSpec) IsUnpartitioned() bool      { return len(ps.fields) == 0 }
+
+func (ps *PartitionSpec) IsUnpartitioned() bool {
+	if len(ps.fields) == 0 {
+		return true
+	}
+
+	for _, f := range ps.fields {
+		if _, ok := f.Transform.(VoidTransform); !ok {
+			return false
+		}
+	}
+
+	return true
+}
+
 func (ps *PartitionSpec) FieldsBySourceID(fieldID int) []PartitionField {
 	return slices.Clone(ps.sourceIdToFields[fieldID])
 }
@@ -176,7 +190,7 @@ func (ps PartitionSpec) String() string {
 
 func (ps *PartitionSpec) LastAssignedFieldID() int {
 	if len(ps.fields) == 0 {
-		return partitionDataIDStart
+		return partitionDataIDStart - 1
 	}
 
 	id := ps.fields[0].FieldID

partitions_test.go

@@ -27,9 +27,9 @@ import (
 )
 
 func TestPartitionSpec(t *testing.T) {
-	assert.Equal(t, 1000, iceberg.UnpartitionedPartitionSpec.LastAssignedFieldID())
+	assert.Equal(t, 999, iceberg.UnpartitionedSpec.LastAssignedFieldID())
 
-	bucket := iceberg.BucketTransform{N: 4}
+	bucket := iceberg.BucketTransform{NumBuckets: 4}
 	idField1 := iceberg.PartitionField{
 		SourceID: 3, FieldID: 1001, Name: "id", Transform: bucket}
 	spec1 := iceberg.NewPartitionSpec(idField1)
@@ -59,19 +59,28 @@ func TestPartitionSpec(t *testing.T) {
 	assert.Equal(t, 1002, spec3.LastAssignedFieldID())
 }
 
+func TestUnpartitionedWithVoidField(t *testing.T) {
+	spec := iceberg.NewPartitionSpec(iceberg.PartitionField{
+		SourceID: 3, FieldID: 1001, Name: "void", Transform: iceberg.VoidTransform{},
+	})
+
+	assert.True(t, spec.IsUnpartitioned())
+}
+
 func TestSerializeUnpartitionedSpec(t *testing.T) {
-	data, err := json.Marshal(iceberg.UnpartitionedPartitionSpec)
+	data, err := json.Marshal(iceberg.UnpartitionedSpec)
 	require.NoError(t, err)
 
 	assert.JSONEq(t, `{"spec-id": 0, "fields": []}`, string(data))
+	assert.True(t, iceberg.UnpartitionedSpec.IsUnpartitioned())
 }
 
 func TestSerializePartitionSpec(t *testing.T) {
 	spec := iceberg.NewPartitionSpecID(3,
 		iceberg.PartitionField{SourceID: 1, FieldID: 1000,
-			Transform: iceberg.TruncateTransform{W: 19}, Name: "str_truncate"},
+			Transform: iceberg.TruncateTransform{Width: 19}, Name: "str_truncate"},
 		iceberg.PartitionField{SourceID: 2, FieldID: 1001,
-			Transform: iceberg.BucketTransform{N: 25}, Name: "int_bucket"},
+			Transform: iceberg.BucketTransform{NumBuckets: 25}, Name: "int_bucket"},
 	)
 
 	data, err := json.Marshal(spec)
@@ -104,9 +113,9 @@ func TestSerializePartitionSpec(t *testing.T) {
 func TestPartitionType(t *testing.T) {
 	spec := iceberg.NewPartitionSpecID(3,
 		iceberg.PartitionField{SourceID: 1, FieldID: 1000,
-			Transform: iceberg.TruncateTransform{W: 19}, Name: "str_truncate"},
+			Transform: iceberg.TruncateTransform{Width: 19}, Name: "str_truncate"},
 		iceberg.PartitionField{SourceID: 2, FieldID: 1001,
-			Transform: iceberg.BucketTransform{N: 25}, Name: "int_bucket"},
+			Transform: iceberg.BucketTransform{NumBuckets: 25}, Name: "int_bucket"},
 		iceberg.PartitionField{SourceID: 3, FieldID: 1002,
 			Transform: iceberg.IdentityTransform{}, Name: "bool_identity"},
 		iceberg.PartitionField{SourceID: 1, FieldID: 1003,

transforms.go

@@ -37,15 +37,15 @@ func ParseTransform(s string) (Transform, error) {
 		}
 
 		n, _ := strconv.Atoi(matches[1])
-		return BucketTransform{N: n}, nil
+		return BucketTransform{NumBuckets: n}, nil
 	case strings.HasPrefix(s, "truncate"):
 		matches := regexFromBrackets.FindStringSubmatch(s)
 		if len(matches) != 2 {
 			break
 		}
 
 		n, _ := strconv.Atoi(matches[1])
-		return TruncateTransform{W: n}, nil
+		return TruncateTransform{Width: n}, nil
 	default:
 		switch s {
 		case "identity":
@@ -104,27 +104,27 @@ func (VoidTransform) ResultType(t Type) Type { return t }
 // a 32-bit hash of the source value to produce a positive value by mod
 // the bucket number.
 type BucketTransform struct {
-	N int
+	NumBuckets int
 }
 
 func (t BucketTransform) MarshalText() ([]byte, error) {
 	return []byte(t.String()), nil
 }
 
-func (t BucketTransform) String() string { return fmt.Sprintf("bucket[%d]", t.N) }
+func (t BucketTransform) String() string { return fmt.Sprintf("bucket[%d]", t.NumBuckets) }
 
 func (BucketTransform) ResultType(Type) Type { return PrimitiveTypes.Int32 }
 
 // TruncateTransform is a transformation for truncating a value to a specified width.
 type TruncateTransform struct {
-	W int
+	Width int
 }
 
 func (t TruncateTransform) MarshalText() ([]byte, error) {
 	return []byte(t.String()), nil
 }
 
-func (t TruncateTransform) String() string { return fmt.Sprintf("truncate[%d]", t.W) }
+func (t TruncateTransform) String() string { return fmt.Sprintf("truncate[%d]", t.Width) }
 
 func (TruncateTransform) ResultType(t Type) Type { return t }
 

transforms_test.go

@@ -43,14 +43,14 @@ func TestParseTransform(t *testing.T) {
 		{"DaY", iceberg.DayTransform{}},
 		{"hour", iceberg.HourTransform{}},
 		{"hOuR", iceberg.HourTransform{}},
-		{"bucket[5]", iceberg.BucketTransform{N: 5}},
-		{"bucket[100]", iceberg.BucketTransform{N: 100}},
-		{"BUCKET[5]", iceberg.BucketTransform{N: 5}},
-		{"bUCKeT[100]", iceberg.BucketTransform{N: 100}},
-		{"truncate[10]", iceberg.TruncateTransform{W: 10}},
-		{"truncate[255]", iceberg.TruncateTransform{W: 255}},
-		{"TRUNCATE[10]", iceberg.TruncateTransform{W: 10}},
-		{"tRuNCATe[255]", iceberg.TruncateTransform{W: 255}},
+		{"bucket[5]", iceberg.BucketTransform{NumBuckets: 5}},
+		{"bucket[100]", iceberg.BucketTransform{NumBuckets: 100}},
+		{"BUCKET[5]", iceberg.BucketTransform{NumBuckets: 5}},
+		{"bUCKeT[100]", iceberg.BucketTransform{NumBuckets: 100}},
+		{"truncate[10]", iceberg.TruncateTransform{Width: 10}},
+		{"truncate[255]", iceberg.TruncateTransform{Width: 255}},
+		{"TRUNCATE[10]", iceberg.TruncateTransform{Width: 10}},
+		{"tRuNCATe[255]", iceberg.TruncateTransform{Width: 255}},
 	}
 
 	for _, tt := range tests {