Skip to content

Commit 98bfbc6

Browse files
authored
feat: Add deletion vector file writer (delta-io#1425)
Adds a deletion vector writer and related helper classes, as part of work to expose writing deletion vectors in the Kernel. - Adds a trait for `DeletionVector` and a default implementation that is backed by a `RoaringTreemap`. - Adds a DV Writer that can write multiple DVs to a single file. - Adds a first class concept for DeletionVector path that can be used to generate either the absolute path or the encoded relative path. This is used to give back a location to writers that want to write new DVs, and then create new DeletionVectorDescriptor objects that combine the offsets. - To facilitate testing, updated `DeletionVectorDescriptor` to have a read method that supports validating CRC while reading the deletion vector. TESTING=Verified both values written directly, and round tripping with DeletionVectorDescriptor for reads.
1 parent 6468b2f commit 98bfbc6

5 files changed

Lines changed: 1087 additions & 35 deletions

File tree

kernel/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ pre-release-hook = [
4242
delta_kernel_derive = { path = "../derive-macros", version = "0.16.0" }
4343
bytes = "1.10"
4444
chrono = "0.4.41"
45+
crc = "3.2.2"
4546
indexmap = "2.10.0"
4647
itertools = "0.14"
4748
roaring = "0.11.2"

kernel/src/actions/deletion_vector.rs

Lines changed: 235 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ use delta_kernel_derive::ToSchema;
1010
use roaring::RoaringTreemap;
1111
use url::Url;
1212

13+
use crc::{Crc, CRC_32_ISO_HDLC};
14+
1315
use crate::schema::DataType;
1416
use crate::utils::require;
1517
use crate::{DeltaResult, Error, StorageHandler};
@@ -57,6 +59,63 @@ impl ToDataType for DeletionVectorStorageType {
5759
}
5860
}
5961

62+
/// Represents an abstract path to a deletion vector file.
63+
///
64+
/// This is used in the public API to construct the path to a deletion vector file and
65+
/// has logic to convert [`crate::actions::deletion_vector_writer::DeletionVectorWriteResult`]
66+
/// to a [`DeletionVectorDescriptor`] with appropriate storage type and path.
67+
pub struct DeletionVectorPath {
68+
/// The base URL path to the Delta table
69+
table_path: Url,
70+
/// Unique identifier for this deletion vector file
71+
uuid: uuid::Uuid,
72+
/// Optional directory prefix within the table path where the DV file will be located,
73+
/// this is to allow for randomizing reads/writes to avoid object store throttling.
74+
prefix: String,
75+
}
76+
77+
impl DeletionVectorPath {
78+
pub(crate) fn new(table_path: Url, prefix: String) -> Self {
79+
Self {
80+
table_path,
81+
uuid: uuid::Uuid::new_v4(),
82+
prefix,
83+
}
84+
}
85+
86+
#[cfg(test)]
87+
pub(crate) fn new_with_uuid(table_path: Url, prefix: String, uuid: uuid::Uuid) -> Self {
88+
Self {
89+
table_path,
90+
uuid,
91+
prefix,
92+
}
93+
}
94+
95+
/// Helper method to construct the relative path to a deletion vector file
96+
/// from the prefix and UUID suffix.
97+
fn relative_path(prefix: &str, uuid: &uuid::Uuid) -> String {
98+
if !prefix.is_empty() {
99+
format!("{prefix}/deletion_vector_{uuid}.bin")
100+
} else {
101+
format!("deletion_vector_{uuid}.bin")
102+
}
103+
}
104+
105+
/// Returns the absolute path to the deletion vector file.
106+
pub fn absolute_path(&self) -> DeltaResult<Url> {
107+
let dv_suffix = Self::relative_path(&self.prefix, &self.uuid);
108+
self.table_path
109+
.join(&dv_suffix)
110+
.map_err(|_| Error::DeletionVector(format!("invalid path: {dv_suffix}")))
111+
}
112+
113+
/// Returns the compressed encoded path for use in descriptor (prefix + z85 encoded UUID).
114+
pub(crate) fn encoded_relative_path(&self) -> String {
115+
format!("{}{}", self.prefix, z85::encode(self.uuid.as_bytes()))
116+
}
117+
}
118+
60119
#[derive(Debug, Clone, PartialEq, Eq, ToSchema)]
61120
#[cfg_attr(
62121
test,
@@ -126,14 +185,8 @@ impl DeletionVectorDescriptor {
126185
.map_err(|_| Error::deletion_vector("Failed to decode DV uuid"))?;
127186
let uuid = uuid::Uuid::from_slice(&decoded)
128187
.map_err(|err| Error::DeletionVector(err.to_string()))?;
129-
let dv_suffix = if prefix_len > 0 {
130-
format!(
131-
"{}/deletion_vector_{uuid}.bin",
132-
&self.path_or_inline_dv[..prefix_len]
133-
)
134-
} else {
135-
format!("deletion_vector_{uuid}.bin")
136-
};
188+
let dv_suffix =
189+
DeletionVectorPath::relative_path(&self.path_or_inline_dv[..prefix_len], &uuid);
137190
let dv_path = parent
138191
.join(&dv_suffix)
139192
.map_err(|_| Error::DeletionVector(format!("invalid path: {dv_suffix}")))?;
@@ -174,54 +227,118 @@ impl DeletionVectorDescriptor {
174227
}
175228
}
176229
Some(path) => {
177-
let offset = self.offset;
178-
let size_in_bytes = self.size_in_bytes;
230+
let size_in_bytes: u32 =
231+
self.size_in_bytes
232+
.try_into()
233+
.or(Err(Error::DeletionVector(format!(
234+
"size_in_bytes doesn't fit in usize for {path}"
235+
))))?;
179236

180237
let dv_data = storage
181-
.read_files(vec![(path, None)])?
238+
.read_files(vec![(path.clone(), None)])?
182239
.next()
183-
.ok_or(Error::missing_data("No deletion vector data"))??;
240+
.ok_or(Error::missing_data(format!(
241+
"No deletion vector data for {path}"
242+
)))??;
243+
let dv_data_len = dv_data.len();
184244

185245
let mut cursor = Cursor::new(dv_data);
186246
let mut version_buf = [0; 1];
187-
cursor
188-
.read(&mut version_buf)
189-
.map_err(|err| Error::DeletionVector(err.to_string()))?;
247+
cursor.read(&mut version_buf).map_err(|err| {
248+
Error::DeletionVector(format!("Failed to read version from {path}: {err}"))
249+
})?;
190250
let version = u8::from_be_bytes(version_buf);
191251
require!(
192252
version == 1,
193-
Error::DeletionVector(format!("Invalid version: {version}"))
253+
Error::DeletionVector(format!("Invalid version {version} for {path}"))
194254
);
195255

196-
if let Some(offset) = offset {
197-
cursor.set_position(offset as u64);
198-
}
256+
// Deletion vector file format:
257+
// +---------------+-----------------+
258+
// | num bytes | value |
259+
// +===============+=================+
260+
// | 1 byte | version |
261+
// +---------------+-----------------+
262+
// | offset-1 | other dvs... |
263+
// +---------------+-----------------+ <- this_dv_start
264+
// | 4 bytes | dv_size |
265+
// +---------------+-----------------+
266+
// | 4 bytes | magic value |
267+
// +---------------+-----------------+ <- bitmap_start
268+
// | dv_size - 4 | bitmap |
269+
// +---------------+-----------------+ <- crc_start
270+
// | 4 bytes | CRC |
271+
// +---------------+-----------------+
272+
273+
let this_dv_start: usize =
274+
self.offset
275+
.unwrap_or(1)
276+
.try_into()
277+
.or(Err(Error::DeletionVector(format!(
278+
"Offset {:?} doesn't fit in usize for {path}",
279+
self.offset
280+
))))?;
281+
let magic_start = this_dv_start + 4;
282+
// bitmap_start = this_dv_start + 4 (dv_size field) + 4 (magic field)
283+
let bitmap_start = this_dv_start + 8;
284+
// crc_start = this_dv_start + 4 (dv_size field) + dv_size (magic field + bitmap)
285+
// Safety: size_in_bytes is checked to fit in u32 which for all known platforms should
286+
// fix in usize range.
287+
let crc_start = this_dv_start + 4 + (size_in_bytes as usize);
288+
require!(
289+
this_dv_start < dv_data_len,
290+
Error::DeletionVector(format!(
291+
"This DV start is out of bounds for {path} (Offset: {this_dv_start} >= Size: {dv_data_len})"
292+
))
293+
);
294+
295+
cursor.set_position(this_dv_start as u64);
199296
let dv_size = read_u32(&mut cursor, Endian::Big)?;
200297
require!(
201-
dv_size == size_in_bytes as u32,
298+
dv_size == size_in_bytes,
202299
Error::DeletionVector(format!(
203-
"DV size mismatch. Log indicates {size_in_bytes}, file says: {dv_size}"
300+
"DV size mismatch for {path}. Log indicates {size_in_bytes}, file says: {dv_size}"
204301
))
205302
);
206303
let magic = read_u32(&mut cursor, Endian::Little)?;
207304
require!(
208305
magic == 1681511377,
209-
Error::DeletionVector(format!("Invalid magic: {magic}"))
306+
Error::DeletionVector(format!("Invalid magic {magic} for {path}"))
210307
);
211308

212-
// get the Bytes back out and limit it to dv_size
213-
let position = cursor.position();
214-
let mut bytes = cursor.into_inner();
215-
let truncate_pos = position + dv_size as u64;
216-
assert!(
217-
truncate_pos <= usize::MAX as u64,
218-
"Can't truncate as truncate_pos is > usize::MAX"
309+
let bytes = cursor.into_inner();
310+
311+
// +4 to account for CRC value
312+
require!(
313+
bytes.len() >= crc_start + 4,
314+
Error::DeletionVector(format!(
315+
"Can't read deletion vector for {path} as there are not enough bytes. Expected {}, but got {}",
316+
crc_start + 4,
317+
bytes.len()
318+
))
219319
);
220-
bytes.truncate(truncate_pos as usize);
221-
let mut cursor = Cursor::new(bytes);
222-
cursor.set_position(position);
223-
RoaringTreemap::deserialize_from(cursor)
224-
.map_err(|err| Error::DeletionVector(err.to_string()))
320+
321+
let mut crc_cursor: Cursor<Bytes> =
322+
Cursor::new(bytes.slice(crc_start..crc_start + 4));
323+
let crc = read_u32(&mut crc_cursor, Endian::Big)?;
324+
let crc32 = create_dv_crc32();
325+
// CRC is calculated from magic field through end of bitmap
326+
// Safety: verified bytes is larger than crc_start + 4, above.
327+
let expected_crc = crc32.checksum(&bytes.slice(magic_start..crc_start));
328+
require!(
329+
crc == expected_crc,
330+
Error::DeletionVector(format!(
331+
"CRC32 checksum mismatch for {path}. Got: {crc}, expected: {expected_crc}"
332+
))
333+
);
334+
// Safety: verified bytes is larger than crc_start + 4, above.
335+
let dv_bytes = bytes.slice(bitmap_start..crc_start);
336+
let cursor = Cursor::new(dv_bytes);
337+
RoaringTreemap::deserialize_from(cursor).map_err(|err| {
338+
Error::DeletionVector(format!(
339+
"Failed to deserialize deletion vector for {path}: {err}"
340+
))
341+
})
225342
}
226343
}
227344
}
@@ -242,6 +359,12 @@ enum Endian {
242359
Little,
243360
}
244361

