ryanworl opened a new issue, #1760:
URL: https://github.com/apache/iceberg-go/issues/1760
### Apache Iceberg version
main (development)
### Please describe the bug 🐞
`ReadManifestList` picks its reader schema from the `format-version` key in
the Avro file metadata, and assumes v1 when the key is missing. As the doc
comment on the function itself notes, manifest lists aren't required to
carry that key — but the v1 fallback means a perfectly valid v2/v3 list
from a writer that skips it gets decoded through the v1 schema, and the
fields v1 doesn't have are silently zeroed:
- `content` reads as `data`, so delete manifests look like data manifests
- `sequence-number` / `min-sequence-number` read as 0
- v3 `first-row-id` is lost
No error, no warning — Avro schema resolution just drops the extra writer
fields.
I hit this with real files: DuckDB's iceberg extension (v1.5.x) writes
manifest lists without the key, so reading any DuckDB-written v3 table
with iceberg-go makes its deletion vectors invisible and zeroes every
sequence number. Java and PyIceberg read the same files fine since they
project from the embedded writer schema.
Observed on v0.6.0.
## Reproduction
Round-trip through iceberg-go itself: write a v2 list, drop the optional
key from the header (data blocks untouched), read it back.
```go
package main
import (
"bytes"
"fmt"
"github.com/apache/iceberg-go"
)
func main() {
// A v2 manifest list holding one delete manifest with sequence number
5.
mf := iceberg.NewManifestFile(2,
"s3://bucket/metadata/deletes-m0.avro", 100, 0, 42).
Content(iceberg.ManifestContentDeletes).
SequenceNum(5, 3).
Build()
var buf bytes.Buffer
seq := int64(5)
if err := iceberg.WriteManifestList(2, &buf, 42, nil, &seq, 0,
[]iceberg.ManifestFile{mf}); err != nil {
panic(err)
}
stripped := stripMetadataKey(buf.Bytes(), "format-version")
got, err := iceberg.ReadManifestList(bytes.NewReader(stripped))
if err != nil {
panic(err)
}
fmt.Printf("content = %v (want deletes)\n",
got[0].ManifestContent())
fmt.Printf("sequence-number = %d (want 5)\n", got[0].SequenceNum())
fmt.Printf("min-sequence-number = %d (want 3)\n",
got[0].MinSequenceNum())
}
// stripMetadataKey re-encodes an Avro OCF header without one metadata key;
// the sync marker and data blocks are copied through untouched.
func stripMetadataKey(data []byte, key string) []byte {
off := 4 // past "Obj\x01"
meta := map[string][]byte{}
for {
n, next := readLong(data, off)
if n == 0 {
off++ // consume terminator
break
}
if n < 0 {
n = -n
_, next = readLong(data, next) // block byte size
}
off = next
for i := int64(0); i < n; i++ {
var k, v []byte
k, off = readBytes(data, off)
v, off = readBytes(data, off)
meta[string(k)] = v
}
}
delete(meta, key)
out := append([]byte(nil), data[:4]...)
out = appendLong(out, int64(len(meta)))
for k, v := range meta {
out = appendLong(out, int64(len(k)))
out = append(out, k...)
out = appendLong(out, int64(len(v)))
out = append(out, v...)
}
out = append(out, 0)
return append(out, data[off:]...)
}
func readLong(data []byte, off int) (int64, int) {
var u uint64
var shift uint
for {
b := data[off]
off++
u |= uint64(b&0x7f) << shift
if b&0x80 == 0 {
break
}
shift += 7
}
return int64(u>>1) ^ -int64(u&1), off
}
func readBytes(data []byte, off int) ([]byte, int) {
n, off := readLong(data, off)
return data[off : off+int(n)], off + int(n)
}
func appendLong(dst []byte, v int64) []byte {
u := uint64(v<<1) ^ uint64(v>>63)
for u >= 0x80 {
dst = append(dst, byte(u)|0x80)
u >>= 7
}
return append(dst, byte(u))
}
```
Output on v0.6.0:
```
content = data (want deletes)
sequence-number = 0 (want 5)
min-sequence-number = 0 (want 3)
```
Or with real files: create a v3 table with DuckDB's iceberg extension and
read `metadata/snap-*.avro` with `ReadManifestList` — same result.
## Suggested fix
The version is recoverable from the embedded writer schema when the key
is absent: `sequence_number`/`content` fields (ids 515/517) imply v2+,
`first_row_id` (id 520) implies v3. Even an error would beat silently
returning wrong values, since anything that rewrites metadata based on
them (compaction, snapshot management) writes the corruption back out.
Happy to send a PR if the schema-inference approach sounds right to you.
--
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]
