slachiewicz commented on issue #657:
URL:
https://github.com/apache/incubator-xtable/issues/657#issuecomment-5382309772
A reproduction of the known limitation tracked in #339, with an independent
reader as the
oracle, in case it helps move this flag along. It supports @ashvina's
position above that the
default should be an error rather than a warning.
Setup: a local Delta table with `delta.enableDeletionVectors = true`, 1,000
rows, synced
DELTA → ICEBERG with the bundled jar from `main`.
```
sync 1 -> Iceberg snapshot: total-records=1000, total-data-files=4,
total-delete-files=0 correct
DELETE FROM delta.`...` WHERE id < 100
-> Delta now reads 900 rows; one deletion_vector_*.bin on disk
sync 2 -> "Sync is successful", sourceIdentifier advances 1 -> 2, a NEW
snapshot is written
-> that snapshot: total-records=1000, total-data-files=4,
total-delete-files=0
```
Read back with DuckDB, which reads both formats and is neither XTable nor
Spark — its Delta
support is delta-kernel-rs, so both sides of the comparison are foreign
readers:
```sql
SELECT count(*) FROM delta_scan('.../dv_table'); -- 900
SELECT count(*) FROM iceberg_scan('.../metadata/v4.metadata.json'); -- 1000
SELECT id FROM iceberg_scan(...) EXCEPT SELECT id FROM delta_scan(...) ORDER
BY id LIMIT 10;
-- 0,1,2,3,4,5,6,7,8,9 -- exactly the deleted rows
```
Delta says 900, Iceberg says 1000, and the extra rows are precisely the ones
that were deleted.
Exit code 0, nothing in the log — `grep -i "deletion vector"` over the run
output returns nothing.
pyiceberg gives the same answer independently.
For what it is worth as a control, the same harness over five tables
converted without deletion
vectors reports matching row counts *and* identical value-level checksums
between
`delta_scan` and `iceberg_scan`. The ordinary conversion path is faithful;
this is a specific
hole, not general drift.
The path is in `DeltaConversionSource`. A Delta DELETE with deletion vectors
emits
`RemoveFile(old)` plus `AddFile(same path, with DV)`, and the pair is
cancelled out of both the
added and removed maps, since at file level nothing changed:
```java
for (String deletionVector : deletionVectors) {
if (removedFiles.containsKey(deletionVector)) {
addedFiles.remove(deletionVector);
removedFiles.remove(deletionVector);
}
```
That is correct as file-level bookkeeping. The consequence is that the
row-level delete leaves
no trace in the diff at all, so the incremental sync is a well-formed no-op
over a table whose
contents changed.
Why the flag matters. The RFC merged in #634 (`rfc/rfc-2`) already describes
the intended
conversion, and notes its cost is "proportional to the number of records
deleted, and not the
size of metadata or number of files" — so this is tractable work rather than
a blocked design.
But it has been open since, and in the meantime the failure mode is silent.
Deleted rows remain
readable through the translated Iceberg metadata, and nothing in the run
says so. That is the
case for erroring by default, with an opt-in flag to warn and continue,
exactly as suggested
above.
One thing that may be worth folding into the plan later, explicitly **not**
as a reason to
delay any of the above. The RFC targets Iceberg v2 positional delete files.
The Iceberg spec now
reads:
> Position deletes are encoded in a _position delete file_ (V2) or _deletion
vector_ (V3 or above).
XTable writes v2 today — `iceberg-core` 1.9.2 has
`SUPPORTED_TABLE_FORMAT_VERSION = 3` but
`DEFAULT_TABLE_FORMAT_VERSION = 2`, and I found no `format-version` handling
in the codebase to
override it. A v3 deletion vector is closer in shape to what Delta already
stores, so it may
eventually be the better encoding. But v2 metadata is readable by v3
readers, so implementing
rfc-2 as specified is not wasted work. Suggested ordering: the fail-loudly
flag now, rfc-2's v2
path as designed, and v3 deletion vectors as a later optimisation if it
proves worthwhile.
Versions, so the repro is actionable: XTable `main` at dafce0a
(0.5.0-SNAPSHOT, bundled jar) on
Corretto 11; source table written by delta-spark 3.2.0 / PySpark 3.5.1;
`iceberg-core` 1.9.2;
read back with DuckDB 1.4.5 and PyIceberg.
Happy to contribute the reproduction as a test case if that would help.
*This comment was created with AI assistance.*
--
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]