362+
/// Factory function to create a CRC-32 instance using the ISO HDLC algorithm.
363+
/// This ensures consistent CRC algorithm usage for deletion vectors.
364+
pub(crate) fn create_dv_crc32() -> Crc<u32> {
365+
Crc::<u32>::new(&CRC_32_ISO_HDLC)
366+
}
367+
245368
/// small helper to read a big or little endian u32 from a cursor
246369
fn read_u32(cursor: &mut Cursor<Bytes>, endian: Endian) -> DeltaResult<u32> {
247370
let mut buf = [0; 4];
@@ -441,7 +564,7 @@ mod tests {
441564
let storage = sync_engine.storage_handler();
442565

443566
let example = dv_example();
444-
let tree_map = example.read(storage, &parent).unwrap();
567+
let tree_map = example.read(storage.clone(), &parent).unwrap();
445568

446569
let expected: Vec<u64> = vec![0, 9];
447570
let found = tree_map.iter().collect::<Vec<_>>();
@@ -566,4 +689,81 @@ mod tests {
566689
assert_eq!(variant, parsed);
567690
}
568691
}
692+
693+
#[test]
694+
fn test_deletion_vector_path_uniqueness() {
695+
// Verify that two DeletionVectorPath instances created with the same arguments
696+
// produce different absolute paths due to unique UUIDs
697+
let table_path = Url::parse("file:///tmp/test_table/").unwrap();
698+
let prefix = String::from("deletion_vectors");
699+
700+
let dv_path1 = DeletionVectorPath::new(table_path.clone(), prefix.clone());
701+
let dv_path2 = DeletionVectorPath::new(table_path.clone(), prefix.clone());
702+
703+
let abs_path1 = dv_path1.absolute_path().unwrap();
704+
let abs_path2 = dv_path2.absolute_path().unwrap();
705+
706+
// The absolute paths should be different because each DeletionVectorPath
707+
// gets a unique UUID
708+
assert_ne!(abs_path1, abs_path2);
709+
assert_ne!(
710+
dv_path1.encoded_relative_path(),
711+
dv_path2.encoded_relative_path()
712+
);
713+
}
714+
715+
#[test]
716+
fn test_deletion_vector_path_absolute_path_with_prefix() {
717+
let table_path = Url::parse("file:///tmp/test_table/").unwrap();
718+
let prefix = String::from("dv");
719+
let known_uuid = uuid::Uuid::parse_str("abcdef01-2345-6789-abcd-ef0123456789").unwrap();
720+
721+
let dv_path = DeletionVectorPath::new_with_uuid(table_path.clone(), prefix, known_uuid);
722+
let abs_path = dv_path.absolute_path().unwrap();
723+
724+
// Verify the exact path with known UUID
725+
let expected =
726+
"file:///tmp/test_table/dv/deletion_vector_abcdef01-2345-6789-abcd-ef0123456789.bin";
727+
assert_eq!(abs_path.as_str(), expected);
728+
}
729+
730+
#[test]
731+
fn test_deletion_vector_path_absolute_path_with_known_uuid() {
732+
// Test with a known UUID to verify exact path construction
733+
let table_path = Url::parse("file:///tmp/test_table/").unwrap();
734+
let prefix = String::from("dv");
735+
let known_uuid = uuid::Uuid::parse_str("550e8400-e29b-41d4-a716-446655440000").unwrap();
736+
737+
let dv_path = DeletionVectorPath::new_with_uuid(table_path, prefix, known_uuid);
738+
let abs_path = dv_path.absolute_path().unwrap();
739+
740+
// Verify the exact path is constructed correctly
741+
let expected_path =
742+
"file:///tmp/test_table/dv/deletion_vector_550e8400-e29b-41d4-a716-446655440000.bin";
743+
assert_eq!(abs_path.as_str(), expected_path);
744+
745+
// Verify the encoded_relative_path is exactly as expected (prefix + z85 encoded UUID: 20 chars)
746+
let encoded = dv_path.encoded_relative_path();
747+
assert_eq!(encoded, "dvrsTVZ&*Sl-RXRWjryu/!");
748+
}
749+
750+
#[test]
751+
fn test_deletion_vector_path_absolute_path_with_known_uuid_empty_prefix() {
752+
// Test with a known UUID and empty prefix
753+
let table_path = Url::parse("file:///tmp/test_table/").unwrap();
754+
let prefix = String::from("");
755+
let known_uuid = uuid::Uuid::parse_str("123e4567-e89b-12d3-a456-426614174000").unwrap();
756+
757+
let dv_path = DeletionVectorPath::new_with_uuid(table_path, prefix, known_uuid);
758+
let abs_path = dv_path.absolute_path().unwrap();
759+
760+
// Verify the exact path is constructed correctly without prefix directory
761+
let expected_path =
762+
"file:///tmp/test_table/deletion_vector_123e4567-e89b-12d3-a456-426614174000.bin";
763+
assert_eq!(abs_path.as_str(), expected_path);
764+
765+
// Verify the encoded_relative_path is exactly as expected (z85 encoded UUID: 20 chars)
766+
let encoded = dv_path.encoded_relative_path();
767+
assert_eq!(encoded, "5<w-%>:JjlQ/G/]6C<1m");
768+
}
569769
}

0 commit comments

Comments
 (0)