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

Conversation

samansmink
Copy link
Contributor

What changes are proposed in this pull request?

  • Add a new ffi function called snapshot_at_version, which allows getting a specific snapshot of a table.

While this is by itself not super useful to users yet (there's not really a way to list snapshots through the ffi), it will allow testing timetravel in the DuckDB delta extension. A follow up to this PR could be to add:

  • s function to list snapshots
  • a function that gets a snapshot that corresponds to a timestamp

This PR affects the following public APIs

  • snapshot_at_version added to the ffi

How was this change tested?

using a few extra lines that are added to the existing snapshot ffi function tests.

Copy link
Collaborator

@joonyoo181 joonyoo181 left a comment

Choose a reason for hiding this comment

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

added a small nit. but otherwise, it looks good to me!

@samansmink samansmink force-pushed the add-timetravel-by-version-number branch from 907cbc3 to f5e815b Compare July 3, 2025 08:27
Copy link
Collaborator

@nicklan nicklan left a comment

Choose a reason for hiding this comment

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

lgtm, maybe just with the type change if it works. thanks!

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

Copy link

codecov bot commented Jul 3, 2025

Codecov Report

Attention: Patch coverage is 94.11765% with 2 lines in your changes missing coverage. Please review.

Project coverage is 83.93%. Comparing base (4a13e74) to head (f5e815b).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
ffi/src/lib.rs 94.11% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1044      +/-   ##
==========================================
- Coverage   84.74%   83.93%   -0.81%     
==========================================
  Files          92       92              
  Lines       23407    21097    -2310     
  Branches    23407    21097    -2310     
==========================================
- Hits        19836    17708    -2128     
+ Misses       2587     2473     -114     
+ Partials      984      916      -68     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants