Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 20 additions & 10 deletions data_file_codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,16 +175,29 @@ func newDecodeEntry(version int) (any, *dataFile) {
return &manifestEntry{Data: df}, df
}

var dataFileAvroFieldIndexes = avroFieldIndexes(reflect.TypeOf(dataFile{}))

func avroFieldIndexes(t reflect.Type) []int {
indexes := make([]int, 0, t.NumField())
for i := range t.NumField() {
if _, hasAvroTag := t.Field(i).Tag.Lookup("avro"); hasAvroTag {
indexes = append(indexes, i)
}
}

return indexes
}

// cloneDataFileAvroFields returns a fresh *dataFile populated with src's
// avro-tagged fields. Internal state (sync.Once, lazy-init caches,
// specID, the field-id lookup maps) is intentionally left at zero
// values because the avro encoder reads only the avro-tagged fields.
//
// Using reflection over the tag set means a new avro-tagged field
// upstream is auto-copied without an update here — the dataFile struct
// remains the single source of truth for the wire shape. It also
// sidesteps the go-vet copies-lock warning that would fire on a
// struct-literal copy of *dataFile (it embeds sync.Once).
// The avro-tagged field indexes are discovered once when the package is
// initialized. This keeps dataFile as the single source of truth for the
// wire shape while avoiding a reflect.Type and StructTag lookup for every
// encoded manifest entry. It also sidesteps the go-vet copies-lock warning
// that would fire on a struct-literal copy of *dataFile (it embeds sync.Once).
//
// Note: this is a shallow copy. Pointer-typed avro fields (ColSizes,
// LowerBounds, etc.) share their backing storage with the source.
Expand All @@ -196,11 +209,8 @@ func cloneDataFileAvroFields(src *dataFile) *dataFile {
out := &dataFile{}
srcVal := reflect.ValueOf(src).Elem()
outVal := reflect.ValueOf(out).Elem()
t := srcVal.Type()
for i := range t.NumField() {
if _, hasAvroTag := t.Field(i).Tag.Lookup("avro"); hasAvroTag {
outVal.Field(i).Set(srcVal.Field(i))
}
for _, i := range dataFileAvroFieldIndexes {
outVal.Field(i).Set(srcVal.Field(i))
}

return out
Expand Down
49 changes: 48 additions & 1 deletion data_file_codec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,19 @@ func TestMarshalAvroEntryDoesNotMutateAnyAvroField(t *testing.T) {
"including pointer-typed fields whose backing storage is shared with the clone")
}

func TestDataFileAvroFieldIndexesCoverEveryAvroField(t *testing.T) {
typ := reflect.TypeOf(dataFile{})
want := make([]int, 0, typ.NumField())
for i := range typ.NumField() {
if _, ok := typ.Field(i).Tag.Lookup("avro"); ok {
want = append(want, i)
}
}

require.Equal(t, want, dataFileAvroFieldIndexes,
"the precomputed clone indexes must track every avro-tagged dataFile field")
}

func TestMarshalAvroEntryDecimalPartitionRoundTrip(t *testing.T) {
schema := NewSchema(0,
NestedField{ID: 1, Name: "price", Type: DecimalTypeOf(10, 2)},
Expand Down Expand Up @@ -246,7 +259,7 @@ func deepCopyReflect(src reflect.Value) reflect.Value {
}
}

func fullyPopulatedDataFileForCodec(t *testing.T, version int) (PartitionSpec, *Schema, DataFile) {
func fullyPopulatedDataFileForCodec(t testing.TB, version int) (PartitionSpec, *Schema, DataFile) {
t.Helper()
schema := NewSchema(123,
NestedField{ID: 1, Name: "id", Type: Int64Type{}, Required: true},
Expand Down Expand Up @@ -296,3 +309,37 @@ func fullyPopulatedDataFileForCodec(t *testing.T, version int) (PartitionSpec, *

return spec, schema, builder.Build()
}

var (
benchmarkDataFileCloneSink *dataFile
benchmarkEncodedEntrySize int
)

func BenchmarkCloneDataFileAvroFields(b *testing.B) {
_, _, df := fullyPopulatedDataFileForCodec(b, 2)
impl := df.(*dataFile)

b.ReportAllocs()
b.ResetTimer()
for range b.N {
benchmarkDataFileCloneSink = cloneDataFileAvroFields(impl)
}
}

func BenchmarkMarshalAvroEntry(b *testing.B) {
for _, version := range []int{1, 2, 3} {
b.Run("v"+strconv.Itoa(version), func(b *testing.B) {
spec, schema, df := fullyPopulatedDataFileForCodec(b, version)
impl := df.(*dataFile)
b.ReportAllocs()
b.ResetTimer()
for range b.N {
encoded, err := impl.MarshalAvroEntry(spec, schema, version)
if err != nil {
b.Fatal(err)
}
benchmarkEncodedEntrySize = len(encoded)
}
})
}
}
2 changes: 1 addition & 1 deletion table/internal/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func TestTruncateUpperBoundBinary(t *testing.T) {
assert.Equal(t, first, second)
assert.Equal(t, original, tt.value)

if tt.truncate < len(tt.value) && len(first) > 0 {
if tt.truncate >= 0 && tt.truncate < len(tt.value) && len(first) > 0 {
assert.LessOrEqual(t, len(first), tt.truncate)
assert.Greater(t, bytes.Compare(first, tt.value), 0)
}
Expand Down
Loading