tanmayrauth commented on issue #997: URL: https://github.com/apache/iceberg-go/issues/997#issuecomment-4462826436
@laskoviymishka can I pick this up ? Here's my thinking on the
implementation.
Serialization
We have DeserializeDV() but no inverse. I'd add a SerializeDV(bitmap
*RoaringPositionBitmap) ([]byte, error) in table/dv/deletion_vector.go that
produces the spec format: [length(4)] [magic(4)] [bitmap bytes] [crc32(4)].
Straightforward mirror of the deserialization logic we already have.
Writer API
I'm thinking something like:
```
// table/dv/dv_writer.go
type DVWriter struct { ... }
func NewDVWriter(fs iceio.WriteFileIO) *DVWriter
// Add accumulates positions to delete for a given data file.
func (w *DVWriter) Add(dataFilePath string, positions []int64)
// Flush writes one Puffin file containing one blob per data file,
// returns manifest entries ready for RowDelta.AddDeletes().
func (w *DVWriter) Flush(ctx context.Context, location string)
([]iceberg.DataFile, error)
```
Each blob gets the cardinality property (spec-mandated), and each returned
DataFile has ContentType=PosDeletes, FileFormat=PUFFIN, ReferencedDataFile,
ContentOffset, and ContentSizeInBytes set so the existing
RowDelta.AddDeletes() path commits it without any changes to snapshot
producers.
One design choice: I'd put multiple data files' DVs into a single Puffin
file (one blob per data file) rather than one Puffin file per data file. This
matches Java's approach and avoids file count explosion on wide deletes. The
offset/size fields on the manifest entry let the reader seek to the right
blob.
Property gating
I'd add write.delete.format to table/properties.go:
- v2 default: position (Parquet pos-delete, current behavior)
- v3 default: dv (deletion vector via Puffin)
The switch point would be in positionDeleteRecordsToDataFiles
(arrow_utils.go:1583) if v3 + format=dv, route through DVWriter instead of the
Parquet position-delete writer. The existing v3 warning at line 1614 ("prefer
deletion vectors") goes away once this lands.
What stays unchanged
- RowDelta.AddDeletes() : already accepts any delete file
- Snapshot producers : don't care about file format
- Conflict validation : works on content type, not format
- Scanner/reader : DV read path already landed
Proposed PR split:
1. SerializeDV() + DVWriter struct with Add/Flush + unit tests
(round-trip: serialize → deserialize → same positions)
2. Producer wiring in arrow_utils.go + property gating + integration test
(delete via Go, scan confirms rows filtered)
3. Cross-client test (write DV via iceberg-go, read back via
Java/pyiceberg, assert match)
@zeroshade @laskoviymishka Does this split make sense? Also curious if
you'd prefer the DVWriter to live in table/dv/ alongside the reader or
somewhere else.
--
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]
