Skip to content

Commit 02aecba

Browse files
fix(scan): read deletion vectors via manifest content_offset/content_size
The deletion vector read path located the DV blob by parsing a Puffin footer (PuffinReader::file_metadata). That only works when the DV is wrapped in a full Puffin container — but the Iceberg spec addresses a DV blob by the manifest entry's content_offset + content_size_in_bytes, and writers such as DuckDB store the deletion-vector-v1 blob standalone (no PFA1 framing). Reading those via the footer fails with 'Bad magic value ... should be PFA1'. Now FileScanTaskDeleteFile carries content_offset + content_size_in_bytes, and the loader reads exactly those bytes and parses them with the new DeleteVector::from_serialized_bytes (factored out of from_puffin_blob). This works whether the DV is standalone or inside a Puffin container. Validated cross-engine on real data: iceberg-rust scans tables with DuckDB-written DVs and returns the correct post-delete row counts (900K/1M, 9M/10M). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Raghvendra Singh <raghav@cashify.in>
1 parent 7a082a1 commit 02aecba

3 files changed

Lines changed: 66 additions & 18 deletions

File tree

crates/iceberg/src/arrow/caching_delete_file_loader.rs

Lines changed: 44 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ use crate::delete_vector::DeleteVector;
3131
use crate::expr::Predicate::AlwaysTrue;
3232
use crate::expr::{Predicate, Reference};
3333
use crate::io::FileIO;
34-
use crate::puffin::{DELETION_VECTOR_V1, PuffinReader};
3534
use crate::runtime::Runtime;
3635
use crate::scan::{ArrowRecordBatchStream, FileScanTaskDeleteFile};
3736
use crate::spec::{
@@ -59,11 +58,13 @@ enum DeleteFileContext {
5958
file_path: String,
6059
stream: ArrowRecordBatchStream,
6160
},
62-
/// A V3 deletion vector stored in a Puffin file; the blob is read and parsed
63-
/// into a `DeleteVector` in the parse phase.
61+
/// A V3 deletion vector; the blob located by `content_offset`/`content_size`
62+
/// is read and parsed into a `DeleteVector` in the parse phase.
6463
DelVec {
6564
file_path: String,
6665
referenced_data_file: String,
66+
content_offset: u64,
67+
content_size: u64,
6768
file_io: FileIO,
6869
},
6970
FreshEqDel {
@@ -258,7 +259,8 @@ impl CachingDeleteFileLoader {
258259
}
259260
PosDelLoadAction::Load => {
260261
if task.file_format == DataFileFormat::Puffin {
261-
// V3 deletion vector — parsed from the Puffin file in the
262+
// V3 deletion vector — the blob located by
263+
// content_offset/content_size is read + parsed in the
262264
// parse phase (not a Parquet positional-delete stream).
263265
Ok(DeleteFileContext::DelVec {
264266
file_path: task.file_path.clone(),
@@ -271,6 +273,18 @@ impl CachingDeleteFileLoader {
271273
"deletion vector is missing referenced_data_file",
272274
)
273275
})?,
276+
content_offset: task.content_offset.ok_or_else(|| {
277+
Error::new(
278+
ErrorKind::DataInvalid,
279+
"deletion vector is missing content_offset",
280+
)
281+
})? as u64,
282+
content_size: task.content_size_in_bytes.ok_or_else(|| {
283+
Error::new(
284+
ErrorKind::DataInvalid,
285+
"deletion vector is missing content_size_in_bytes",
286+
)
287+
})? as u64,
274288
file_io: basic_delete_file_loader.file_io().clone(),
275289
})
276290
} else {
@@ -338,10 +352,13 @@ impl CachingDeleteFileLoader {
338352
DeleteFileContext::DelVec {
339353
file_path,
340354
referenced_data_file,
355+
content_offset,
356+
content_size,
341357
file_io,
342358
} => {
343359
let delete_vector =
344-
Self::parse_deletion_vector_from_puffin(&file_io, &file_path).await?;
360+
Self::read_deletion_vector(&file_io, &file_path, content_offset, content_size)
361+
.await?;
345362
let mut results = HashMap::default();
346363
results.insert(referenced_data_file, delete_vector);
347364
Ok(ParsedDeleteFileContext::DelVecs { file_path, results })
@@ -368,26 +385,34 @@ impl CachingDeleteFileLoader {
368385
}
369386
}
370387

371-
/// Reads a V3 `deletion-vector-v1` blob from a Puffin file into a `DeleteVector`.
372-
async fn parse_deletion_vector_from_puffin(
388+
/// Reads a V3 deletion-vector-v1 blob located at `content_offset` (length
389+
/// `content_size`) within `file_path` and parses it into a `DeleteVector`.
390+
///
391+
/// The blob is addressed by the manifest entry's content offset/size rather
392+
/// than by parsing a Puffin footer, so this works whether the deletion vector
393+
/// is a standalone blob file (e.g. written by DuckDB) or stored inside a
394+
/// Puffin container.
395+
async fn read_deletion_vector(
373396
file_io: &FileIO,
374397
file_path: &str,
398+
content_offset: u64,
399+
content_size: u64,
375400
) -> Result<DeleteVector> {
376-
let input = file_io.new_input(file_path)?;
377-
let reader = PuffinReader::new(input);
378-
let metadata = reader.file_metadata().await?;
379-
let blob_metadata = metadata
380-
.blobs()
381-
.iter()
382-
.find(|blob| blob.r#type == DELETION_VECTOR_V1)
401+
let bytes = file_io.new_input(file_path)?.read().await?;
402+
let start = content_offset as usize;
403+
let end = start
404+
.checked_add(content_size as usize)
405+
.filter(|end| *end <= bytes.len())
383406
.ok_or_else(|| {
384407
Error::new(
385408
ErrorKind::DataInvalid,
386-
format!("Puffin file {file_path} contains no deletion-vector-v1 blob"),
409+
format!(
410+
"deletion vector blob range {content_offset}..+{content_size} is out of bounds for {file_path} ({} bytes)",
411+
bytes.len()
412+
),
387413
)
388414
})?;
389-
let blob = reader.blob(blob_metadata).await?;
390-
DeleteVector::from_puffin_blob(blob)
415+
DeleteVector::from_serialized_bytes(&bytes[start..end])
391416
}
392417

393418
/// Parses a record batch stream coming from positional delete files
@@ -1099,6 +1124,8 @@ mod tests {
10991124
file_type: DataContentType::PositionDeletes,
11001125
partition_spec_id: 0,
11011126
equality_ids: None,
1127+
content_offset: dv_data_file.content_offset(),
1128+
content_size_in_bytes: dv_data_file.content_size_in_bytes(),
11021129
file_format: DataFileFormat::Puffin,
11031130
referenced_data_file: dv_data_file.referenced_data_file(),
11041131
};

crates/iceberg/src/delete_vector.rs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,8 +156,17 @@ impl DeleteVector {
156156
format!("unsupported puffin blob type: {}", blob.blob_type()),
157157
));
158158
}
159+
Self::from_serialized_bytes(blob.data())
160+
}
159161

160-
let data = blob.data();
162+
/// Parse a serialized `deletion-vector-v1` blob payload:
163+
/// `[u32 BE length][magic 0xD1D33964][serialized RoaringTreemap][u32 BE CRC32]`.
164+
///
165+
/// These are exactly the bytes located by a manifest entry's `content_offset`
166+
/// and `content_size_in_bytes`, so this works whether the deletion vector is a
167+
/// standalone blob file (e.g. written by DuckDB) or stored inside a Puffin
168+
/// container (the manifest offset points past the container framing).
169+
pub fn from_serialized_bytes(data: &[u8]) -> Result<Self> {
161170
if data.len() < MIN_SERIALIZED_DELETION_VECTOR_BLOB {
162171
return Err(Error::new(
163172
ErrorKind::DataInvalid,

crates/iceberg/src/scan/task.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,8 @@ impl From<&DeleteFileContext> for FileScanTaskDeleteFile {
163163
.with_equality_ids(ctx.manifest_entry.data_file.equality_ids.clone())
164164
.with_file_format(ctx.manifest_entry.data_file().file_format())
165165
.with_referenced_data_file(ctx.manifest_entry.data_file().referenced_data_file())
166+
.with_content_offset(ctx.manifest_entry.data_file().content_offset())
167+
.with_content_size_in_bytes(ctx.manifest_entry.data_file().content_size_in_bytes())
166168
.build()
167169
}
168170
}
@@ -196,4 +198,14 @@ pub struct FileScanTaskDeleteFile {
196198
/// `referenced_data_file`). `None` for positional/equality delete files.
197199
#[builder(default)]
198200
pub referenced_data_file: Option<String>,
201+
202+
/// Offset of the deletion-vector blob within the file (the manifest entry's
203+
/// `content_offset`). `None` for positional/equality delete files.
204+
#[builder(default)]
205+
pub content_offset: Option<i64>,
206+
207+
/// Size in bytes of the deletion-vector blob (the manifest entry's
208+
/// `content_size_in_bytes`). `None` for positional/equality delete files.
209+
#[builder(default)]
210+
pub content_size_in_bytes: Option<i64>,
199211
}

0 commit comments

Comments
 (0)