Skip to content

feat: add timetravel by version number #1044

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 53 additions & 8 deletions ffi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -568,14 +568,31 @@ pub unsafe extern "C" fn snapshot(
) -> ExternResult<Handle<SharedSnapshot>> {
let url = unsafe { unwrap_and_parse_path_as_url(path) };
let engine = unsafe { engine.as_ref() };
snapshot_impl(url, engine).into_extern_result(&engine)
snapshot_impl(url, engine, None).into_extern_result(&engine)
}

/// Get the snapshot from the specified table at a specific version
///
/// # Safety
///
/// Caller is responsible for passing valid handles and path pointer.
#[no_mangle]
pub unsafe extern "C" fn snapshot_at_version(
path: KernelStringSlice,
engine: Handle<SharedExternEngine>,
version: u64,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do things work if we make this version: Version (i.e. is cbindgen clever enough to translate that?)

I'd prefer to have this change too if we ever change the type for Version

) -> ExternResult<Handle<SharedSnapshot>> {
let url = unsafe { unwrap_and_parse_path_as_url(path) };
let engine = unsafe { engine.as_ref() };
snapshot_impl(url, engine, version.into()).into_extern_result(&engine)
}

fn snapshot_impl(
url: DeltaResult<Url>,
extern_engine: &dyn ExternEngine,
version: Option<u64>,
) -> DeltaResult<Handle<SharedSnapshot>> {
let snapshot = Snapshot::try_new(url?, extern_engine.engine().as_ref(), None)?;
let snapshot = Snapshot::try_new(url?, extern_engine.engine().as_ref(), version)?;
Ok(Arc::new(snapshot).into())
}

Expand Down Expand Up @@ -776,6 +793,12 @@ mod tests {
Some(ptr)
}

// helper to recover an error from the above
fn recover_error(ptr: *mut EngineError) -> EngineError {
let ptr = ptr.cast();
*unsafe { Box::from_raw(ptr) }
}

// helper to recover a string from the above
fn recover_string(ptr: NonNull<c_void>) -> String {
let ptr = ptr.as_ptr().cast();
Expand Down Expand Up @@ -834,18 +857,40 @@ mod tests {
let engine = engine_to_handle(Arc::new(engine), allocate_err);
let path = "memory:///";

let snapshot =
// Test getting latest snapshot
let snapshot1 =
unsafe { ok_or_panic(snapshot(kernel_string_slice!(path), engine.shallow_copy())) };
let version1 = unsafe { version(snapshot1.shallow_copy()) };
assert_eq!(version1, 0);

// Test getting snapshot at version
let snapshot2 = unsafe {
ok_or_panic(snapshot_at_version(
kernel_string_slice!(path),
engine.shallow_copy(),
0,
))
};
let version2 = unsafe { version(snapshot2.shallow_copy()) };
assert_eq!(version2, 0);

// Test getting non-existent snapshot
let snapshot_at_non_existent_version =
unsafe { snapshot_at_version(kernel_string_slice!(path), engine.shallow_copy(), 1) };
assert!(snapshot_at_non_existent_version.is_err());

// Avoid leaking the error by recovering it
if let ExternResult::Err(e) = snapshot_at_non_existent_version {
recover_error(e);
}

let version = unsafe { version(snapshot.shallow_copy()) };
assert_eq!(version, 0);

let table_root = unsafe { snapshot_table_root(snapshot.shallow_copy(), allocate_str) };
let table_root = unsafe { snapshot_table_root(snapshot1.shallow_copy(), allocate_str) };
assert!(table_root.is_some());
let s = recover_string(table_root.unwrap());
assert_eq!(&s, path);

unsafe { free_snapshot(snapshot) }
unsafe { free_snapshot(snapshot1) }
unsafe { free_snapshot(snapshot2) }
unsafe { free_engine(engine) }
Ok(())
}
Expand Down
Loading